Skip to content
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

Fix flaky retry-timer test #158

Merged
merged 2 commits into from
Oct 30, 2023
Merged

Fix flaky retry-timer test #158

merged 2 commits into from
Oct 30, 2023

Conversation

HDegroote
Copy link
Contributor

@HDegroote HDegroote commented Oct 30, 2023

The 'retry timer - proven peer reinsertion' test is flaky, I think because there's 0 margin built-in. I added an additional check to verify that the retry was triggered once after the first wait, and noticed that it rarely was, so I added some margin.

I'm not 100% sure that the changes I made make sense though, so do review.

This is an example output for a failing run of the old test:

# retry timer - proven peer reinsertion
    not ok 1 - should be equal
      ---
      actual: 1
      expected: 2
      operator: is
      source: |
        
          t.is(calls, 2)
        ----^
        
          rt.destroy()
      stack: |
        ./test/retry-timer.js:37:5
        runNextTicks (node:internal/process/task_queues:60:5)
        process.processTimers (node:internal/timers:511:9)
        async Test._run (./node_modules/brittle/index.js:573:7)
` ` 

@HDegroote HDegroote marked this pull request as ready for review October 30, 2023 16:44
@mafintosh mafintosh merged commit bf1e34a into main Oct 30, 2023
3 checks passed
@mafintosh mafintosh deleted the unflaky-retry-test branch October 30, 2023 16:46
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.

2 participants