test: add required testing on Python 3.8.12 #29116
Conversation
|
Oh whoops, the 3.8 tests need #29012. |
Ideally, one day we won't need to do this. We could use this API to determine the current set of required checks. |
Makefile
Outdated
| @@ -108,7 +108,7 @@ test-snuba: | |||
|
|
|||
| backend-typing: | |||
| @echo "--> Running Python typing checks" | |||
| mypy --strict --warn-unreachable --config-file mypy.ini | |||
| mypy --warn-unreachable --config-file mypy.ini | |||
There was a problem hiding this comment.
It overrides everything relevant that's defined in the config file. Like, I set warn_unused_ignores to False, but passing --strict turns it on again.
There was a problem hiding this comment.
Removing --strict removes much more static type safety than simple disabling warn_unused_ignores=True. It would be safer to not test mypy on 3.8 at all than to do this, or simply let the extra check fail.
There was a problem hiding this comment.
since 3.8 is now gone again I suppose we can revert this anyway
There was a problem hiding this comment.
Oh, I just trusted ; Enable all options used with --strict. Why have those options in the config file then?
There was a problem hiding this comment.
Gonna revert all my mypy changes.
There was a problem hiding this comment.
good question but if I run mypy locally I did not get as many errors without --strict in the past. so that comment must be wrong or mean something else...
There was a problem hiding this comment.
it can also be that mypy added more options over time that are enabled by --strict.
| # XXX: We should be using something like len(strategy.matrix.instance) (not possible atm) | ||
| # If you have other things like python-version: [foo, bar, baz] then the sharding logic | ||
| # isn't right because job-total will be 3x larger and you'd never run 2/3 of the tests. | ||
| # MATRIX_INSTANCE_TOTAL: ${{ strategy.job-total }} |
There was a problem hiding this comment.
Ah yeah -- when we were doing 2.7, 3.6 - I opted to make a new workflow file with this being a reason. The other reason being that restarting the acceptance workflow would re-run acceptance tests for both python versions + frontend acceptance tests (when a flake probably wouldn't affect all jobs in the workflow).
There was a problem hiding this comment.
Oh, I could have done that too... given that it's already working + I expect being able to remove 3.6 pretty quickly, I'll just leave it like this if that's ok? The way it is now, it's less prone to introducing something to the matrix and not realizing some sharded tests aren't being run. Plus the comments about the strategy matrix are helpful for the future.
|
re typing: I believe it would be kind of annoying to run mypy twice if it gives conflicting errors. Wouldn't it be easier to run it for 3.8 only? |
Hmm agreed, we'll need to keep the fix I have but I can disable it running on 3.8 and then when we switch over, run it on 3.8 just as I'm doing with visual snapshots. |
This reverts commit 28bb46f.
This reverts commit 1a94b1e.
|
Force merging this as we are changing the job names of acceptance/snuba tests for python 3.6 - they are all passing. |
This shouldn't be merged until we're ready to attempt the initial full deploy, which could break beyond what we can ensure in tests.
Since this introduces a lot of additional CI I'm trying to keep the window of enforcing 3.8 tests passing in addition to 3.6 to just 3.8 tests at a minimum. The ideal situation is we merge this, deploy 3.8 to great success, then remove all of 3.6. But if the 3.8 deploy doesn't go well, we should keep enforcing 3.6 + 3.8 tests passing.
Note, visual snapshotting is disabled on 3.8, and when we make the switch to it I'll enable it in the PR and don't think there'll be any differences.
Gonna need to change repo settings again for these new check titles. And again when 3.6 is removed.