Skip to content

fix: resolve remaining race condition in TestWaitForState flaky test#7635

Merged
wozniakjan merged 15 commits into
kedacore:mainfrom
RokeshVS:fix/flaky-testwait-for-state-7542
Apr 13, 2026
Merged

fix: resolve remaining race condition in TestWaitForState flaky test#7635
wozniakjan merged 15 commits into
kedacore:mainfrom
RokeshVS:fix/flaky-testwait-for-state-7542

Conversation

@RokeshVS

@RokeshVS RokeshVS commented Apr 9, 2026 •
edited by rickbrouwer
Loading

Copy link
Copy Markdown
Contributor

Fixes the remaining race condition in TestWaitForState in pkg/scalers/external_scaler_test.go that was not resolved by #7547.

Problem

TestWaitForState was still flaky under -race due to two issues:

  • grpcClient starts in Idle state after grpc.NewClient(), causing waitForState to miss state transitions
  • context.TODO() in the goroutine had no cancellation, so t.Log was called after the test completed → panic

Fix

  • Call grpcClient.Connect() to force the client out of Idle before watching state
  • Introduce waitCtx with defer waitCancel() so the goroutine exits cleanly when the test ends
  • Call grpcClient.Close() explicitly in the main body after GracefulStop() to drive a deterministic Shutdown transition

Test Results

Before Fix

go test ./pkg/scalers/ -run TestWaitForState -v -count=10 -race
=== RUN   TestWaitForState
    external_scaler_test.go:268: close state: SHUTDOWN
--- PASS: TestWaitForState (5.01s)
=== RUN   TestWaitForState
    external_scaler_test.go:268: close state: SHUTDOWN
--- PASS: TestWaitForState (5.00s)
=== RUN   TestWaitForState
    external_scaler_test.go:268: close state: SHUTDOWN
--- PASS: TestWaitForState (5.01s)
=== RUN   TestWaitForState
    external_scaler_test.go:268: close state: SHUTDOWN
--- PASS: TestWaitForState (5.01s)
=== RUN   TestWaitForState
    external_scaler_test.go:268: close state: SHUTDOWN
--- PASS: TestWaitForState (5.01s)
=== RUN   TestWaitForState
    external_scaler_test.go:268: close state: SHUTDOWN
--- PASS: TestWaitForState (5.00s)
=== RUN   TestWaitForState
    external_scaler_test.go:268: close state: SHUTDOWN
--- PASS: TestWaitForState (5.01s)
=== RUN   TestWaitForState
    external_scaler_test.go:301: waitForState should be get connectivity.Shutdown.
--- FAIL: TestWaitForState (10.01s)
=== RUN   TestWaitForState
    external_scaler_test.go:268: close state: SHUTDOWN
--- PASS: TestWaitForState (5.00s)
=== RUN   TestWaitForState
panic: Log in goroutine after TestWaitForState has completed: close state: SHUTDOWN

goroutine 281 [running]:
testing.(*common).log(0xc000dfa540, {0xc000c9e060, 0x16})
        /usr/local/go/src/testing/testing.go:1030 +0x1df
testing.(*common).Log(0xc000dfa540, {0xc000c13f98, 0x2, 0x2})
        /usr/local/go/src/testing/testing.go:1180 +0x77
github.com/kedacore/keda/v2/pkg/scalers.TestWaitForState.func3()
        /workspaces/keda/pkg/scalers/external_scaler_test.go:268 +0x165
created by github.com/kedacore/keda/v2/pkg/scalers.TestWaitForState in goroutine 275
        /workspaces/keda/pkg/scalers/external_scaler_test.go:263 +0x8fd
FAIL    github.com/kedacore/keda/v2/pkg/scalers 55.202s
FAIL

After Fix

go test ./pkg/scalers/ -run TestWaitForState -v -count=20 -race
=== RUN   TestWaitForState
    external_scaler_test.go:273: close state: SHUTDOWN
--- PASS: TestWaitForState (5.01s)
=== RUN   TestWaitForState
    external_scaler_test.go:273: close state: SHUTDOWN
--- PASS: TestWaitForState (5.01s)
=== RUN   TestWaitForState
    external_scaler_test.go:273: close state: SHUTDOWN
--- PASS: TestWaitForState (5.01s)
=== RUN   TestWaitForState
    external_scaler_test.go:273: close state: SHUTDOWN
--- PASS: TestWaitForState (5.01s)
=== RUN   TestWaitForState
    external_scaler_test.go:273: close state: SHUTDOWN
--- PASS: TestWaitForState (5.01s)
=== RUN   TestWaitForState
    external_scaler_test.go:273: close state: SHUTDOWN
--- PASS: TestWaitForState (5.01s)
=== RUN   TestWaitForState
    external_scaler_test.go:273: close state: SHUTDOWN
--- PASS: TestWaitForState (5.01s)
=== RUN   TestWaitForState
    external_scaler_test.go:273: close state: SHUTDOWN
--- PASS: TestWaitForState (5.01s)
=== RUN   TestWaitForState
    external_scaler_test.go:273: close state: SHUTDOWN
--- PASS: TestWaitForState (5.01s)
=== RUN   TestWaitForState
    external_scaler_test.go:273: close state: SHUTDOWN
--- PASS: TestWaitForState (5.01s)
=== RUN   TestWaitForState
    external_scaler_test.go:273: close state: SHUTDOWN
--- PASS: TestWaitForState (5.00s)
=== RUN   TestWaitForState
    external_scaler_test.go:273: close state: SHUTDOWN
--- PASS: TestWaitForState (5.01s)
=== RUN   TestWaitForState
    external_scaler_test.go:273: close state: SHUTDOWN
--- PASS: TestWaitForState (5.01s)
=== RUN   TestWaitForState
    external_scaler_test.go:273: close state: SHUTDOWN
--- PASS: TestWaitForState (5.01s)
=== RUN   TestWaitForState
    external_scaler_test.go:273: close state: SHUTDOWN
--- PASS: TestWaitForState (5.01s)
=== RUN   TestWaitForState
    external_scaler_test.go:273: close state: SHUTDOWN
--- PASS: TestWaitForState (5.01s)
=== RUN   TestWaitForState
    external_scaler_test.go:273: close state: SHUTDOWN
--- PASS: TestWaitForState (5.01s)
=== RUN   TestWaitForState
    external_scaler_test.go:273: close state: SHUTDOWN
--- PASS: TestWaitForState (5.01s)
=== RUN   TestWaitForState
    external_scaler_test.go:273: close state: SHUTDOWN
--- PASS: TestWaitForState (5.01s)
=== RUN   TestWaitForState
    external_scaler_test.go:273: close state: SHUTDOWN
--- PASS: TestWaitForState (5.01s)
PASS
ok      github.com/kedacore/keda/v2/pkg/scalers 101.336s

Checklist

Fixes #7542

RokeshVS and others added 11 commits March 16, 2026 02:24
@RokeshVS
fix: correct context cancellation in waitForState to fix flaky test
48aff47 
Signed-off-by: RokeshVS <[email protected]>
@RokeshVS
fix: update changelog entry per maintainer feedback
094608d 
Signed-off-by: RokeshVS <[email protected]>
@RokeshVS @rickbrouwer
Update CHANGELOG.md
d98eb1f 
Co-authored-by: Rick Brouwer <[email protected]>
Signed-off-by: RokeshVS <[email protected]>
@RokeshVS
fix: update changelog by alphabetical order as per maintainer feedback
cdfa537 
Signed-off-by: RokeshVS <[email protected]>
@RokeshVS @rickbrouwer
Update CHANGELOG.md
0737d3f 
Co-authored-by: Rick Brouwer <[email protected]>
Signed-off-by: RokeshVS <[email protected]>
@rickbrouwer
Update CHANGELOG.md
bc71bd8 
Signed-off-by: Rick Brouwer <[email protected]>
@rickbrouwer
Update CHANGELOG.md
25de80e 
Signed-off-by: Rick Brouwer <[email protected]>
@RokeshVS
Merge branch 'kedacore:main' into fix/flaky-testwait-for-state-7542
ef2233d
@RokeshVS
fix: resolve race condition in TestWaitForState with context and expl…
6bdadc4 
…icit Connect

