Skip to content

test(pubsub): lower flaky high-volume test to 500 messages#2766

Merged
spomichter merged 2 commits into
mainfrom
fix/ivan/flakey_volumetest
Jul 8, 2026
Merged

test(pubsub): lower flaky high-volume test to 500 messages#2766
spomichter merged 2 commits into
mainfrom
fix/ivan/flakey_volumetest

Conversation

@leshy

@leshy leshy commented Jul 7, 2026

Copy link
Copy Markdown
Member

test_high_volume_messages was flakey

test_high_volume_messages is an acceptance test, not a benchmark, and
5k messages made it flaky. Drop to 500 to keep it stable while still
verifying no messages are lost under moderate volume.
@greptile-apps

greptile-apps Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR reduces the flaky pubsub high-volume test size.

  • Updates the test description to call it moderate-volume coverage.
  • Lowers the message count from 5000 to 500.
  • Keeps the same publish-and-receive assertion flow.

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.

Important Files Changed

Filename Overview
dimos/protocol/pubsub/test_spec.py Updates the pubsub high-volume test to use 500 messages and matching comments.

Reviews (2): Last reviewed commit: "Merge branch 'main' into fix/ivan/flakey..." | Re-trigger Greptile

Comment thread dimos/protocol/pubsub/test_spec.py
@codecov

codecov Bot commented Jul 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

@@            Coverage Diff             @@
##             main    #2766      +/-   ##
==========================================
- Coverage   72.08%   72.07%   -0.01%     
==========================================
  Files         941      941              
  Lines       84224    84224              
  Branches     7650     7650              
==========================================
- Hits        60709    60706       -3     
- Misses      21472    21473       +1     
- Partials     2043     2045       +2     
Flag Coverage Δ
OS-ubuntu-24.04-arm 64.46% <0.00%> (+<0.01%) ⬆️
OS-ubuntu-latest 67.03% <0.00%> (-0.01%) ⬇️
Py-3.10 67.02% <0.00%> (+<0.01%) ⬆️
Py-3.11 67.02% <0.00%> (-0.01%) ⬇️
Py-3.12 67.02% <0.00%> (+<0.01%) ⬆️
Py-3.13 67.02% <0.00%> (+<0.01%) ⬆️
Py-3.14 67.03% <0.00%> (+<0.01%) ⬆️
Py-3.14t 67.02% <0.00%> (-0.01%) ⬇️
SelfHosted-Large 30.20% <0.00%> (ø)
SelfHosted-Linux 38.03% <100.00%> (ø)
SelfHosted-macOS 36.32% <0.00%> (+<0.01%) ⬆️

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

Files with missing lines Coverage Δ
dimos/protocol/pubsub/test_spec.py 90.47% <100.00%> (ø)

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread dimos/protocol/pubsub/test_spec.py

@spomichter spomichter left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approving since change makes sense but also unsure what the test does and hard coding 5k messages seems arbitrary as we move to more transports.

@leshy

leshy commented Jul 8, 2026

Copy link
Copy Markdown
Member Author

Approving since change makes sense but also unsure what the test does and hard coding 5k messages seems arbitrary as we move to more transports.

Thinking was for a local transport if you cannot exchange 5k small messages it's not a functional transport

@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Jul 8, 2026
@spomichter spomichter merged commit 8e0a81b into main Jul 8, 2026
50 of 52 checks passed
@spomichter spomichter deleted the fix/ivan/flakey_volumetest branch July 8, 2026 19:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PlzReview ready-to-merge Required CI checks have passed on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants