Skip to content

Conversation

@ofrobots
Copy link
Contributor

@ofrobots ofrobots commented Feb 13, 2019

  1. The test timeout was not effectively applied.
  2. All of the tests needed a longer timeout rather than just the error
    reporting ones.
  3. The error-reporting tests would race with a concurrent execution of
    the tests in the CI because they were logging and polling against
    the same service. Uniquify the service name.

Evidence of 2:
image

The default timeout was 10s.

1. The test timeout was not effectively applied.
2. All of the tests needed a longer timeout rather than just the error
   reporting ones.
3. The error-reporting tests would race with a concurrent execution of
   the tests in the CI because they were logging and polling against
   the same service. Uniquify the service name.
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 13, 2019
@ofrobots ofrobots requested a review from soldair February 13, 2019 00:30
Copy link
Contributor

@soldair soldair left a comment

Choose a reason for hiding this comment

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

lgtm

ofrobots added a commit to googleapis/nodejs-logging-bunyan that referenced this pull request Feb 13, 2019
Ref: googleapis/nodejs-logging-winston#265

1. Synchronize the system test with logging-winston. Use the same
   structure and reliable mechanism for polling.
2. Apply the timeout uniformly across the test.
3. Use unique serviceContext for the error reporting tests to ensure
   concurrent executions of the tests do no race.
ofrobots added a commit to googleapis/nodejs-logging-bunyan that referenced this pull request Feb 13, 2019
Ref: googleapis/nodejs-logging-winston#265

1. Synchronize the system test with logging-winston. Use the same
   structure and reliable mechanism for polling.
2. Apply the timeout uniformly across the test.
3. Use unique serviceContext for the error reporting tests to ensure
   concurrent executions of the tests do no race.
@JustinBeckwith JustinBeckwith merged commit b97e806 into googleapis:master Feb 13, 2019
@ofrobots ofrobots deleted the deflake branch February 13, 2019 18:59
JustinBeckwith pushed a commit to googleapis/nodejs-logging-bunyan that referenced this pull request Feb 13, 2019
* fix: de-flake system tests

Ref: googleapis/nodejs-logging-winston#265

1. Synchronize the system test with logging-winston. Use the same
   structure and reliable mechanism for polling.
2. Apply the timeout uniformly across the test.
3. Use unique serviceContext for the error reporting tests to ensure
   concurrent executions of the tests do no race.

* fix: disabled tests had were not being run

These tests were not even running before because of the errant
describe.only in logging-bunyan.ts fixed in the previous commit.

Fix. Deflake.
miguelvelezsa pushed a commit to googleapis/google-cloud-node that referenced this pull request Jan 14, 2026
* fix: de-flake system tests

Ref: googleapis/nodejs-logging-winston#265

1. Synchronize the system test with logging-winston. Use the same
   structure and reliable mechanism for polling.
2. Apply the timeout uniformly across the test.
3. Use unique serviceContext for the error reporting tests to ensure
   concurrent executions of the tests do no race.

* fix: disabled tests had were not being run

These tests were not even running before because of the errant
describe.only in logging-bunyan.ts fixed in the previous commit.

Fix. Deflake.
miguelvelezsa pushed a commit to googleapis/google-cloud-node that referenced this pull request Jan 21, 2026
* fix: de-flake system tests

Ref: googleapis/nodejs-logging-winston#265

1. Synchronize the system test with logging-winston. Use the same
   structure and reliable mechanism for polling.
2. Apply the timeout uniformly across the test.
3. Use unique serviceContext for the error reporting tests to ensure
   concurrent executions of the tests do no race.

* fix: disabled tests had were not being run

These tests were not even running before because of the errant
describe.only in logging-bunyan.ts fixed in the previous commit.

Fix. Deflake.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants