-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
tests: fix edge case where non-default python is used, by skipping it #11005
tests: fix edge case where non-default python is used, by skipping it #11005
Conversation
I discovered this locally running on a self-built python 3.11, and it turns out that one of the tests (not the cython one) is reproducing on the macOS CI jobs too. /cc @xclaesse |
Also a curious note. @carlocab, seems that /usr/local/bin/python3 is 3.11, but (But I found this issue locally when python3 is 3.10, when running the testsuite as |
Codecov Report
@@ Coverage Diff @@
## master #11005 +/- ##
===========================================
- Coverage 68.29% 50.44% -17.86%
===========================================
Files 412 207 -205
Lines 87861 44929 -42932
Branches 20728 9927 -10801
===========================================
- Hits 60007 22663 -37344
+ Misses 23354 20226 -3128
+ Partials 4500 2040 -2460
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
@carlocab alright, bigger issue is genuinely an issue.
And attempting to run the codecov tool as the final stage of the CI run fails because command not found. Also I guess the cython job only doesn't reproduce on macOS because it currently isn't found, so it gets skipped. :D ... Why is the bin directory for pip install not in PATH? We could add it, but I don't really want to hardcode paths. Don't really want to pin the version either. |
87d5d58
to
1bcc711
Compare
Alright, the python macOS issue is silly, as it turns out the base image scribbled all over homebrew despite literally installing homebrew python on its own. See Homebrew/discussions#3895 Should be fixed by having homebrew force relink itself. |
1bcc711
to
7ea3f1a
Compare
LGTM if ci is green now. |
Well, it worked a week ago. :D Now llvm is forcing python3 to 3.11, so the brew install fails before link --force can run. Hold on, reordering... |
I'll merge this once the test goes green. |
7ea3f1a
to
ceaf991
Compare
@carlocab is there some future-proof way to say "install whichever version of python is drawn in by llvm, and --force it, but don't --force any other packages"? If not, I can always manually specify |
😢 |
Yep. Just waiting on an idea of the cleanest way to do this before committing and pushing. GitHub's CI images are full of "wat" right now. |
67fdda3
to
9f8f06c
Compare
In a couple of python module tests, we try to test things that rely on the default python being the same one we look up in the python module. This is unsolvable for the deprecated python3 module, as it actually uses the in-process version of python for everything. For the python module, we could actually look up the default system python instead of the one we are running with, but then we wouldn't be testing the functionality of that alternative python... and also the install manifest tests would see files installed for the wrong version of python, and report a combination of missing+extra files... Solve both tests by just skipping the parts we cannot check.
The default actions one is broken in two ways, and additionally overwrote homebrew's symlinks to begin with.
9f8f06c
to
0e5d632
Compare
Something like
might do what you want here. |
run: | | ||
find /usr/local/bin -lname '*/Library/Frameworks/Python.framework/*' -delete | ||
sudo rm -rf /Library/Frameworks/Python.framework/ | ||
brew install --force python3 && brew unlink python3 && brew unlink python3 && brew link --overwrite python3 |
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.
It might suffice to just do
brew install --force --overwrite python3
here.
In a couple of python module tests, we try to test things that rely on the default python being the same one we look up in the python module. This is unsolvable for the deprecated python3 module, as it actually uses the in-process version of python for everything. For the python module, we could actually look up the default system python instead of the one we are running with, but then we wouldn't be testing the functionality of that alternative python... and also the install manifest tests would see files installed for the wrong version of python, and report a combination of missing+extra files...
Solve both tests by just skipping the parts we cannot check.