Skip to content

Conversation

@Yaminik1996
Copy link
Contributor

@Yaminik1996 Yaminik1996 commented Dec 12, 2025

Removing explicit timeouts in pubsub tests because they make them flaky. Sometimes the test is almost over but the timeout aborts it resulting in a failure. The test performance has also been improved in this PR by removing conditional and unconditional sleep statements.

Some conventions to follow

  1. add the module name as a prefix
    • for example: add a prefix: docstore: for document store module, blobstore for Blob Store module
  2. for a test only PR, add test:
  3. for a perf improvement only PR, add perf:
  4. for a refactoring only PR, add "refactor:"

@codecov-commenter
Copy link

codecov-commenter commented Dec 12, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.18%. Comparing base (7a7efd0) to head (89f4306).

Additional details and impacted files
@@            Coverage Diff            @@
##               main     #203   +/-   ##
=========================================
  Coverage     83.18%   83.18%           
  Complexity       91       91           
=========================================
  Files           150      150           
  Lines          8182     8182           
  Branches        948      948           
=========================================
  Hits           6806     6806           
  Misses          926      926           
  Partials        450      450           
Flag Coverage Δ
unittests 83.18% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

long timeoutSeconds = isRecording ? 120 : 60; // Increased timeout for integration tests
long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(timeoutSeconds);

System.out.println("Starting to collect " + toSend.size() + " messages with timeout: " + timeoutSeconds + "s");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let’s remove this System.out.println from the code and use the logger instead.

Message r = subscription.receive();
if (r != null && r.getAckID() != null) {
ackIDs.add(r.getAckID());
System.out.println("Received message " + ackIDs.size() + "/" + toSend.size() +
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here

System.out.println("Received message " + ackIDs.size() + "/" + toSend.size() +
" with AckID: " + r.getAckID());
} else {
System.out.println("Received null message, waiting...");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here

@Yaminik1996 Yaminik1996 force-pushed the W-20494137/pubsubFlakyTest branch from 89f4306 to f0f05e7 Compare December 15, 2025 22:00
@Yaminik1996 Yaminik1996 force-pushed the W-20494137/pubsubFlakyTest branch from f0f05e7 to 89f4306 Compare December 15, 2025 23:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants