-
Notifications
You must be signed in to change notification settings - Fork 66
Actions: run functional tests on all platforms (without Watchman) #431
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
Conversation
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.
Here, the proof is in the pudding! The tests run, I can see their output. I manually checked that the test output does not contain "Failed at" which would happen if a failure occurred.
It might be good to purposefully break a test to make sure the build goes red. Just a thought.
Fix the nit and you are good to merge!
run: | | ||
echo "::set-env name=BUILD_PLATFORM::${{ runner.os }}" | ||
echo '::set-env name=BUILD_FILE_EXT::.exe' |
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.
I had trouble getting this to work on Windows in GCM Core. Making a note to see why this is different from my attempt.
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.
I futzed around a lot with these Actions workflow commands and while I've forgotten some of the details, it was not as straightforward as I might have naively hoped. I recall wanting to do something more elaborate but being unable to get it to work.
dotnet new classlib -n Scalar.GitInstaller | ||
cd Scalar.GitInstaller | ||
cp ../../scalar/nuget.config . | ||
dotnet add Scalar.GitInstaller.csproj package "GitFor${BUILD_PLATFORM}.GVFS.Installer" --package-directory . --version "$GIT_VERSION" |
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.
I think the difference is how you use ${ }
here. Interesting.
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.
Could be ... I was able to use a plain $BUILD_FILE_EXT
below, though, so I don't know.
One thing I can definitely attest to is that if the functional test suite doesn't pass, the CI status goes red. That happened too many times to count, so I'm positive it works as expected, at least in that regard! 😄 |
a4c0767
to
2bc0374
Compare
To run the functional test suite in GitHub Actions, we write platform-specific steps to install the GVFS Git package, but then try to use a single common step to run the functional tests themselves. The commands in the Git installer steps are drawn in part from those found in the existing Scalar.Installer.{Mac,Windows}/InstallScalar.template.{sh,bat} scripts. On Windows, we specifically need to ensure that our version of Git is the one in "C:\Program Files\Git\cmd\git.exe", so we start install process by uninstalling the GitForWindows version provided on the Actions instance. For future debugging purposes we also include a final Get-ItemProperty PowerShell query to report whether we successfully updated the GitForWindows registry entry. We also disable the fast-fail option on our matrix builds so all platforms will run to completion, allowing us to see any mix of platform-specific errors without short-circuting due to the first CI failure on any platform. Note that we do not yet install Watchman or provide additional functional test runs using it; that work remains to be done.
2bc0374
to
a01d852
Compare
To run the functional test suite in GitHub Actions, we write platform-specific steps to install the GVFS Git package, but then use a single common step to run the functional tests themselves.
The Actions YAML steps added in this PR resulted in a recent successful run on all platforms.
This PR builds on #422, which adds functional test support on Linux, and can be rebased once that PR is accepted and merged into
main
.The commands in the Git installer steps are drawn in part from those found in the existing
Scalar.Installer.{Mac,Windows}/InstallScalar.template.{sh,bat}
scripts.On Windows, we specifically need to ensure that our version of Git is the one in
C:\Program Files\Git\cmd\git.exe
, so we start install process by uninstalling the GitForWindows version provided on the Actions instance. For future debugging purposes we also include a finalGet-ItemProperty
PowerShell query to report whether we successfully updated the GitForWindows registry entry.We also disable the fast-fail option on our matrix builds so all platforms will run to completion, allowing us to see any mix of platform-specific errors without short-circuting due to the first CI failure on any platform.
Note that we do not yet install Watchman or provide additional functional test runs using it; that work remains to be done.