Run functional tests against Scalar-in-C #505

derrickstolee · 2021-04-23T14:09:09Z

The latest vfs-2.32.0 branch in microsoft/git includes the Scalar CLI, so start testing that version of Scalar instead of the .NET version from this repo.

The prefetch refs moved in Git v2.32.0 in response to interactions with custom refspecs. This re-inserted the word "remotes" into the refspec used to fetch the prefetch refs. Signed-off-by: Derrick Stolee <[email protected]>

As is frequently the case with upstream reviews, extensive changes are necessary, and one such change was to move FSMonitor from command modes (à la `--start`, `--stop`) to subcommands (`start`, `stop`, without the double-dash). Therefore we must adjust downstream projects such as Scalar's Functional Test suite accordingly. Signed-off-by: Johannes Schindelin <[email protected]>

Signed-off-by: Derrick Stolee <[email protected]>

We expect microsoft/git to be installed on the PATH, so use that as our source for Scalar, not the recent build of the .NET project.

Signed-off-by: Derrick Stolee <[email protected]>

dscho · 2021-06-09T19:52:25Z

@derrickstolee FYI I rebased this onto vfs-2.32.0 (because I have a hunch that the fsmonitor--daemon --stop vs fsmonitor--daemon stop issue might be responsible for the remaining flakiness of the Functional Tests).

I also used the opportunity to reorder and squash and drop some commits. Most notably, there were some extra files in ade3040 (e.g. a full Git for Windows installer); Would you mind verifying that I did not mess up when I turned it into 39ef264?

derrickstolee · 2021-06-09T20:43:37Z

Directory.Build.props

@@ -36,14 +36,15 @@

    <!-- Version information -->
    <ScalarVersion>0.2.173.2</ScalarVersion>
+    <MinimumGitVersion>v2.25.0.vfs.1.1</MinimumGitVersion>


I see we are adding this even though it is present further down. Something to clean up when we actually merge this.

Hmm. Let me try to remove it.

derrickstolee · 2021-06-09T20:44:17Z

Scalar.FunctionalTests/Tools/ScalarHelpers.cs

        public static string GetInternalParameter()
        {
-            return $"\"{{\\\"ServiceName\\\":\\\"{ScalarServiceProcess.TestServiceName}\\\"," +
-                    "\\\"StartedByService\\\":false}\"";
+            return "\"{}\"";
        }


I think we should be able to delete this entirely.

Let's try that.

Signed-off-by: Derrick Stolee <[email protected]>

Remove tests that check behavior that is unnecessary now: 1. Clones can happen anywhere. The virtualization is gone, so we don't need to worry about collisions this way. Allow users to create confusing setups. 2. We should not be locked to the strange output from previous Scalar commands. Specifically, the cache server output doesn't need to be so specific. We know from the Git tests that setting the cache server works. Signed-off-by: Derrick Stolee <[email protected]>

These tests are unnecessary now because we rely upon the Git commands themselves. The C port of Git has dropped the --batch-size parameter, and that is fine. It only existed for testing purposes, anyway.

This relies on a brittle output check. We have better tests for this in the Git codebase.

We have control over this in the Git maintenance steps. No need to check these tests.

This functionality is mostly replaced by 'git maintenance run' and we have Git tests that check 'scalar run' now.

These have different behavior now that we use the Git norms. Instead rely on the Git tests.

Signed-off-by: Johannes Schindelin <[email protected]>

The C version of Scalar reports the paths with a forward slash as directory separator, even on Windows. Signed-off-by: Johannes Schindelin <[email protected]>

The way Git creates alternate files can include a trailing newline, when the .NET implementation of Scalar did not.

This fits Git's die() error code. Signed-off-by: Derrick Stolee <[email protected]>

Signed-off-by: Derrick Stolee <[email protected]>

This flag is not implemented in the C version of Scalar because there is no Scalar service anymore: all of the scheduled tasks are handled by `git maintenance` instead.

The `vsts/info` endpoint always requires authentication. In the .NET version, this is handled by Scalar directly, but in the C version, authentication is handled by the `gvfs-helper`. Since this only really matters in CI (interactive usage of Scalar with Azure Repos will ask the user to authenticate, but that's handled conveniently by the credential helper), we added support to skip that endpoint via the environment variable `SCALAR_TEST_SKIP_VSTS_INFO`. Signed-off-by: Johannes Schindelin <[email protected]>