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

Avoid failing tests on gRPC server shutdown errors #8060

Merged
merged 1 commit into from
Jan 6, 2023

Conversation

ashwanthgoli
Copy link
Contributor

@ashwanthgoli ashwanthgoli commented Jan 6, 2023

What this PR does / why we need it:
TestGRPCGetCacheGenNumbers & TestGRPCGetDeleteRequests keep failing with the following error at line require.NoError(t, baseServer.Serve(lis)) when we shutdown the gRPC server.

Fail in goroutine after TestGRPCGetCacheGenNumbers/error_getting_from_store has completed
...
github.com/stretchr/testify/require.NoError({0x1640260, 0xc000a10340}, {0x16398e0, 0xc000935ef0}, {0x0, 0x0, 0x0})

	/src/loki/vendor/github.com/stretchr/testify/require/require.go:1261 +0x96

github.com/grafana/loki/pkg/storage/stores/indexshipper/compactor/deletion.server.func3()

	/src/loki/pkg/storage/stores/indexshipper/compactor/deletion/grpc_request_handler_test.go:36 +0x50

created by github.com/grafana/loki/pkg/storage/stores/indexshipper/compactor/deletion.server

	/src/loki/pkg/storage/stores/indexshipper/compactor/deletion/grpc_request_handler_test.go:35 +0x2dc

FAIL	github.com/grafana/loki/pkg/storage/stores/indexshipper/compactor/deletion	0.579s

This pr replicates the changes from #6191 to avoid failing the test case on gRPC teardown errors.

replicates the changes
Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Checklist

  • Reviewed the CONTRIBUTING.md guide
  • Documentation added
  • Tests updated
  • CHANGELOG.md updated
  • Changes that require user attention or interaction to upgrade are documented in docs/sources/upgrading/_index.md

fixes the following errors: "Fail in goroutine after (TestGRPCGetCacheGenNumbers/*|TestGRPCGetDeleteRequests/*) has completed"
@ashwanthgoli ashwanthgoli requested a review from a team as a code owner January 6, 2023 14:10
@ashwanthgoli ashwanthgoli changed the title Avoid failing test cases on gRPC server shutdown Avoid failing tests on gRPC server shutdown Jan 6, 2023
@ashwanthgoli ashwanthgoli changed the title Avoid failing tests on gRPC server shutdown Avoid failing tests on gRPC server shutdown errors Jan 6, 2023
@grafanabot
Copy link
Collaborator

./tools/diff_coverage.sh ../loki-target-branch/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

Copy link
Contributor

@dannykopping dannykopping left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@ssncferreira ssncferreira left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@dannykopping dannykopping merged commit dbf0924 into main Jan 6, 2023
@dannykopping dannykopping deleted the ashwanth/fix-flaky-tests branch January 6, 2023 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants