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

Reduce flakiness of tests #3596

Closed
wants to merge 2 commits into from
Closed

Reduce flakiness of tests #3596

wants to merge 2 commits into from

Conversation

newhoggy
Copy link
Contributor

No description provided.

@@ -189,7 +190,7 @@ jobs:
if: ${{ always() }}
continue-on-error: true
with:
name: chairman-test-artifacts-${{ matrix.os }}-${{ matrix.ghc }}
name: chairman-test-artifacts-${{ matrix.os }}-${{ matrix.n }}-${{ matrix.ghc }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is needed so that when there is more than one n, the logs don't clobber each other.

@newhoggy newhoggy force-pushed the newhoggy/less-flaky-tests branch 5 times, most recently from 282e602 to 68e2d98 Compare February 15, 2022 07:06
* Retry testnet tests when they fail
* Retry parts of complex tests when they fail
* Reduce number of BFT nodes
* Set BFT nodes to 1
* Update cardano-crypto dependency
@newhoggy newhoggy force-pushed the newhoggy/less-flaky-tests branch from 86100a0 to 4f2f1a5 Compare February 16, 2022 13:16
@newhoggy newhoggy marked this pull request as ready for review February 17, 2022 10:27
@newhoggy newhoggy changed the title Retry testnet tests when they fail Reduce flakiness of tests Feb 17, 2022
Copy link
Contributor

@Jimbo4350 Jimbo4350 left a comment

Choose a reason for hiding this comment

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

Nice work 👍. Just a couple of questions.

@@ -25,6 +25,7 @@ jobs:
strategy:
fail-fast: false
matrix:
n: [1] # Add more elements here to run additional copies of tests
Copy link
Contributor

Choose a reason for hiding this comment

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

What does n do here?

if currEpoch == desiredEpoch
Just currEpoch -> do

if currEpoch >= desiredEpoch
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

import GHC.Stack (callStack)
import qualified System.Directory as IO
import Hedgehog (Property, (/==), (===))
Copy link
Contributor

Choose a reason for hiding this comment

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

Why all the module import rearrangement?

@@ -571,7 +558,7 @@ hprop_plutus_certifying_withdrawing = H.integration . H.runFinallies . H.workspa

utxoPlutus /== mempty

H.note_ "Wait for rewards to be paid out. This will be current epoch + 4"
H.note_ "Wait for rewards to be paid out. This will be current epoch + 5"
Copy link
Contributor

Choose a reason for hiding this comment

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

Where did you read it was + 5?

H.note_ "Check we have reached 4 epochs ahead"
waitedEpoch === rewardsEpoch

H.note_ "Check we have reached 5 epochs ahead"
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

@@ -611,7 +611,7 @@ hprop_plutus_certifying_withdrawing = H.integration . H.runFinallies . H.workspa

let minrequtxo = 999978

pr <- H.byDurationM 3 60 $ do
pr <- H.noteShowM . H.byDurationM 3 60 $ do
Copy link
Contributor

Choose a reason for hiding this comment

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

We can squash this commit

@newhoggy newhoggy mentioned this pull request Oct 28, 2022
@newhoggy
Copy link
Contributor Author

This PR is replaced by #4575

@newhoggy newhoggy closed this Oct 28, 2022
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