Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LPv2: UTs for Batches #1952

Merged
merged 1 commit into from
Aug 9, 2024
Merged

LPv2: UTs for Batches #1952

merged 1 commit into from
Aug 9, 2024

Conversation

lemunozm
Copy link
Contributor

@lemunozm lemunozm commented Aug 8, 2024

Description

Add UTs for batches

@lemunozm lemunozm added the I4-tests Test needs fixing or improving. label Aug 8, 2024
@lemunozm lemunozm requested review from cdamian and wischli August 8, 2024 11:12
@lemunozm lemunozm self-assigned this Aug 8, 2024
Comment on lines -511 to -512
let weight = Weight::from_parts(0, T::Message::max_encoded_len() as u64)
.saturating_add(LP_DEFENSIVE_WEIGHT);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The defensive weight shadows the length of a message PoV. Anyways the max_encoded_len is not used here in any storage. Just process the content of the message, which is not dependent of the bytes of the message

Copy link

codecov bot commented Aug 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 47.90%. Comparing base (4bfeabe) to head (78dddfb).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1952      +/-   ##
==========================================
+ Coverage   47.83%   47.90%   +0.07%     
==========================================
  Files         183      183              
  Lines       12904    12902       -2     
==========================================
+ Hits         6172     6181       +9     
+ Misses       6732     6721      -11     

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

@lemunozm lemunozm enabled auto-merge (squash) August 9, 2024 08:44
Copy link
Contributor

@wischli wischli left a comment

Choose a reason for hiding this comment

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

LGTM!

@lemunozm lemunozm merged commit 9485070 into main Aug 9, 2024
9 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I4-tests Test needs fixing or improving.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants