microsoft / scalar

Scalar: A set of tools and extensions for Git to allow very large monorepos to run on Git without a virtualization layer
MIT License
1.39k stars 63 forks source link

add Watchman-enabled functional tests to GitHub Actions CI #436

Closed chrisd8088 closed 4 years ago

chrisd8088 commented 4 years ago

We add Watchman-enabled runs of the functional test suite on all GitHub Actions runners for all three platforms.

The code changes in this PR are primarily the result of needing to work around some inconsistent behaviour from Watchman when running on macOS Catalina, which is used by the Actions macOS runners. As described in facebook/watchman#858, creating a hard link to an existing inode from inside the watched Git working tree may not be reported to Git as a new file if the inode's other, existing "source" path (as passed to link(2)) resides outside the watched directory.

This problem is tickled by our functional test suite because the MoveFileFromOutsideRepoToInsideRepoAndAdd() test relies on the System.IO.File.Move() method, which on Unix/POSIX systems like macOS, actually performs a link(2) call rather than rename(2).

While the underlying problem is outside of Scalar's control, we can work around it by using rename(2) directly to move files in the SystemIORunner test helper class.

This particular intermittent but persistent test failure also exposed another, minor issue in the ValidateGitCommand() utility method, which passed its actual and expected string values to the LinesShouldMatch() method in the reverse order from what was intended, so we swap them here.

Debugging these issues also turned up some minor typos in the error messages output from the run clone command when a repository can not be initialized, so those are fixed here also.

Finally, to enable the functional test suite to run in CI on macOS, we add to the Mac version of the RunFunctionalTests.sh script so that like the Linux and Windows versions it copies the Scalar binary into the functional test binary's build location so it can be located by the functional test program.

chrisd8088 commented 4 years ago

Thanks @derrickstolee for the review! It looks like you may need to temporarily disable the required Actions runs so we can merge this, as the PR is waiting on the older no-Watchman Actions jobs and those, of course, aren't running on this PR.