test: fix error reporting system test race #279
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The system tests for error reporting were still not safe to run concurrently. The issue is that at the end we were deleting all events (there is no API to delete events selectively, AFAICT). This meant that concurrent executions of the system tests against the same project could clobber anothers logged errors. This is consistent with the observed timeouts looking for events to show up. Ref: googleapis/nodejs-logging-bunyan#275
Codecov Report
@@ Coverage Diff @@
## master #279 +/- ##
=======================================
Coverage 92.15% 92.15%
=======================================
Files 6 6
Lines 102 102
Branches 18 18
=======================================
Hits 94 94
Misses 4 4
Partials 4 4Continue to review full report at Codecov.
|
Previously we were comparing the list of services using the representative sample. This means that if more than one service have the same error, we may not necessarily get the sample that we want to observe. Instead look for all the affected services for all groups to find the matching group.
Contributor
Author
|
@soldair note that this has an additional commit here. It was necessary here because we run the error reporting test twice, once for winston2 and once for winston3. This needed even more smarts to deflake. Once this LGTY, I can copy this over to bunyan next. |
soldair
approved these changes
Mar 14, 2019
ofrobots
added a commit
to googleapis/nodejs-logging-bunyan
that referenced
this pull request
Mar 14, 2019
Previously we were comparing the list of services using the representative sample. This means that if more than one service have the same error, we may not necessarily get the sample that we want to observe. Instead look for all the affected services for all groups to find the matching group. Ref: googleapis/nodejs-logging-winston#279
ofrobots
added a commit
to googleapis/nodejs-logging-bunyan
that referenced
this pull request
Mar 14, 2019
Previously we were comparing the list of services using the representative sample. This means that if more than one service have the same error, we may not necessarily get the sample that we want to observe. Instead look for all the affected services for all groups to find the matching group. Ref: googleapis/nodejs-logging-winston#279
miguelvelezsa
pushed a commit
to googleapis/google-cloud-node
that referenced
this pull request
Jan 14, 2026
Previously we were comparing the list of services using the representative sample. This means that if more than one service have the same error, we may not necessarily get the sample that we want to observe. Instead look for all the affected services for all groups to find the matching group. Ref: googleapis/nodejs-logging-winston#279
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The system tests for error reporting were still not safe to run
concurrently. The issue is that at the end we were deleting all events
(there is no API to delete events selectively, AFAICT). This meant that
concurrent executions of the system tests against the same project could
clobber anothers logged errors.
This is consistent with the observed timeouts looking for events to
show up.
Ref: googleapis/nodejs-logging-bunyan#275