Skip to content

chore: add standard test logger ignoring db canceled #15556

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

Merged
merged 1 commit into from
Nov 18, 2024

Conversation

spikecurtis
Copy link
Contributor

@spikecurtis spikecurtis commented Nov 18, 2024

Refactors our use of slogtest to instantiate a "standard logger" across most of our tests. This standard logger incorporates coder/slog#217 to also ignore database query canceled errors by default, which are a source of low-severity flakes.

Any test that has set non-default slogtest.Options is left alone. In particular, coderdtest defaults to ignoring all errors. We might consider revisiting that decision now that we have better tools to target the really common flaky Error logs on shutdown.

Copy link
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@spikecurtis spikecurtis marked this pull request as ready for review November 18, 2024 08:05
Copy link
Member

@johnstcn johnstcn left a comment

Choose a reason for hiding this comment

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

I'm a fan of this overall approach of extracting commonly-used test apparatus into testutil. I think your refactor missed some imports in tailnet/test/integration but I'm happy to approve once those are fixed and tests are passing.

@spikecurtis spikecurtis force-pushed the spike/std-test-logger branch from 6391f65 to 5ed8bb2 Compare November 18, 2024 08:20
Copy link
Member

@johnstcn johnstcn left a comment

Choose a reason for hiding this comment

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

I only have that one comment regarding provisioner/terraform/install.go but that's not blocking.

@spikecurtis spikecurtis force-pushed the spike/std-test-logger branch from 5ed8bb2 to f186d26 Compare November 18, 2024 10:00
@spikecurtis spikecurtis merged commit 5861e51 into main Nov 18, 2024
26 of 27 checks passed
Copy link
Contributor Author

Merge activity

  • Nov 18, 5:09 AM EST: A user merged this pull request with Graphite.

@spikecurtis spikecurtis deleted the spike/std-test-logger branch November 18, 2024 10:09
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