Merge pull request #295: scalar run all

Resolves #287.

The first commit refactors some functional test code to make it a bit easier to see that we are running `scalar run <task>` in these tests. Also reduces the code required to add a new task (i.e. the "all" task).

The second commit adds a new "all" task. This requires modifying the code quite a bit to pull off with less duplication.

The end result looks like this:

```
$ scalar run all
Setting recommended config settings...Succeeded
Fetching from remotes...Succeeded
Updating commit-graph...Succeeded
Cleaning up loose objects...Succeeded
Cleaning up pack-files...Succeeded
```

The ordering is documented in `docs/advanced.md`, including some reasoning for the order. Basically, earlier steps may produce data that can be used by the later steps (fetch gets new packs and loose objects, commit-graph operates on new refs and may download new loose objects, loose-object step creates a pack, pack-file step repacks).

Notice that an output line is being added to every task, where previously only the "fetch" task had this kind of output.

There is a slight change of behavior here that was missed when we started running the verb from the service: we had `forceRun: true` in some of the steps that check a timestamp before running. This should not be true when run from the service, as the service wants to ensure a foreground run delays background runs for the right length of time. This is really only important for multiple enlistments sharing an object cache.

Scalar.Common/Maintenance/CommitGraphStep.cs

@@ -17,6 +17,8 @@ public CommitGraphStep(ScalarContext context, bool requireObjectCacheLock = true
 
         public override string Area => "CommitGraphStep";
 
+        public override string ProgressMessage => "Updating commit-graph";
+
         protected override void PerformMaintenance()
         {
             using (ITracer activity = this.Context.Tracer.StartActivity("TryWriteGitCommitGraph", EventLevel.Informational))

Scalar.Common/Maintenance/ConfigStep.cs

@@ -71,6 +71,8 @@ public ConfigStep(ScalarContext context, bool? useGvfsProtocol = null) : base(co
             this.UseGvfsProtocol = useGvfsProtocol;
         }
 
+        public override string ProgressMessage => "Setting recommended config settings";
+
         public bool TrySetConfig(out string error)
         {
             string coreGVFSFlags = Convert.ToInt32(

Scalar.Common/Maintenance/FetchStep.cs

@@ -33,6 +33,25 @@ public FetchStep(
 
         public override string Area => "FetchCommitsAndTreesStep";
 
+        public override string ProgressMessage
+        {
+            get
+            {
+                if (!this.Context.Enlistment.UsesGvfsProtocol)
+                {
+                    return "Fetching from remotes";
+                }
+                else if (this.GitObjects.IsUsingCacheServer())
+                {
+                    return "Fetching from cache server";
+                }
+                else
+                {
+                    return "Fetching from origin (no cache server)";
+                }
+            }
+        }
+
         // Used only for vanilla Git repos
         protected override TimeSpan TimeBetweenRuns => this.timeBetweenFetches;
 

Scalar.Common/Maintenance/GitMaintenanceStep.cs

@@ -19,6 +19,9 @@ public GitMaintenanceStep(ScalarContext context, bool requireObjectCacheLock, Gi
         }
 
         public abstract string Area { get; }
+
+        public abstract string ProgressMessage { get; }
+
         protected virtual TimeSpan TimeBetweenRuns { get; }
         protected virtual string LastRunTimeFilePath { get; set; }
         protected ScalarContext Context { get; }

Scalar.Common/Maintenance/LooseObjectsStep.cs

@@ -41,6 +41,8 @@ public enum CreatePackResult
         protected override string LastRunTimeFilePath => Path.Combine(this.Context.Enlistment.GitObjectsRoot, "info", LooseObjectsLastRunFileName);
         protected override TimeSpan TimeBetweenRuns => TimeSpan.FromDays(1);
 
+        public override string ProgressMessage => "Cleaning up loose objects";
+
         public void CountLooseObjects(out int count, out long size)
         {
             count = 0;

Scalar.Common/Maintenance/PackfileMaintenanceStep.cs

@@ -45,6 +45,9 @@ public PackfileMaintenanceStep(
         }
 
         public override string Area => nameof(PackfileMaintenanceStep);
+
+        public override string ProgressMessage => "Cleaning up pack-files";
+
         protected override string LastRunTimeFilePath => Path.Combine(this.Context.Enlistment.GitObjectsRoot, "info", PackfileLastRunFileName);
         protected override TimeSpan TimeBetweenRuns => TimeSpan.FromDays(1);
 

Scalar.Common/ScalarConstants.cs

@@ -198,11 +198,12 @@ public static class Unmount
 
             public static class Maintenance
             {
+                public const string AllTasksName = "all";
+                public const string ConfigTaskName = "config";
+                public const string CommitGraphTaskName = "commit-graph";
                 public const string FetchTaskName = "fetch";
                 public const string LooseObjectsTaskName = "loose-objects";
                 public const string PackFilesTaskName = "pack-files";
-                public const string CommitGraphTaskName = "commit-graph";
-                public const string ConfigTaskName = "config";
 
                 public const string BatchSizeOptionName = "batch-size";
             }

Scalar.FunctionalTests/Tests/EnlistmentPerFixture/CommitGraphStepTests.cs

@@ -29,7 +29,7 @@ public void CreateCommitGraphWhenMissing()
         {
             RepositoryHelpers.DeleteTestDirectory(this.CommitGraphsRoot);
 
-            this.Enlistment.CommitGraphStep();
+            this.Enlistment.RunVerb("commit-graph");
 
             this.fileSystem
                 .FileExists(this.CommitGraphsChain)
@@ -58,7 +58,7 @@ public void CleansUpOphanedLockFiles()
 
             this.fileSystem.CreateEmptyFile(graphLockPath);
 
-            this.Enlistment.CommitGraphStep();
+            this.Enlistment.RunVerb("commit-graph");
 
             this.fileSystem.FileExists(graphLockPath).ShouldBeFalse(nameof(graphLockPath));
             this.fileSystem.FileExists(this.CommitGraphsChain).ShouldBeTrue(nameof(this.CommitGraphsChain));

Scalar.FunctionalTests/Tests/EnlistmentPerFixture/FetchStepTests.cs

@@ -26,7 +26,7 @@ public FetchStepTests()
         [Category(Categories.MacTODO.TestNeedsToLockFile)]
         public void FetchStepCleansUpStaleFetchLock()
         {
-            this.Enlistment.FetchStep();
+            this.Enlistment.RunVerb("fetch");
             string fetchCommitsLockFile = Path.Combine(
                 ScalarHelpers.GetObjectsRootFromGitConfig(this.Enlistment.RepoRoot),
                 "pack",
@@ -42,7 +42,7 @@ public void FetchStepCleansUpStaleFetchLock()
                 .Count()
                 .ShouldEqual(1, "Incorrect number of .keep files in pack directory");
 
-            this.Enlistment.FetchStep();
+            this.Enlistment.RunVerb("fetch");
             fetchCommitsLockFile.ShouldNotExistOnDisk(this.fileSystem);
         }
     }

Scalar.FunctionalTests/Tests/EnlistmentPerFixture/FetchStepWithoutSharedCacheTests.cs

@@ -43,7 +43,7 @@ private string TempPackRoot
         [TestCase, Order(1)]
         public void FetchStepCommitsToEmptyCache()
         {
-            this.Enlistment.FetchStep();
+            this.Enlistment.RunVerb("fetch");
 
             // Verify prefetch pack(s) are in packs folder and have matching idx file
             string[] prefetchPacks = this.ReadPrefetchPackFileNames();
@@ -66,7 +66,7 @@ public void FetchStepBuildsIdxWhenMissingFromPrefetchPack()
             idxPath.ShouldNotExistOnDisk(this.fileSystem);
 
             // fetch should rebuild the missing idx
-            this.Enlistment.FetchStep();
+            this.Enlistment.RunVerb("fetch");
 
             idxPath.ShouldBeAFile(this.fileSystem);
 
@@ -90,7 +90,7 @@ public void FetchStepCleansUpBadPrefetchPack()
             badPackPath.ShouldBeAFile(this.fileSystem).WithContents(badContents);
 
             // fetch should delete the bad pack
-            this.Enlistment.FetchStep();
+            this.Enlistment.RunVerb("fetch");
 
             badPackPath.ShouldNotExistOnDisk(this.fileSystem);
 
@@ -119,12 +119,12 @@ public void FetchStepFailsWhenItCannotRemoveABadPrefetchPack()
             // Open a handle to the bad pack that will prevent fetch-commits-and-trees from being able to delete it
             using (FileStream stream = new FileStream(badPackPath, FileMode.Open, FileAccess.Read, FileShare.None))
             {
-                string output = this.Enlistment.FetchStep(failOnError: false);
+                string output = this.Enlistment.RunVerb("fetch", failOnError: false);
                 output.ShouldContain($"Unable to delete {badPackPath}");
             }
 
             // After handle is closed fetching commits and trees should succeed
-            this.Enlistment.FetchStep();
+            this.Enlistment.RunVerb("fetch");
 
             badPackPath.ShouldNotExistOnDisk(this.fileSystem);
 
@@ -164,7 +164,7 @@ public void FetchCommitsAndTreesCleansUpStaleTempPrefetchPacks()
             this.fileSystem.WriteAllText(otherFilePath, otherFileContents);
             otherFilePath.ShouldBeAFile(this.fileSystem).WithContents(otherFileContents);
 
-            this.Enlistment.FetchStep();
+            this.Enlistment.RunVerb("fetch");
 
             // Validate stale prefetch packs are cleaned up
             Directory.GetFiles(this.TempPackRoot, $"{PrefetchPackPrefix}*.pack").ShouldBeEmpty("There should be no .pack files in the tempPack folder");

Scalar.FunctionalTests/Tests/EnlistmentPerFixture/LooseObjectStepTests.cs

@@ -38,7 +38,7 @@ public void NoLooseObjectsDoesNothing()
             this.GetLooseObjectFiles().Count.ShouldEqual(0);
             int startingPackFileCount = this.CountPackFiles();
 
-            this.Enlistment.LooseObjectStep();
+            this.Enlistment.RunVerb("loose-objects");
 
             this.GetLooseObjectFiles().Count.ShouldEqual(0);
             this.CountPackFiles().ShouldEqual(startingPackFileCount);
@@ -56,7 +56,7 @@ public void RemoveLooseObjectsInPackFiles()
             this.CountPackFiles().ShouldEqual(1);
 
             // Cleanup should delete all loose objects, since they are in the packfile
-            this.Enlistment.LooseObjectStep();
+            this.Enlistment.RunVerb("loose-objects");
 
             this.GetLooseObjectFiles().Count.ShouldEqual(0);
             this.CountPackFiles().ShouldEqual(1);
@@ -76,13 +76,13 @@ public void PutLooseObjectsInPackFiles()
             looseObjectCount.ShouldBeAtLeast(1);
 
             // This step should put the loose objects into a packfile
-            this.Enlistment.LooseObjectStep();
+            this.Enlistment.RunVerb("loose-objects");
 
             this.GetLooseObjectFiles().Count.ShouldEqual(looseObjectCount);
             this.CountPackFiles().ShouldEqual(1);
 
             // Running the step a second time should remove the loose obects and keep the pack file
-            this.Enlistment.LooseObjectStep();
+            this.Enlistment.RunVerb("loose-objects");
 
             this.GetLooseObjectFiles().Count.ShouldEqual(0);
             this.CountPackFiles().ShouldEqual(1);
@@ -110,7 +110,7 @@ public void CorruptLooseObjectIsDeleted()
 
             // This step should fail to place the objects, but
             // succeed in deleting the given file.
-            this.Enlistment.LooseObjectStep();
+            this.Enlistment.RunVerb("loose-objects");
 
             this.fileSystem.FileExists(fakeBlob).ShouldBeFalse(
                    "Step failed to delete corrupt blob");
@@ -120,13 +120,13 @@ public void CorruptLooseObjectIsDeleted()
                 "unexpected number of loose objects after step");
 
             // This step should create a pack.
-            this.Enlistment.LooseObjectStep();
+            this.Enlistment.RunVerb("loose-objects");
 
             this.CountPackFiles().ShouldEqual(1, "Incorrect number of packs after second loose object step");
             this.GetLooseObjectFiles().Count.ShouldEqual(looseObjectCount);
 
             // This step should delete the loose objects
-            this.Enlistment.LooseObjectStep();
+            this.Enlistment.RunVerb("loose-objects");
 
             this.GetLooseObjectFiles().Count.ShouldEqual(0, "Incorrect number of loose objects after third loose object step");
         }

Scalar.FunctionalTests/Tests/EnlistmentPerFixture/PackfileMaintenanceStepTests.cs

@@ -45,15 +45,15 @@ public void RepackAllToOnePack()
 
             // Run the step to ensure we don't have any packs that will be expired during the repack step.
             // Use a non-standard, but large batchSize to avoid logic that resizes the batchSize.
-            this.Enlistment.PackfileMaintenanceStep(batchSize: BatchSizeForNoRepack);
+            this.Enlistment.RunVerb("pack-files", batchSize: BatchSizeForNoRepack);
 
             this.GetPackSizes(out int afterPrefetchPackCount, out maxSize, out minSize, out totalSize);
 
             // Cannot be sure of the count, as the fetch-commits-and-trees uses parallel threads to get multiple packs
             afterPrefetchPackCount.ShouldBeAtLeast(2);
 
             // Batch size 0 tells Git to repack everything into a single pack!
-            this.Enlistment.PackfileMaintenanceStep(batchSize: 0);
+            this.Enlistment.RunVerb("pack-files", batchSize: 0);
             this.GetPackSizes(out int packCount, out maxSize, out minSize, out totalSize);
 
             // We should not have expired any packs, but created a new one with repack
@@ -70,8 +70,8 @@ public void ExpireAllButOneAndKeep()
 
             // We should expire all packs except the one we just created,
             // and the prefetch pack which is marked as ".keep"
-            this.Enlistment.PackfileMaintenanceStep(batchSize: BatchSizeForNoRepack);
-            
+            this.Enlistment.RunVerb("pack-files", batchSize: BatchSizeForNoRepack);
+
             List<string> packsAfter = this.GetPackfiles();
 
             packsAfter.Count.ShouldEqual(2, $"incorrect number of packs after final expire step: {packsAfter.Count}");

Scalar.FunctionalTests/Tests/GitRepoPerFixture/RunVerbTests.cs

@@ -28,7 +28,7 @@ public RunVerbTests()
         public void CommitGraphStep()
         {
             this.fileSystem.FileExists(CommitGraphChain).ShouldBeFalse();
-            this.Enlistment.CommitGraphStep();
+            this.Enlistment.RunVerb("commit-graph");
             this.fileSystem.FileExists(CommitGraphChain).ShouldBeTrue();
         }
 
@@ -47,7 +47,7 @@ public void PackfileMaintenanceStep()
             minSize.ShouldNotEqual(0, "min size means empty pack-file?");
 
             this.Enlistment
-                .PackfileMaintenanceStep(batchSize: totalSize - minSize + 1)
+                .RunVerb("pack-files", batchSize: totalSize - minSize + 1)
                 .ShouldNotContain(false, "Skipping pack maintenance due to no .keep file.");
 
             this.GetPackSizes(out int countAfterStep, out maxSize, out minSize, out totalSize);
@@ -56,7 +56,7 @@ public void PackfileMaintenanceStep()
             countAfterStep.ShouldEqual(countAfterRepack + 1, nameof(countAfterStep));
 
             this.Enlistment
-                .PackfileMaintenanceStep(batchSize: totalSize - minSize + 1)
+                .RunVerb("pack-files", batchSize: totalSize - minSize + 1)
                 .ShouldNotContain(false, "Skipping pack maintenance due to no .keep file.");
 
             this.GetPackSizes(out int countAfterStep2, out maxSize, out minSize, out totalSize);
@@ -76,14 +76,14 @@ public void LooseObjectsStep()
             minSize.ShouldNotEqual(0, "min size means empty pack-file?");
 
             // This step packs the loose object into a pack.
-            this.Enlistment.LooseObjectStep();
+            this.Enlistment.RunVerb("loose-objects");
             this.GetPackSizes(out int countAfterStep1, out _, out minSize, out _);
             minSize.ShouldNotEqual(0, "min size means empty pack-file?");
             this.GetLooseObjectFiles().Count.ShouldBeAtLeast(1);
             countAfterStep1.ShouldEqual(countBeforeStep + 1, "First step should create a pack");
 
             // This step deletes the loose object that is already in a pack
-            this.Enlistment.LooseObjectStep();
+            this.Enlistment.RunVerb("loose-objects");
             this.GetPackSizes(out int countAfterStep2, out _, out minSize, out _);
             minSize.ShouldNotEqual(0, "min size means empty pack-file?");
             this.GetLooseObjectFiles().Count.ShouldEqual(0);
@@ -104,7 +104,7 @@ public void FetchStep()
             this.fileSystem.DeleteDirectory(refsRemotes);
             this.fileSystem.DeleteDirectory(this.PackRoot);
 
-            this.Enlistment.FetchStep();
+            this.Enlistment.RunVerb("fetch");
 
             this.GetPackSizes(out int countAfterFetch, out _, out _, out _);
 
@@ -114,7 +114,7 @@ public void FetchStep()
             this.fileSystem.DirectoryExists(refsHeads).ShouldBeFalse("background fetch should not have created refs/heads/*");
             this.fileSystem.DirectoryExists(refsRemotes).ShouldBeFalse("background fetch should not have created refs/remotes/*");
 
-            this.Enlistment.FetchStep();
+            this.Enlistment.RunVerb("fetch");
 
             this.fileSystem.DirectoryExists(refsHeads).ShouldBeFalse("background fetch should not have created refs/heads/*");
             this.fileSystem.DirectoryExists(refsRemotes).ShouldBeFalse("background fetch should not have created refs/remotes/*");
@@ -123,6 +123,14 @@ public void FetchStep()
             countAfterFetch2.ShouldEqual(1, "sceond fetch should not download a pack");
         }
 
+
+        [TestCase]
+        [Order(5)]
+        public void AllSteps()
+        {
+            this.Enlistment.RunVerb("all");
+        }
+
         private List<string> GetPackfiles()
         {
             return Directory.GetFiles(this.PackRoot, "*.pack").ToList();

Scalar.FunctionalTests/Tools/ScalarFunctionalTestEnlistment.cs

@@ -187,9 +187,9 @@ public void CloneGitRepo()
             this.InitializeConfig();
         }
 
-        public string FetchStep(bool failOnError = true, string standardInput = null)
+        public string RunVerb(string task, long? batchSize = null, bool failOnError = true)
         {
-            return this.scalarProcess.FetchStep(failOnError, standardInput);
+            return this.scalarProcess.RunVerb(task, batchSize, failOnError);
         }
 
         public void Unregister()
@@ -207,21 +207,6 @@ public string Diagnose()
             return this.scalarProcess.Diagnose();
         }
 
-        public string CommitGraphStep()
-        {
-            return this.scalarProcess.CommitGraphStep();
-        }
-
-        public string LooseObjectStep()
-        {
-            return this.scalarProcess.LooseObjectStep();
-        }
-
-        public string PackfileMaintenanceStep(long? batchSize = null)
-        {
-            return this.scalarProcess.PackfileMaintenanceStep(batchSize);
-        }
-
         public string Status(string trace = null)
         {
             return this.scalarProcess.Status(trace);