Skip to content

Conversation

@mathbunnyru
Copy link
Contributor

@mathbunnyru mathbunnyru commented Oct 23, 2025

High Level Overview of Change

In #5817 only rippled is uploaded as artifact, but there are multiple testing binaries to upload (and test).

This approach:

  • builds everything during build (it's counter-intuitive to build test binaries during test, which happened before)
  • puts them in the same dir (to make it easy to work with on both Windows and non-Windows)
  • runs them in test workflow properly

Context of Change

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (non-breaking change that only restructures code)
  • Performance (increase or change in throughput and/or latency)
  • Tests (you added tests for code that already exists, or your new feature included in this PR)
  • Documentation update
  • Chore (no impact to binary, e.g. .gitignore, formatting, dropping support for older tooling)
  • Release

API Impact

  • Public API: New feature (new methods and/or new fields)
  • Public API: Breaking change (in general, breaking changes should only impact the next api_version)
  • libxrpl change (any change that may affect libxrpl or dependents of libxrpl)
  • Peer protocol change (must be backward compatible or bump the peer protocol version)

@mathbunnyru mathbunnyru changed the title fix: Upload all test binaries fix: Upload all test binaries [DO NOT MERGE] Oct 23, 2025
@codecov
Copy link

codecov bot commented Oct 23, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.5%. Comparing base (e192ffe) to head (94d7f7e).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           develop   #5932   +/-   ##
=======================================
  Coverage     78.5%   78.5%           
=======================================
  Files          817     817           
  Lines        69017   69017           
  Branches      8276    8297   +21     
=======================================
+ Hits         54197   54207   +10     
+ Misses       14820   14810   -10     

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.

@mathbunnyru mathbunnyru force-pushed the upload_tests branch 2 times, most recently from 92e9b32 to 47c1fe7 Compare October 23, 2025 18:08
@mathbunnyru mathbunnyru changed the title fix: Upload all test binaries [DO NOT MERGE] fix: Upload all test binaries Oct 23, 2025
@mathbunnyru mathbunnyru force-pushed the upload_tests branch 2 times, most recently from 6dbc5de to e0b9f86 Compare October 24, 2025 07:51
Copy link
Collaborator

@bthomee bthomee left a comment

Choose a reason for hiding this comment

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

Looks good. There's still my other comment regarding parallelization - hopefully you can address that one too.

@mathbunnyru
Copy link
Contributor Author

mathbunnyru commented Oct 24, 2025

Looks good. There's still my other comment regarding parallelization - hopefully you can address that one too.

Yes, I will, I've been busy with trying to make it work for Windows though - it's a bit tricky, since I don't know much about where build files end up there

Copy link
Collaborator

@vvysokikh1 vvysokikh1 left a comment

Choose a reason for hiding this comment

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

I approve cmake file change, as for workflows I don't really feel like I understand anything anyway :)

@mathbunnyru mathbunnyru 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 Oct 24, 2025
@mathbunnyru mathbunnyru force-pushed the upload_tests branch 3 times, most recently from 352e430 to 10b8df0 Compare October 24, 2025 15:59
…tiple testing binaries to upload (and test).

This change:
* Builds everything during build.
* Puts them in the same dir (to make it easy to work with on both Windows and non-Windows).
* Runs them in test workflow properly.
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.

3 participants