-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Don't attempt playTests.sh cmake test if running on Windows. #3289
Conversation
It seems there is already a property flag to decide running Also, I'm not sure if "Windows" is a good enough discriminant for such a decision. Finally, maybe we need a CI test to ensure this property is running as intended and continues to run as intended in the future, when future contributors change some part of the code base. |
Are you referring to As it currently sits this cmake tests would fail to run on Windows, using |
What about:
|
I don't think that is it... I'm not sure how I would enable/disable that. I could do a check for find_program(UNAME uname)
if (UNAME)
// Must be shell terminal
else()
// Must not be
endif() |
|
Hi @nmoinvaz! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks! |
Signed. |
Added check for uname. |
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
There is another problem, that playTest.sh is triggered when the project is added with
The problem is that |
Also is it weird that |
I fixed the logic around the uname detection, but don't have a solution for detecting if datagen will be built. |
Looks like using
|
Does it matter if |
Yes, it does matter because
Correct. Now this PR makes it so that |
Thanks for the PR, it LGTM! |
Disable this test on Windows because it runs a bash script. I'm not sure of a way to easily determine if the shell is bash, so I have just disabled it on Windows.