Skip to content

enable race condition check for unit tests #6760

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 12, 2025
Merged

enable race condition check for unit tests #6760

merged 1 commit into from
May 12, 2025

Conversation

wozniakjan
Copy link
Member

@wozniakjan wozniakjan commented May 6, 2025 •
edited
Loading

This PR adds an option to run unit tests with race detection enabled. Also adding a trivial test for the scalers cache which discovers existing race condition.

WARNING: DATA RACE
Write at 0x00c000b25ec8 by goroutine 73:
  github.com/kedacore/keda/v2/pkg/scaling/cache.(*ScalersCache).Close()
      /home/projects/go/src/github.com/kedacore/keda/pkg/scaling/cache/scalers_cache.go:94 +0xa4
  github.com/kedacore/keda/v2/pkg/scaling/cache.TestEmptyScalersCache.func5()
      /home/projects/go/src/github.com/kedacore/keda/pkg/scaling/cache/scalers_cache_test.go:55 +0x3c

Previous read at 0x00c000b25ec8 by goroutine 70:
  github.com/kedacore/keda/v2/pkg/scaling/cache.(*ScalersCache).GetPushScalers()
      /home/projects/go/src/github.com/kedacore/keda/pkg/scaling/cache/scalers_cache.go:82 +0x49
  github.com/kedacore/keda/v2/pkg/scaling/cache.TestEmptyScalersCache.func2()
      /home/projects/go/src/github.com/kedacore/keda/pkg/scaling/cache/scalers_cache_test.go:39 +0x44

Goroutine 73 (running) created at:
  github.com/kedacore/keda/v2/pkg/scaling/cache.TestEmptyScalersCache()
      /home/projects/go/src/github.com/kedacore/keda/pkg/scaling/cache/scalers_cache_test.go:54 +0x2e4
  testing.tRunner()
      /usr/lib/go/src/testing/testing.go:1792 +0x225
  testing.(*T).Run.gowrap1()
      /usr/lib/go/src/testing/testing.go:1851 +0x44

Goroutine 70 (finished) created at:
  github.com/kedacore/keda/v2/pkg/scaling/cache.TestEmptyScalersCache()
      /home/projects/go/src/github.com/kedacore/keda/pkg/scaling/cache/scalers_cache_test.go:38 +0x1a4
  testing.tRunner()
      /usr/lib/go/src/testing/testing.go:1792 +0x225
  testing.(*T).Run.gowrap1()
      /usr/lib/go/src/testing/testing.go:1851 +0x44

Intentionally not enabling this by default and not requiring PRs to pass because there is a number of already existing race conditions that are outside of the scope of this PR to address, one getting fixed already in #6739.

Checklist

  • Tests have been added
  • Commits are signed with Developer Certificate of Origin (DCO - learn more)

List of currently reported race conditions - all_races.txt

relates to: #6739
see also: #6761

@wozniakjan wozniakjan requested a review from a team as a code owner May 6, 2025 14:30
@wozniakjan wozniakjan mentioned this pull request May 6, 2025
Merged
1 task
@wozniakjan
Copy link
Member Author

/skip-e2e

@wozniakjan
enable race condition check for unit tests
cfd1e27 
Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com>
@wozniakjan wozniakjan force-pushed the enable_race_condition_check_for_unit_tests branch from b1561d7 to cfd1e27 Compare May 6, 2025 14:52
@wozniakjan wozniakjan mentioned this pull request May 6, 2025
Closed
2 tasks
@rickbrouwer rickbrouwer mentioned this pull request May 7, 2025
Merged
2 tasks
@wozniakjan wozniakjan requested a review from JorTurFer May 7, 2025 07:42
@wozniakjan wozniakjan enabled auto-merge (squash) May 9, 2025 06:45
@wozniakjan wozniakjan disabled auto-merge May 12, 2025 12:09
@wozniakjan wozniakjan merged commit b0d1c01 into kedacore:main May 12, 2025
19 checks passed
@wozniakjan wozniakjan mentioned this pull request May 15, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zroubalik zroubalik zroubalik approved these changes

@JorTurFer JorTurFer Awaiting requested review from JorTurFer

Assignees
No one assigned
Labels
skip-e2e
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@wozniakjan @zroubalik @keda-automation