Signed-off-by: RokeshVS <[email protected]>
@RokeshVS
Update Changelog
c287d05 
Signed-off-by: RokeshVS <[email protected]>
@RokeshVS
Update Changelog
77caf95 
Signed-off-by: RokeshVS <[email protected]>
@RokeshVS RokeshVS requested a review from a team as a code owner April 9, 2026 18:37
@snyk-io

snyk-io Bot commented Apr 9, 2026 •
edited
Loading

Copy link
Copy Markdown

✅ Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
✅ Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@github-actions

github-actions Bot commented Apr 9, 2026

Copy link
Copy Markdown

Thank you for your contribution! 🙏

Please understand that we will do our best to review your PR and give you feedback as soon as possible, but please bear with us if it takes a little longer as expected.

While you are waiting, make sure to:

  • Add an entry in our changelog in alphabetical order and link related issue
  • Update the documentation, if needed
  • Add unit & e2e tests for your changes
  • GitHub checks are passing
  • Is the DCO check failing? Here is how you can fix DCO issues

Once the initial tests are successful, a KEDA member will ensure that the e2e tests are run. Once the e2e tests have been successfully completed, the PR may be merged at a later date. Please be patient.

Learn more about our contribution guide.

@keda-automation keda-automation requested a review from a team April 9, 2026 18:37
@RokeshVS RokeshVS mentioned this pull request Apr 9, 2026
Merged
2 tasks
rickbrouwer
rickbrouwer approved these changes Apr 9, 2026
View reviewed changes

@rickbrouwer rickbrouwer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the follow-up fix!

@rickbrouwer rickbrouwer added the Awaiting/2nd-approval This PR needs one more approval review label Apr 9, 2026
zroubalik
zroubalik previously requested changes Apr 10, 2026
View reviewed changes
Comment thread CHANGELOG.md Outdated
@zroubalik zroubalik added the waiting-author-response All PR's or Issues where we are waiting for a response from the author label Apr 10, 2026
@keda-automation keda-automation requested a review from a team April 10, 2026 16:32
RokeshVS added 2 commits April 10, 2026 22:12
@RokeshVS
Update fix in order
2164d37 
Signed-off-by: RokeshVS <[email protected]>
@RokeshVS
Update CHANGELOG.md
3a62100 
Signed-off-by: RokeshVS <[email protected]>
@rickbrouwer rickbrouwer removed the waiting-author-response All PR's or Issues where we are waiting for a response from the author label Apr 10, 2026
@wozniakjan wozniakjan requested a review from zroubalik April 13, 2026 06:58
@wozniakjan wozniakjan enabled auto-merge (squash) April 13, 2026 07:06
@wozniakjan wozniakjan dismissed zroubalik’s stale review April 13, 2026 07:07

changelog recommendation addressed

@wozniakjan wozniakjan merged commit 7d059fa into kedacore:main Apr 13, 2026
26 checks passed
@rickbrouwer rickbrouwer removed the Awaiting/2nd-approval This PR needs one more approval review label Apr 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wozniakjan wozniakjan wozniakjan approved these changes
@rickbrouwer rickbrouwer rickbrouwer approved these changes
@zroubalik zroubalik Awaiting requested review from zroubalik

Assignees

No one assigned

Labels

skip-e2e

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Flaky external_scaler_test test (TestWaitForState)

4 participants

@RokeshVS @zroubalik @wozniakjan @rickbrouwer