diff --git a/GVFS/GVFS.FunctionalTests/Tests/GitCommands/CorruptionReproTests.cs b/GVFS/GVFS.FunctionalTests/Tests/GitCommands/CorruptionReproTests.cs index 9e7eec0bb..4dade3f56 100644 --- a/GVFS/GVFS.FunctionalTests/Tests/GitCommands/CorruptionReproTests.cs +++ b/GVFS/GVFS.FunctionalTests/Tests/GitCommands/CorruptionReproTests.cs @@ -12,7 +12,10 @@ namespace GVFS.FunctionalTests.Tests.GitCommands { /// - /// This class is used to reproduce corruption scenarios in the GVFS virtual projection. + /// Reproduces reported scenarios that corrupted the GVFS virtual projection or + /// gave results that did not match a normal git repo. Each test is a black-box + /// regression test: it runs the command sequence that exposed the problem and + /// compares the results with the control repo. /// [Category(Categories.GitCommands)] [TestFixtureSource(typeof(GitRepoTests), nameof(GitRepoTests.ValidateWorkingTree))] @@ -76,6 +79,32 @@ public void ReproCherryPickRestoreCorruption() this.FilesShouldMatchCheckoutOfSourceBranch(); } + /// + /// Regression test for a sequence of commands that a user ran and that + /// showed incorrect behavior: after "git blame" and "git reset --mixed", + /// GVFS did not report a changed file as modified. The control repo did. + /// + /// This is a black-box test. It does not assert why the behavior was wrong. + /// It only runs the same commands and compares the results with the + /// control repo. ResetMixedTests.ResetMixedClearsSkipWorktreeOnHydratedPlaceholder + /// asserts the conditions that cause the problem. + /// + /// The fix requires microsoft/git v2.55.0.vfs.0.3 or later. + /// + /// On the FunctionalTests/20201014 branch, Readme.md is the only file that + /// differs between HEAD and HEAD~1. + /// + [TestCase] + public void ReproResetMixedSkipWorktree() + { + this.ValidateGitCommand("blame Readme.md"); + this.ValidateGitCommand("checkout -b tests/functional/ReproResetMixedSkipWorktree"); + + // Both repos must report Readme.md as modified. + this.ValidateGitCommand("reset HEAD~1"); + this.ValidateGitCommand("ls-files -v Readme.md"); + } + /// /// Reproduction of a reported issue: /// Restoring a file after its parent directory was deleted fails with diff --git a/GVFS/GVFS.FunctionalTests/Tests/GitCommands/ResetMixedTests.cs b/GVFS/GVFS.FunctionalTests/Tests/GitCommands/ResetMixedTests.cs index 0f7740d90..af77ee6e4 100644 --- a/GVFS/GVFS.FunctionalTests/Tests/GitCommands/ResetMixedTests.cs +++ b/GVFS/GVFS.FunctionalTests/Tests/GitCommands/ResetMixedTests.cs @@ -1,6 +1,9 @@ using GVFS.FunctionalTests.Properties; using GVFS.FunctionalTests.Should; +using GVFS.FunctionalTests.Tools; +using GVFS.Tests.Should; using NUnit.Framework; +using System.IO; namespace GVFS.FunctionalTests.Tests.GitCommands { @@ -30,6 +33,52 @@ public void ResetMixedAfterPrefetch() this.FilesShouldMatchCheckoutOfTargetBranch(); } + /// + /// A mixed reset must clear skip-worktree on a hydrated placeholder whose + /// index entry the reset changes. A hydrated placeholder is on disk but is + /// not in ModifiedPaths, so it still has skip-worktree. The reset does not + /// update the working tree, so the file on disk no longer matches the index. + /// If skip-worktree stays set, git does not compare the file to the index, + /// and reset and status do not report it as modified. + /// + /// Placeholders that are not on disk do not have this problem, because git + /// writes them to disk and clears skip-worktree. For that reason, this test + /// asserts each precondition before the reset. + /// + /// The git side of this behavior requires microsoft/git v2.55.0.vfs.0.3 or later. + /// + [TestCase] + public void ResetMixedClearsSkipWorktreeOnHydratedPlaceholder() + { + string filePath = Path.Combine("Test_ConflictTests", "ModifiedFiles", "ChangeInTarget.txt"); + string gitPath = filePath.Replace(Path.DirectorySeparatorChar, TestConstants.GitPathSeparator); + + // Create local branches for both commits in both repos. + this.ValidateGitCommand("checkout " + GitRepoTests.ConflictSourceBranch); + this.ValidateGitCommand("checkout " + GitRepoTests.ConflictTargetBranch); + + // Precondition: the reset changes the index entry for the file. + GitProcess.InvokeProcess( + this.ControlGitRepo.RootPath, + $"diff --quiet {GitRepoTests.ConflictSourceBranch} {GitRepoTests.ConflictTargetBranch} -- {gitPath}") + .ExitCode.ShouldEqual(1, $"{gitPath} must differ between the reset source and target"); + + // Precondition: the file is a hydrated placeholder. A read hydrates it + // but does not add it to ModifiedPaths, so skip-worktree stays set. + this.Enlistment.GetVirtualPathTo(filePath).ShouldBeAFile(this.FileSystem).WithContents(); + GVFSHelpers.ModifiedPathsShouldNotContain(this.Enlistment, this.FileSystem, gitPath); + this.SkipWorktreeFlagShouldBe(gitPath, expectedFlag: 'S'); + + // The reset output and status must report the file as modified, as in the control repo. + this.ValidateGitCommand("reset --mixed " + GitRepoTests.ConflictSourceBranch); + + // After the reset, skip-worktree is cleared, and GVFS adds the file to + // ModifiedPaths so that later git commands also compare it to the index. + this.SkipWorktreeFlagShouldBe(gitPath, expectedFlag: 'H'); + GVFSHelpers.ModifiedPathsShouldContain(this.Enlistment, this.FileSystem, gitPath); + this.FileContentsShouldMatch(filePath); + } + [TestCase] public void ResetMixedAndCheckoutNewBranch() { @@ -108,5 +157,13 @@ protected override void CreateEnlistment() this.ControlGitRepo.Fetch(GitRepoTests.ConflictTargetBranch); this.ControlGitRepo.Fetch(GitRepoTests.ConflictSourceBranch); } + + private void SkipWorktreeFlagShouldBe(string gitPath, char expectedFlag) + { + // "ls-files -v" prefixes each entry with a tag: "S" means skip-worktree is set, "H" means it is not. + ProcessResult result = GitProcess.InvokeProcess(this.Enlistment.RepoRoot, "ls-files -v -- " + gitPath); + result.ExitCode.ShouldEqual(0, result.Errors); + result.Output.Trim().ShouldEqual($"{expectedFlag} {gitPath}"); + } } } diff --git a/GVFS/GVFS.FunctionalTests/Tools/GitHelpers.cs b/GVFS/GVFS.FunctionalTests/Tools/GitHelpers.cs index 8bdc74f17..48418452d 100644 --- a/GVFS/GVFS.FunctionalTests/Tools/GitHelpers.cs +++ b/GVFS/GVFS.FunctionalTests/Tools/GitHelpers.cs @@ -130,6 +130,11 @@ private static string FilterMessages( return input; } + /// + /// Runs a git command in the control repo and in the GVFS repo, and asserts + /// that the output and errors match. If the command is not "status", this + /// method then runs "status" in both repos and compares that output too. + /// public static void ValidateGitCommand( GVFSFunctionalTestEnlistment enlistment, ControlGitRepo controlGitRepo,