Reenable explicit comms tests #770
Conversation
madsbk
added
2 - In Progress
Codecov Report
@@ Coverage Diff @@
## branch-21.12 #770 +/- ##
===============================================
Coverage ? 69.61%
===============================================
Files ? 15
Lines ? 1965
Branches ? 0
===============================================
Hits ? 1368
Misses ? 597
Partials ? 0 Continue to review full report at Codecov.
|
madsbk
added
3 - Ready for Review
| @@ -20,7 +20,7 @@ | |||
| from dask_cuda.utils import get_ucx_config | |||
|
|
|||
| pytestmark = pytest.mark.skipif( | |||
| sys.version_info.minor < 80, | |||
| sys.version_info.minor < 8, | |||
There was a problem hiding this comment.
It seems this is still needed because of dask/distributed#5380, when that gets merged will we be able to remove this skipif?
There was a problem hiding this comment.
Yes, it is still needed. It is just fixing the typo -- we want to run the test on Python v3.8+ not Python v3.80+ :P
There was a problem hiding this comment.
Yes, that makes sense. Will dask/distributed#5380 resolve that, i.e., will we be able to run on Python 3.7 again after it's merged?
There was a problem hiding this comment.
dask/distributed#5380 is in now, are we safe removing the skipif here?
There was a problem hiding this comment.
dask/distributed#5380 is in now, are we safe removing the
skipifhere?
True, but let's merge this to get the type hints and then merge #754
|
I don't think the failing tests are due to this, let's try rerunning. |
|
rerun tests |
|
That’s weird, both this and #771 (comment) PRs segfaulted on the centos7 build, so it's clear it's not an issue with the PR. |
|
rerun tests |
1 similar comment
|
rerun tests |
|
Sorry Peter 😅 |
|
Ha! You're 3 seconds too slow @jakirkham ! 😄 |
|
rerun tests |
|
@gpucibot merge |
The explicit-comms tests are always skipped
sys.version_info.minor < 80, fixed.Also adding some type hints