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

test(command-output): stabilize server jobs test #3194

Merged
merged 2 commits into from
Mar 7, 2023

Conversation

finnag
Copy link
Contributor

@finnag finnag commented Mar 7, 2023

what

Fix two race conditions in TestProjectCommandOutputHandler

  • use buffered channels
  • add missing wg.Wait()

why

The test github.com/runatlantis/atlantis/server/jobs , in TestProjectCommandOutputHandler, sometimes times out.

tests

  • I have tested my changes by running go test -timeout 14s -count 500 github.com/runatlantis/atlantis/server/jobs 50 times. no timeouts.

references

Use buffered channels in TestProjectCommandOutputHandler

The output handler registered with
ProjectCommandOutputHandler.Register() needs the channel that
is given to not block writes.  If the channel
blocks writes, the channel is removed from the list
of active receivers on that JobID.

The tests are set up with unbuffered channels, and there
is a race condition where the channel reader does
not hit `for msg := range ch` before the message that
is sent is handled by AsyncProjectCommandOutputHandler.writeLogLine

By making the channel buffered it does not matter
that the reader isn't reading yet.
There is a rare race condition where messages come in
while the asserts run in the
"clean up all jobs when PR is closed" test.

Added  wg.Wait() to make sure all messages are processed
before running the final asserts.
@finnag finnag requested a review from a team as a code owner March 7, 2023 13:28
@github-actions github-actions bot added the go Pull requests that update Go code label Mar 7, 2023
@nitrocode nitrocode changed the title Fix unstable server jobs test test(command-output): stabilize server jobs test Mar 7, 2023
@nitrocode nitrocode added this to the v0.23.3 milestone Mar 7, 2023
@nitrocode nitrocode merged commit 739d6e4 into runatlantis:main Mar 7, 2023
nitrocode pushed a commit that referenced this pull request May 5, 2023
* Bugfix: TestProjectCommandOutputHandler is unstable

Use buffered channels in TestProjectCommandOutputHandler

The output handler registered with
ProjectCommandOutputHandler.Register() needs the channel that
is given to not block writes.  If the channel
blocks writes, the channel is removed from the list
of active receivers on that JobID.

The tests are set up with unbuffered channels, and there
is a race condition where the channel reader does
not hit `for msg := range ch` before the message that
is sent is handled by AsyncProjectCommandOutputHandler.writeLogLine

By making the channel buffered it does not matter
that the reader isn't reading yet.

* Add missing wg.Wait() in TestProjectCommandOutputHandler

There is a rare race condition where messages come in
while the asserts run in the
"clean up all jobs when PR is closed" test.

Added  wg.Wait() to make sure all messages are processed
before running the final asserts.

---------

Co-authored-by: Finn Arne Gangstad <[email protected]>
@finnag finnag deleted the fix-unstable-server-jobs-test branch May 12, 2023 13:49
ijames-gc pushed a commit to gocardless/atlantis that referenced this pull request Feb 13, 2024
* Bugfix: TestProjectCommandOutputHandler is unstable

Use buffered channels in TestProjectCommandOutputHandler

The output handler registered with
ProjectCommandOutputHandler.Register() needs the channel that
is given to not block writes.  If the channel
blocks writes, the channel is removed from the list
of active receivers on that JobID.

The tests are set up with unbuffered channels, and there
is a race condition where the channel reader does
not hit `for msg := range ch` before the message that
is sent is handled by AsyncProjectCommandOutputHandler.writeLogLine

By making the channel buffered it does not matter
that the reader isn't reading yet.

* Add missing wg.Wait() in TestProjectCommandOutputHandler

There is a rare race condition where messages come in
while the asserts run in the
"clean up all jobs when PR is closed" test.

Added  wg.Wait() to make sure all messages are processed
before running the final asserts.

---------

Co-authored-by: Finn Arne Gangstad <[email protected]>
ijames-gc pushed a commit to gocardless/atlantis that referenced this pull request Feb 13, 2024
* Bugfix: TestProjectCommandOutputHandler is unstable

Use buffered channels in TestProjectCommandOutputHandler

The output handler registered with
ProjectCommandOutputHandler.Register() needs the channel that
is given to not block writes.  If the channel
blocks writes, the channel is removed from the list
of active receivers on that JobID.

The tests are set up with unbuffered channels, and there
is a race condition where the channel reader does
not hit `for msg := range ch` before the message that
is sent is handled by AsyncProjectCommandOutputHandler.writeLogLine

By making the channel buffered it does not matter
that the reader isn't reading yet.

* Add missing wg.Wait() in TestProjectCommandOutputHandler

There is a rare race condition where messages come in
while the asserts run in the
"clean up all jobs when PR is closed" test.

Added  wg.Wait() to make sure all messages are processed
before running the final asserts.

---------

Co-authored-by: Finn Arne Gangstad <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
go Pull requests that update Go code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unstable test github.com/runatlantis/atlantis/server/jobs
2 participants