Skip to content

Conversation

@SimonRastikian
Copy link
Contributor

Closes #145

@SimonRastikian SimonRastikian self-assigned this Nov 11, 2025
@codecov-commenter
Copy link

codecov-commenter commented Nov 11, 2025

Codecov Report

❌ Patch coverage is 93.19149% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.11%. Comparing base (08f49aa) to head (948ab62).

Files with missing lines Patch % Lines
src/test_utils/protocol.rs 80.00% 6 Missing and 5 partials ⚠️
src/test_utils/snapshot.rs 96.77% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #218      +/-   ##
==========================================
+ Coverage   87.95%   88.11%   +0.16%     
==========================================
  Files          48       49       +1     
  Lines        9131     9366     +235     
  Branches     9131     9366     +235     
==========================================
+ Hits         8031     8253     +222     
- Misses        653      661       +8     
- Partials      447      452       +5     

☔ 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.

Copy link
Contributor

@gilcu3 gilcu3 left a comment

Choose a reason for hiding this comment

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

I did not find any big issue, but did have a lot of suggestions, therefore marking the review as "Request changes". Most of them should be quite easy to solve though.

Once that is done, I can do a quick pass again and approve.

Copy link
Collaborator

@netrome netrome left a comment

Choose a reason for hiding this comment

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

Some nits from my side, but overall I think it looks good. I agree with the general comments about test organization from Reynaldo.

The only blockers I see are:

  1. Some docstrings are a bit confusing right now.
  2. The tests that use OsRng should be made deterministic.

Base automatically changed from simon/test_utils_folder to main November 20, 2025 14:41
gilcu3
gilcu3 previously approved these changes Nov 21, 2025
Copy link
Contributor

@gilcu3 gilcu3 left a comment

Choose a reason for hiding this comment

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

Thank you for the updates! Left a couple of nits

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Benchmark] [Participant Simulation] Protocols Derandomization: Networking Layer Change

5 participants