test: Explain test_was_seen_recently false-positive and give workaround instructions #5474
Conversation
| "{}", | ||
| green.paint( | ||
| "\nNOTE: This test failure is probably a false-positive, caused by tests running in parallel and shifting the time with `SystemTime::shift()`. | ||
| Until the false-positive is fixed: |
There was a problem hiding this comment.
Does not seem like it is going to be fixed, PR adding a lock was turned into PR adding async and then closed without merging: #5450
Also if this is fixed, this assert will likely be forgotten about.
I think this node better goes to readme where there is a chance it will be read.
There was a problem hiding this comment.
Does not seem like it is going to be fixed,
so, cargo test cannot be used any longer? isn't that quite bad and worth targeting? (maybe rust ppl are used to use cargo test param etc., idk, so maybe also ok not to fix :) (but me, as a casual rust dev, was wondering :)
for understanding: the issue is that the test-ultil shift() changes the time for all threads doing tests - and not only for the running test?
I think this node better goes to readme where there is a chance it will be read
but directly in the test output, there is a pretty large chance that it would be read there. i would have seen the hint (but i would not expect sth. about that in the README :)
my 2ct, feel free to ignore :)
There was a problem hiding this comment.
Does not seem like it is going to be fixed
Hard to say, we'll see how annoying it turns out to be.
Also if this is fixed, this assert will likely be forgotten about.
I would assume that if someone fixes it, it's because they saw the assert's error message, and then they will probably also remove the assert. If not, the assert isn't going to harm anyone until someone notices and removes it.
so, cargo test cannot be used any longer? isn't that quite bad and worth targeting?
Idk how bad it is (I personally never run cargo test, always cargo nextest run, for which I created a shell alias), but it turned out to be not that easy to fix.
for understanding: the issue is that the test-ultil shift() changes the time for all threads doing tests - and not only for the running test?
Yes; I'll add this explanation to the output for clarification.
but directly in the test output, there is a pretty large chance that it would be read there. i would have seen the hint (but i would not expect sth. about that in the README :)
+1
In order not to block on what seems to me quite unimportant discussions, I'm going to merge in the hope that @link2xt's comment was not meant to be blocking. I'm happy to create or approve a follow-up PR.
Yes, the problem is that there's no way to pass the current test context to |
…nd instructions (#5474) Until the issue is fixed, keep others from running into the same issue.
Until the issue is fixed, keep others from running into the same issue.