Skip to content

explain time-with-isolation test better #3379

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

Merged
merged 1 commit into from
Mar 14, 2024

Conversation

RalfJung
Copy link
Member

Fixes #3377

@saethlin do you think this is better?

@RalfJung RalfJung force-pushed the time-with-isolation branch from f1d8f15 to 31f2ecd Compare March 14, 2024 07:19
@@ -0,0 +1,2 @@
The loop took around 7s
(It's fine for this number to change a bit when you `--bless` this test.)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How much is "a bit"? The recent rust-lang/rust PR changed this number from 7 to 12 which is quite a large swing normally, but per the comment in the source code within an order of magnitude is fine.

So probably we should say "within an order of magnitude" here if that is our expectation.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can also just say "it is okay to change"; we'll get pinged anyway when it happens so we can intervene if something seems to go terribly wrong.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works for me!

@RalfJung RalfJung force-pushed the time-with-isolation branch from 31f2ecd to f0c8848 Compare March 14, 2024 14:30
@saethlin
Copy link
Member

r=me

@RalfJung
Copy link
Member Author

@bors r=saethlin

(Btw, you should have "r+" powers here. :)

@bors
Copy link
Contributor

bors commented Mar 14, 2024

📌 Commit f0c8848 has been approved by saethlin

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Mar 14, 2024

⌛ Testing commit f0c8848 with merge 1fd2315...

@bors
Copy link
Contributor

bors commented Mar 14, 2024

☀️ Test successful - checks-actions
Approved by: saethlin
Pushing 1fd2315 to master...

@bors bors merged commit 1fd2315 into rust-lang:master Mar 14, 2024
8 checks passed
@RalfJung RalfJung deleted the time-with-isolation branch March 14, 2024 19:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

time-with-isolation test is drifting and often confuses people
3 participants