Sponsored Content
Skip to content

fix: Fix flaky CI tests - #7005

Merged
bthomee merged 4 commits into
developfrom
mvadari/fix-flakiness
Apr 24, 2026
Merged

fix: Fix flaky CI tests#7005
bthomee merged 4 commits into
developfrom
mvadari/fix-flakiness

Conversation

@mvadari

@mvadari mvadari commented Apr 23, 2026

Copy link
Copy Markdown
Contributor

High Level Overview of Change

This PR fixes flaky tests in TxQ and PerfLog.

Note: these changes were vibe coded, as are all comment replies, so they should be reviewed carefully. They do not touch source code though, so the risk is low.

Context of Change

These tests suddenly started getting flaky a day or two ago

API Impact

N/A

@xrplf-ai-reviewer xrplf-ai-reviewer Bot 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.

No issues.

Review by Claude Opus 4.6 · Prompt: V15

Copilot AI 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.

Pull request overview

This PR aims to reduce flakiness in the TxQ and PerfLog unit tests by making them more tolerant of persistent temp-directory state and asynchronous WebSocket message timing.

Changes:

  • Add pre-test cleanup to remove any stale PerfLog test directory state from the system temp directory.
  • Update TxQ WebSocket “server” subscription test to drain unexpected extra serverStatus messages before unsubscribing.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
src/test/basics/PerfLog_test.cpp Deletes the perf log test temp directory on fixture construction to avoid stale state across CI runs.
src/test/app/TxQ_test.cpp Replaces a strict “no extra serverStatus” assertion with a drain loop to tolerate asynchronous extra messages.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/test/basics/PerfLog_test.cpp
Comment thread src/test/basics/PerfLog_test.cpp
Comment thread src/test/app/TxQ_test.cpp Outdated
@codecov

codecov Bot commented Apr 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.9%. Comparing base (7a449ed) to head (966b00a).
⚠️ Report is 4 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           develop   #7005   +/-   ##
=======================================
  Coverage     81.9%   81.9%           
=======================================
  Files         1010    1010           
  Lines        76383   76383           
  Branches      7530    7530           
=======================================
+ Hits         62522   62523    +1     
+ Misses       13861   13860    -1     

see 3 files with indirect coverage changes

Impacted file tree graph

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

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 23, 2026 19:08

@xrplf-ai-reviewer xrplf-ai-reviewer Bot 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.

No issues.

Review by Claude Opus 4.6 · Prompt: V15

Copilot AI 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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/test/app/TxQ_test.cpp Outdated

@xrplf-ai-reviewer xrplf-ai-reviewer Bot 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.

No issues.

Review by Claude Opus 4.6 · Prompt: V15

Copilot AI review requested due to automatic review settings April 23, 2026 19:25

@xrplf-ai-reviewer xrplf-ai-reviewer Bot 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.

No issues.

Review by Claude Opus 4.6 · Prompt: V15

Copilot AI 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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/test/app/TxQ_test.cpp
Comment thread src/test/basics/PerfLog_test.cpp
Comment on lines +69 to +71
using namespace boost::filesystem;
boost::system::error_code ec;
remove_all(logDir(), ec);

@mathbunnyru mathbunnyru Apr 23, 2026

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.

Can't we use std::filesystem::remove_all?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The rest of the file uses boost::filesystem throughout (path, exists, remove, create_directories, permissions, etc.). Using std::filesystem just for this one call would be inconsistent. Switching the whole file to std::filesystem would be a separate refactor.

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.

Ah, thanks. It would be great if you made a refactoring PR after this PR is merged.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'll open an issue

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@mvadari mvadari added the Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required. label Apr 24, 2026
@bthomee
bthomee added this pull request to the merge queue Apr 24, 2026
Merged via the queue into develop with commit 9ae2961 Apr 24, 2026
3 checks passed
@bthomee
bthomee deleted the mvadari/fix-flakiness branch April 24, 2026 13:40
marek-foss-neti pushed a commit to marek-foss-neti/rippled that referenced this pull request May 5, 2026
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@mvadari mvadari added this to the 3.2.0 milestone May 20, 2026
beartec-jpg pushed a commit to beartec-jpg/FalconLedger that referenced this pull request Jun 1, 2026
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants