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

Add retry to tests #5968

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

CharlieTLe
Copy link
Member

What this PR does:
Adds retry to tests. This should improve the reliability of the tests.

Which issue(s) this PR fixes:
Fixes #

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

Signed-off-by: Charlie Le <[email protected]>
@CharlieTLe
Copy link
Member Author

Example retry: https://github.com/cortexproject/cortex/actions/runs/9196226564/job/25293837536?pr=5968

  for run in $(seq 1 3); do
    make BUILD_IN_CONTAINER=false test && break
    echo "Retrying tests... Run $run failed."
  done
  shell: sh -e {0}
go test -tags netgo -timeout 30m -race -failfast -count 1 ./...
?   	github.com/cortexproject/cortex/cmd/test-exporter	[no test files]
...
--- FAIL: TestCompactor_ShouldCompactOnlyShardsOwnedByTheInstanceOnShardingEnabledWithShuffleShardingAndMultipleInstancesRunning (19.70s)
    compactor_test.go:1442: 
        	Error Trace:	/__w/cortex/cortex/pkg/compactor/compactor_test.go:1442
        	Error:      	Received unexpected error:
        	            	group with group_hash=106465857 owned by multiple compactors
        	Test:       	TestCompactor_ShouldCompactOnlyShardsOwnedByTheInstanceOnShardingEnabledWithShuffleShardingAndMultipleInstancesRunning
FAIL
FAIL	github.com/cortexproject/cortex/pkg/compactor	37.803s
ok  	github.com/cortexproject/cortex/pkg/configs/api	1.130s
...
--- FAIL: TestSlottedTicker (2.08s)
    --- FAIL: TestSlottedTicker/Get_last_slot (2.08s)
        time_test.go:234: expected true, got false
FAIL
FAIL	github.com/cortexproject/cortex/pkg/util	26.946s
...
ok  	github.com/cortexproject/cortex/tools/querytee	1.085s
ok  	github.com/cortexproject/cortex/tools/thanosconvert	1.161s
FAIL
make: *** [Makefile:226: test] Error 1
Retrying tests... Run 1 failed.
go test -tags netgo -timeout 30m -race -failfast -count 1 ./...
?   	github.com/cortexproject/cortex/cmd/test-exporter	[no test files]
...
ok  	github.com/cortexproject/cortex/tools/thanosconvert	1.183s

@alanprot
Copy link
Member

Im ok with this i guess

Copy link
Member

@friedrichg friedrichg left a comment

Choose a reason for hiding this comment

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

Looks good at first, but at the same time won't this increment the flaky tests?

@friedrichg friedrichg requested a review from yeya24 May 23, 2024 16:39
@alanprot
Copy link
Member

I tried to fix some flaky tests today.

yeya24
yeya24 previously approved these changes May 23, 2024
Copy link
Contributor

@yeya24 yeya24 left a comment

Choose a reason for hiding this comment

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

Thanks!

@yeya24 yeya24 self-requested a review May 23, 2024 22:07
@yeya24 yeya24 dismissed their stale review May 23, 2024 22:07

revoke

@yeya24
Copy link
Contributor

yeya24 commented May 23, 2024

Actually, I am a little bit concerned. If the test doesn't pass because the code has bug, it will still retry 3 times?
I don't know if it is what we want and we will spend more time on CI.

@alanprot
Copy link
Member

Yeah.. lets hold this for now and try to make the tests more stable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants