-
Notifications
You must be signed in to change notification settings - Fork 347
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
Run unit tests on Windows too #1099
Conversation
612ac0e
to
8f89ee5
Compare
c6893e6
to
76ea1ff
Compare
76ea1ff
to
abb2f8a
Compare
This decorator enabled us to use the functionality of the Actions toolcache within the runner too. Now that we've deleted the runner we no longer need it.
41e985e
to
9eed356
Compare
9eed356
to
a73b38a
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.
This looks sensible. I just have a few questions inline so I can understand what is going on.
// Workaround an issue in tests where the case insensitivity of the `$PATH` | ||
// environment variable on Windows isn't preserved, i.e. `process.env.PATH` | ||
// is not the same as `process.env.Path`. |
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.
That's wild. When does this happen?
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 spent a while looking into this, but couldn't track down what was going on. My suspicion is that this weirdness might have something to do with how we use an assignment to process.env
to reset it at the end of every test to its value at the start of the test.
`toolcache.extractTar` currently falls over when `ACTIONS_TEMP` contains a symlink, and the runner no longer exists, so it's unlikely our customers would be running with temporary directories that contain symlinks.
a73b38a
to
130a51d
Compare
9953936
to
b1742f8
Compare
This PR runs our unit tests on Windows, primarily to improve test coverage but also to support developing the Action on Windows machines. In doing so, we fix a couple of bugs.
For review: I spent a while looking at whether we could avoid the ugly hack in 6063828, however couldn't find anything. Do you have any thoughts?
Based on #1104.
Merge / deployment checklist