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

sdk: ignore panics due to stray goroutines logging after a test completes #6632

Merged
merged 1 commit into from
Oct 17, 2019

Conversation

rboyer
Copy link
Member

@rboyer rboyer commented Oct 16, 2019

If there is imperfect goroutine lifespan tracking if we pipe our logs
through testing.T.Logf there is a chance of a stray goroutine attempting
to log after the test that spawned it completes.

This results in a panic of:

panic: Log in goroutine after TestLeader_SecondaryCA_Initialize has completed...

This isn't great and should be fixed, but quickly runs into situations
around externally cancelling blocking queries which isn't terribly
possible at the moment. The concession here is to ignore these specific
panics for now.

This can be triggered easily when running some tests with a high
-count=YYY value.

…etes

If there is imperfect goroutine lifespan tracking if we pipe our logs
through testing.T.Logf there is a chance of a stray goroutine attempting
to log after the test that spawned it completes.

This results in a panic of:

    panic: Log in goroutine after TestLeader_SecondaryCA_Initialize has completed...

This isn't great and should be fixed, but quickly runs into situations
around externally cancelling blocking queries which isn't terribly
possible at the moment. The concession here is to ignore these specific
panics for now.

This can be triggered easily when running some tests with a high
`-count=YYY` value.
@rboyer rboyer requested a review from a team October 16, 2019 20:09
@rboyer rboyer self-assigned this Oct 16, 2019
Copy link
Member

@banks banks left a comment

Choose a reason for hiding this comment

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

I agree that it would be ideal to be able to fix goroutine handling at source but since that's not something we can meaningfully address in the short term without rearchitecting major stuff in Consul this seems like a reasonable way to improve consistency in our tests.

@rboyer rboyer merged commit 274b5a4 into master Oct 17, 2019
rboyer added a commit that referenced this pull request Oct 17, 2019
…etes (#6632)

If there is imperfect goroutine lifespan tracking if we pipe our logs
through testing.T.Logf there is a chance of a stray goroutine attempting
to log after the test that spawned it completes.

This results in a panic of:

    panic: Log in goroutine after TestLeader_SecondaryCA_Initialize has completed...

This isn't great and should be fixed, but quickly runs into situations
around externally cancelling blocking queries which isn't terribly
possible at the moment. The concession here is to ignore these specific
panics for now.

This can be triggered easily when running some tests with a high
`-count=YYY` value.
@rboyer rboyer deleted the ignore-testlog-panics branch October 17, 2019 16:02
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