Skip to content
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

test: GitHub Actions add Windows to the testing #1996

Merged
merged 3 commits into from
Dec 29, 2019

Conversation

cclauss
Copy link
Contributor

@cclauss cclauss commented Dec 16, 2019

@joaocgreis This might be of interest to you.

Checklist
  • npm install && npm test passes
  • tests are included
  • documentation is changed or added
  • commit message follows commit guidelines
Description of change

Copy link
Member

@gengjiawen gengjiawen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Windows test still failed

@cclauss
Copy link
Contributor Author

cclauss commented Dec 18, 2019

Yes. I am not a Windows guy and I do not understand how to fix test_BinaryNamesWindows()
https://github.com/nodejs/node-gyp/pull/1996/checks?check_run_id=350074778#step:6:23

@joaocgreis
Copy link
Member

@cclauss isn't this similar to what happened in Travis, the VS version is more recent than the one supported by our gyp version? If this is the case, a good way to fix is to set the MSVS variables similarly to Travis:

- GYP_MSVS_VERSION=2015 GYP_MSVS_OVERRIDE_PATH="C:\\Dummy" python -m pytest

@cclauss
Copy link
Contributor Author

cclauss commented Dec 29, 2019

@joaocgreis Using shell: bash along with your suggestion enables the Windows tests to pass. Thanks much!

@gengjiawen gengjiawen merged commit 31ecc84 into master Dec 29, 2019
@gengjiawen gengjiawen deleted the GitHub-Actions-add-Windows branch December 29, 2019 07:43
rvagg pushed a commit that referenced this pull request Jan 3, 2020
PR-URL: #1996
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants