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

Fix append_logic_xor test #780

Merged
merged 1 commit into from
Oct 20, 2023
Merged

Fix append_logic_xor test #780

merged 1 commit into from
Oct 20, 2023

Conversation

xevisalle
Copy link
Member

@xevisalle xevisalle commented Oct 18, 2023

Fixes a problem in the tests related to append_logic_xor. It computed an xor operation without taking into account the modulus applied to all operations. Basically:

((a ^ b) & bit_mask) % mod != (((a ^ b) % mod) & bit_mask) % mod

@xevisalle xevisalle linked an issue Oct 18, 2023 that may be closed by this pull request
@codecov
Copy link

codecov bot commented Oct 18, 2023

Codecov Report

Merging #780 (1ad528f) into master (8f9cf5a) will increase coverage by 1.02%.
The diff coverage is 100.00%.

❗ Current head 1ad528f differs from pull request most recent head f7c2fa9. Consider uploading reports for the commit f7c2fa9 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #780      +/-   ##
==========================================
+ Coverage   84.02%   85.05%   +1.02%     
==========================================
  Files          57       57              
  Lines        4094     4047      -47     
==========================================
+ Hits         3440     3442       +2     
+ Misses        654      605      -49     
Files Coverage Δ
tests/logic.rs 98.21% <100.00%> (+30.28%) ⬆️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8f9cf5a...f7c2fa9. Read the comment docs.

@xevisalle xevisalle requested a review from moCello October 19, 2023 08:08
Copy link
Member

@moCello moCello left a comment

Choose a reason for hiding this comment

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

LGTM

@xevisalle xevisalle merged commit 9f4bfc7 into master Oct 20, 2023
5 checks passed
@xevisalle xevisalle deleted the random_bug branch October 20, 2023 08:29
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.

Test fails when using rng
2 participants