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

Make tests generic #105

Merged
merged 3 commits into from
Sep 5, 2022
Merged

Make tests generic #105

merged 3 commits into from
Sep 5, 2022

Conversation

conradoplg
Copy link
Contributor

Based on #103

Create generic version of the tests in frost-core. Make them available if the test-impl feature is set.

Change p256 and ristretto255 implementations to call the generic ones.

@codecov-commenter
Copy link

codecov-commenter commented Jul 28, 2022

Codecov Report

Merging #105 (960a873) into main (d1ddf72) will decrease coverage by 1.65%.
The diff coverage is 91.44%.

❗ Current head 960a873 differs from pull request most recent head 87bc9d4. Consider uploading reports for the commit 87bc9d4 to get more accurate results

@@            Coverage Diff             @@
##             main     #105      +/-   ##
==========================================
- Coverage   89.50%   87.85%   -1.66%     
==========================================
  Files          16       28      +12     
  Lines        1401     2124     +723     
==========================================
+ Hits         1254     1866     +612     
- Misses        147      258     +111     
Impacted Files Coverage Δ
frost-ristretto255/src/lib.rs 69.60% <0.00%> (-24.81%) ⬇️
frost-p256/src/lib.rs 70.07% <42.85%> (-24.41%) ⬇️
frost-core/src/frost/round2.rs 70.00% <80.00%> (ø)
frost-core/src/frost/identifier.rs 81.08% <84.61%> (-0.32%) ⬇️
frost-core/src/frost.rs 91.66% <100.00%> (+0.17%) ⬆️
frost-core/src/frost/keys.rs 83.18% <100.00%> (+0.30%) ⬆️
frost-core/src/frost/round1.rs 90.72% <100.00%> (ø)
frost-core/src/lib.rs 81.81% <100.00%> (+2.40%) ⬆️
frost-core/src/signature.rs 69.76% <100.00%> (-6.24%) ⬇️
frost-core/src/tests.rs 100.00% <100.00%> (ø)
... and 27 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Contributor Author

@conradoplg conradoplg left a comment

Choose a reason for hiding this comment

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

I restored a serialization test which I accidentally used a commented out version for the generic code (thankfully we have coverage and I noticed through that). I also managed to remove the Debug bounds by using the debugless-unwrap crates, see comment.

frost-core/src/tests/vectors.rs Outdated Show resolved Hide resolved
@dconnolly
Copy link
Contributor

@mergify rebase

@mergify
Copy link
Contributor

mergify bot commented Sep 4, 2022

rebase

❌ Base branch update has failed

Git reported the following error:

Rebasing (1/6)
error: could not apply 7a27ed7... use Identifier instead of index
Resolve all conflicts manually, mark them as resolved with
"git add/rm <conflicted_files>", then run "git rebase --continue".
You can instead skip this commit: run "git rebase --skip".
To abort and get back to the state before "git rebase", run "git rebase --abort".
Could not apply 7a27ed7... use Identifier instead of index
Auto-merging frost-ristretto255/src/lib.rs
Auto-merging frost-p256/src/lib.rs
Auto-merging frost-core/src/frost/round1.rs
Auto-merging frost-core/src/frost/keys.rs
CONFLICT (content): Merge conflict in frost-core/src/frost/keys.rs
Auto-merging frost-core/src/frost/identifier.rs
CONFLICT (content): Merge conflict in frost-core/src/frost/identifier.rs
Auto-merging frost-core/src/frost.rs
CONFLICT (content): Merge conflict in frost-core/src/frost.rs

err-code: 50237

@conradoplg
Copy link
Contributor Author

Conflicts were solved

Copy link
Contributor

@dconnolly dconnolly left a comment

Choose a reason for hiding this comment

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

🎉

@dconnolly dconnolly merged commit 298da8f into main Sep 5, 2022
@dconnolly dconnolly deleted the generic-tests branch September 5, 2022 20:34
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.

3 participants