Assert otherwise pointless comparisons (B015)
#6811
Conversation
Unit Test ResultsSee test report for an extended history of previous test failures. This is useful for diagnosing flaky tests. 15 files + 1 15 suites +1 6h 44m 23s ⏱️ + 46m 48s For more details on these failures, see this check. Results for commit b06f421. ± Comparison against base commit 192a8bb. ♻️ This comment has been updated with latest results. |
| """That would be the default in user code""" | ||
| server = Server({}) | ||
| server.status == Status.running | ||
| server.status == Status.running # noqa: B015 |
There was a problem hiding this comment.
Why ignore here instead of adding an assert?
There was a problem hiding this comment.
Based on my understanding of this test, i.e. its name and history (#3875), I think that all we want to do is test that server.status == Status.running doesn't cause an exception or error, not so much test that the result is True/False. If it helps, I'll extend the docstring.
There was a problem hiding this comment.
A comment would be helpful here I think
I haven't had the time yet to dive into this, but that's what I'm thinking as well. I'll do some digging to understand what the desired and expected behavior should be here. |
| @pytest.mark.parametrize("interval, expected", [(None, 100), ("500ms", 500), (2, 2)]) | ||
| @gen_cluster(nthreads=[]) | ||
| @gen_cluster(nthreads=[], config={"distributed.scheduler.work-stealing": False}) | ||
| async def test_parse_stealing_interval(s, interval, expected): |
There was a problem hiding this comment.
I suspect the new implementation is what this test is supposed to be doing. This does not take into account some weirdness around how WorkStealing is added as a plugin in the first place, but that's out of scope.
cc @fjetter
hendrikmakait
requested review from
crusaderky,
fjetter,
gjoseph92 and
graingert
gjoseph92
left a comment
gjoseph92
left a comment
There was a problem hiding this comment.
This LGTM, assuming the work-stealing test is fixed?
| """That would be the default in user code""" | ||
| server = Server({}) | ||
| server.status == Status.running | ||
| server.status == Status.running # noqa: B015 |
There was a problem hiding this comment.
A comment would be helpful here I think
hendrikmakait
force-pushed
the
fix-pointless-comparisons
branch
from
366b032 to
f6c0529
Compare
hendrikmakait
force-pushed
the
fix-pointless-comparisons
branch
from
be85e6f to
b06f421
Compare
|
Thank you |
4 participants
Partially addresses pre-commit failures in #6809
pre-commit run --all-files