-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
grunning: improve some tests #86185
grunning: improve some tests #86185
Conversation
8735beb
to
6f68fc3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @irfansharif)
pkg/util/grunning/enabled_test.go
line 143 at r1 (raw file):
defer runtime.GOMAXPROCS(runtime.GOMAXPROCS(1)) // Create a CPU hog.
Maybe something like the following?
// Create a CPU hog. It makes the two goroutines that want to cycle between running and waiting also have to wait in runnable state, until the CPU hog is finished with its time slice.
pkg/util/grunning/enabled_test.go
line 165 at r1 (raw file):
} pingern = grunning.Time().Nanoseconds() close(stop)
// Stop the CPU hog.
pkg/util/grunning/enabled_test.go
line 176 at r1 (raw file):
}() ping <- true // start ping-pong <-stop // wait for the pinger to finish
// wait until the pinger tells the CPU hog to finish
(the <-done are waiting for the pinger, ponger and CPU hog to actually finish, IIUC)
Release note: None Release justification: test-only changes
6f68fc3
to
0ee5479
Compare
bors r+ |
bors r- |
Canceled. |
I don't really understand the "No pass/skip/fail event found for test" failure in the extended CI but I think it's optional. The Github CI + Bazel Essential CI pass so I'm just going to bors this. bors r+ |
Build succeeded: |
Release note: None
Release justification: test-only changes