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

Disable the Test_DeleteExecutionsWorkflow_ManyExecutions_NoContinueAsNew test because it is flaky #3699

Merged
merged 1 commit into from
Dec 8, 2022

Conversation

MichaelSnowden
Copy link
Contributor

What changed?
We now skip Test_DeleteExecutionsWorkflow_ManyExecutions_NoContinueAsNew.

Why?
I did this because it is flaky.

How did you test it?

master [$] ➜ go test ./service/worker/deletenamespace/deleteexecutions -race -count 100  -json | jq -s 'map(select(.Action == "fail" and .Test))'
[
  {
    "Time": "2022-12-07T23:19:58.608184-08:00",
    "Action": "fail",
    "Package": "go.temporal.io/server/service/worker/deletenamespace/deleteexecutions",
    "Test": "Test_DeleteExecutionsWorkflow_ManyExecutions_NoContinueAsNew",
    "Elapsed": 0.04
  }
]
master [$] took 10.8s ➜ git checkout flaky-delete-test 
Switched to branch 'flaky-delete-test'
Your branch is up to date with 'origin/flaky-delete-test'.
flaky-delete-test [$] ➜ go test ./service/worker/deletenamespace/deleteexecutions -race -count 100  -json | jq -s 'map(select(.Action == "fail" and .Test))'
[]
flaky-delete-test [$] took 6.3s ➜ 

Potential risks
This could cause us to miss bugs that this test would have otherwise caught.

Is hotfix candidate?
No.

@MichaelSnowden MichaelSnowden requested a review from a team as a code owner December 8, 2022 07:22
@MichaelSnowden
Copy link
Contributor Author

We have an internal task to fix this and re-enable the test.

@MichaelSnowden MichaelSnowden merged commit f0c2e58 into master Dec 8, 2022
@MichaelSnowden MichaelSnowden deleted the flaky-delete-test branch December 8, 2022 19:41
alexshtin added a commit that referenced this pull request May 13, 2024
… test (#5912)

## What changed?
<!-- Describe what has changed in this PR -->
Reenable `Test_DeleteExecutionsWorkflow_ManyExecutions_NoContinueAsNew`
test.

## Why?
<!-- Tell your future self why have you made these changes -->
Test was disabled in #3699 due to race condition. I used `atomic` and it
constatnly passed.

## How did you test it?
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
```
go test ./service/worker/deletenamespace/deleteexecutions -race -count 100  -json | jq -s 'map(select(.Action == "fail" and .Test))'
```
ychebotarev pushed a commit to ychebotarev/temporal that referenced this pull request May 16, 2024
… test (temporalio#5912)

## What changed?
<!-- Describe what has changed in this PR -->
Reenable `Test_DeleteExecutionsWorkflow_ManyExecutions_NoContinueAsNew`
test.

## Why?
<!-- Tell your future self why have you made these changes -->
Test was disabled in temporalio#3699 due to race condition. I used `atomic` and it
constatnly passed.

## How did you test it?
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
```
go test ./service/worker/deletenamespace/deleteexecutions -race -count 100  -json | jq -s 'map(select(.Action == "fail" and .Test))'
```
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