-
Notifications
You must be signed in to change notification settings - Fork 63
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add Watchman-enabled functional tests to GitHub Actions CI #436
Conversation
b1828ae
to
604bbcd
Compare
In commit cbf6f82 in PR microsoft#349 the ValidateGitCommand() helper function was refactored slightly to use a new LinesShouldMatch() function; however, the inputs were accidentally reversed, so here we correct the order of the "expected" and "actual" string arguments passed to LinesShouldMatch().
604bbcd
to
47b43ba
Compare
On macOS and Linux platforms, we use rename(2) directly when moving files with the SystemIORunner's MoveFile() method, instead of using .NET Core's File.Move() method. The latter actually implements most file moves using a combination of link(2) followed by unlink(2) on the source file. The consequence of using link() instead of rename() is that on recent versions of macOS, when running with Watchman, file creation events may not be delivered by Watchman to Git when a new hard link within the Git working tree is created if that link points to an existing inode outside of the watched tree. This in turn causes the MoveFileFromOutsideRepoToInsideRepoAndAdd() test in GitCommandsTests to fail intermittently. We can work around the problem by utilizing rename() directly, which also parallels the behaviour of SystemIORunner's RenameDirectory() method, added in commit 0a517fa. See also: facebook/watchman#858 https://github.com/dotnet/runtime/blob/94515f7bd34aabaab5ba6a1316f4a881cbf2370c/src/libraries/System.IO.FileSystem/src/System/IO/FileSystem.Unix.cs#L142-L144
We replicate for the Mac version of RunFunctionalTests.sh the changes made in commit da2b813 for the Linux version, which permit manual runs of the functional test suite after rebuilding the binaries by copying them into the directory where the Scalar.FunctionalTests binary was published, unless the --test-scalar-on-path option was given to the script.
47b43ba
to
6b86640
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for adding these fixups and getting these builds running!
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. |
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 theSystem.IO.File.Move()
method, which on Unix/POSIX systems like macOS, actually performs alink(2)
call rather thanrename(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 theSystemIORunner
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 theLinesShouldMatch()
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.