Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
upgrade macos version for support c++11 #156
upgrade macos version for support c++11 #156
Changes from all commits
758f158
9cb796e
e64916f
1f784a1
0d5a2a7
984df80
2473c40
b64b106
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Why do we need this
tmp_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.
to store result of different cibuildwheel call in different directories. It allows check if result is correct. Before this only first test check content of wheelhouse directory, so there is no collision.
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.
But we don't need this in other tests. See for example test
02_test
.Should we then adapt the other tests, or is this change you made not really necessary because
pip wheel
will already build in a separate directory)?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 see that package name from
01_test
and02_test
use same name and same version of package.When I run test on my personal computer I see that, when I use different names I got wheels for all names (you may see that i name projects
spam11
,spam14
andspam17
).So it looks like
02_test
may pass if it silently fail, because01_test
already produce wheels inwheelhouse
.I think that
cibuildwheel
should usetmp_path
fixture or cleanwheelhouse
before testThere 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.
OK, yes, so we should make all the tests follow the same structure! But maybe that's a different PR, then.
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'm working on adding a test for
manylinux2014
and I ran into the same issue. I'll try to make a PR that reworks a bit testing that should answer @Czaki remarks.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 just finishing refactor of test. https://github.com/Czaki/cibuildwheel/tree/test_fix
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.
So is this now OK, or do we need to still fix this to work with the refactoring of the tests we did some weeks ago?