Skip to content

Fix sporadic fails in timer unit-tests on Windows#10776

Merged
jschmidt-icinga merged 2 commits intomasterfrom
give-windows-more-time
Mar 31, 2026
Merged

Fix sporadic fails in timer unit-tests on Windows#10776
jschmidt-icinga merged 2 commits intomasterfrom
give-windows-more-time

Conversation

@jschmidt-icinga
Copy link
Copy Markdown
Contributor

Fix the test-cases base_timer/invoke and base_timer/scope still sporadically failing on Windows, even after increasing the leeway given to the timers in #10743. Essentially, this gives tests 10x the time on Windows, effectively restoring the timings to the state before #10550, but only on Windows.

@cla-bot cla-bot bot added the cla/signed label Mar 30, 2026
@julianbrost julianbrost added this to the 2.16.0 milestone Mar 30, 2026
@julianbrost
Copy link
Copy Markdown
Member

Windows tests failing here, how ironic. However, that's not related to the change:

Failed to fetch results from V2 feed at 'https://community.chocolatey.org/api/v2/Packages(Id='winflexbison3',Version='2.5.24.20210105')' with following message : Response status code does not indicate success: 503 (Service Unavailable).

Let's try that again, I've restarted the pipeline.

@yhabteab
Copy link
Copy Markdown
Member

Why the scientific notation though, instead of just using 1/10?

even after increasing the leeway given to the timers in #10743.

Wouldn't it make sense to revert that commit now then, since it didn't fix the issue it's supposed to fix?

@jschmidt-icinga
Copy link
Copy Markdown
Contributor Author

jschmidt-icinga commented Mar 30, 2026

Wouldn't it make sense to revert that commit now then, since it didn't fix the issue it's supposed to fix?

This PR essentially does that (values reverted to 0.55), just not in a separate commit. Do you want the separate revert commit for a clearer history or did you just not see that I changed the value back?

Why the scientific notation though, instead of just using 1/10?

I subjectively prefer it for literals of 1, 10, etc. since it seems pointless to write out a zero decimal. I can change it to 1.0 etc. but it really doesn't matter IMHO.

@yhabteab
Copy link
Copy Markdown
Member

Do you want the separate revert commit for a clearer history

Yes, I expected a sperate commit generated with git revert 1430e63e727dd2dddadaa84aec31ea183e9cb82d.

Why the scientific notation though, instead of just using 1/10?

I subjectively prefer it for literals of 1, 10, etc. since it seems pointless to write out a zero decimal. I can change it to 1.0 etc. but it really doesn't matter IMHO.

It does matter, because it requires the reader to understand scientific notation to decipher that 1e0 is the same as 1, and so on. I would personally say it is pointless to use scientific notation for such trivial numbers and not the other way around, which even requires you to write more characters (10e0 instead of just 10).

@jschmidt-icinga jschmidt-icinga force-pushed the give-windows-more-time branch from fa40f01 to 82b244c Compare March 31, 2026 07:30
@jschmidt-icinga jschmidt-icinga force-pushed the give-windows-more-time branch from 82b244c to 72ebd43 Compare March 31, 2026 07:33
@jschmidt-icinga
Copy link
Copy Markdown
Contributor Author

  • Added the revert as a separate commit.
  • Changed the constexprs to be declared as doubles initialized with integers 1 and 10.

Copy link
Copy Markdown
Member

@yhabteab yhabteab left a comment

Choose a reason for hiding this comment

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

If this really fixes the issue, then I'm fine with it.

@julianbrost
Copy link
Copy Markdown
Member

If this really fixes the issue, then I'm fine with it.

I'd expect it to due to the following:

effectively restoring the timings to the state before #10550, but only on Windows

And I think we only started noticing these somewhat regular test failures after that change.

@jschmidt-icinga jschmidt-icinga merged commit 52dcf91 into master Mar 31, 2026
28 checks passed
@jschmidt-icinga jschmidt-icinga deleted the give-windows-more-time branch March 31, 2026 10:30
@yhabteab yhabteab mentioned this pull request Apr 1, 2026
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants