-
-
Notifications
You must be signed in to change notification settings - Fork 99
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
PR: Add other OS and update py versions #102
Conversation
@blink1073 could you add the webhook for the bot to cancel duplicate builds? |
Done! |
.github/workflows/tests.yml
Outdated
- name: Show python environment | ||
run: | | ||
python --version | ||
python -m pip list | ||
- name: Run python tests | ||
run: | | ||
pytest . --tb=long -svv | ||
pytest . --tb=long -svv --cov=jupyterlab_server |
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.
👍 are we gonna send coverage anywhere? If nothing else, i'm a fan of:
--cov-report term:skip-covered --cov-fail-under XX
Where XX is whatever we have now - 1
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.
We could 🙃
@blink1073 do we use codecov or something?
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.
We aren't actively managing coverage in this org iirc but codecov is what I typically use. I can add integration for this repo tomorrow morning.
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.
Thanks!
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.
We did have it set up for pytest-check-links
I added you as a maintainer on this repo @goanpeca and created https://codecov.io/gh/jupyterlab/jupyterlab_server
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.
@bollwyvl you already had team access on the repo
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.
Ok @blink1073 thanks!
name: Linux py${{ matrix.PYTHON_VERSION }} tests | ||
runs-on: ubuntu-latest | ||
name: ${{ matrix.PLATFORM }} py${{ matrix.PYTHON_VERSION }} | ||
runs-on: ${{ matrix.PLATFORM }} | ||
env: | ||
CI: True | ||
PYTHON_VERSION: ${{ matrix.PYTHON_VERSION }} | ||
strategy: | ||
fail-fast: false | ||
matrix: | ||
PYTHON_VERSION: ['3.5', '3.6', '3.8'] |
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.
again, maybe and add 3.9, and drop 3.6?
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 tried and it did not work :-/
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.
Cool/bad! The utter fiasco that was tornado vs 3.8 makes me want to find these things out sooner rather than later. Maybe we can keep it, and just run it against linux, and expect failure? Gotta start somewhere!
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 mean I tried
https://github.com/actions/setup-python
steps:
- uses: actions/checkout@v2
- uses: actions/setup-python@v2
with:
python-version: '3.9.0-beta.4'
And the install of that python did not work.
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.
Maybe try 3.9.0-beta.5
? The manifest is the Source of Truth.
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.
trying this out on #103...
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.
yeah, no dice. weird.
Mea culpa: the windows test timestamp fail is on me (well, and windows' terrible time resolution). Maybe we cheat and round the timestamps before comparing them? I suppose it would still be possible to get occasional fails.... gah, time is why we can't have nice things. Also might be nice to clobber the |
@bollwyvl so this is good to merge and you can continue on the other PR with the win fixes? |
Yeah, it's probably reasonable to introduce a new observed test fail for a PR like this. I do wonder, though, why we're not seeing better cache hits on windows: it's recompiles |
Codecov Report
@@ Coverage Diff @@
## master #102 +/- ##
=========================================
Coverage ? 72.29%
=========================================
Files ? 18
Lines ? 1303
Branches ? 0
=========================================
Hits ? 942
Misses ? 361
Partials ? 0 Continue to review full report at Codecov.
|
Hey, i bet 3.8 is timing because of the tornado ioloop vs Proactor thing. Had to patch that in so many places 😢 |
Yeah, I will skip that one for now, but definitely needs a fix :-p |
I think this is good to go for now @bollwyvl and @blink1073 :-) |
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.
👍🏼
Fixes #97