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

Enable concurrency testing #374

Merged

Conversation

akoshelev
Copy link
Collaborator

I finally found the bug that was blocking shuttle tests. The issue outlined in #256 was fixed in
#363 but there was another bug inside the concurrency test itself. This change fixes it and enables concurrency testing as part of PR workflow approval.

It also adds a test for semi-honest IPA but given that it is incredibly slow, it only runs it for 2 iterations. Once we improve the IPA latency, number of iterations can be increased.

There was a bug inside the test sending left part of the share twice
Now that they are green, we can run them as part of CI workflow
They don't take long to finish. I expect many failures because folks will forget to add `not(feature = "shuttle")` to their test modules.
src/tests/protocol.rs Outdated Show resolved Hide resolved
Comment on lines 9 to 12
fn semi_honest_ipa() {
shuttle::check_random(
|| {
shuttle::future::block_on(async {
Copy link
Member

Choose a reason for hiding this comment

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

That's a lot of indent.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fair enough, I think it can be improved

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was proven wrong by the formatter :(

shuttle::check_random(
|| {
shuttle::future::block_on(async {
const BATCHSIZE: usize = 20;
Copy link
Member

Choose a reason for hiding this comment

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

Would a smaller batch allow you to increase the number of iterations?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yep, setting it to 5 allowed me to have 6 iterations. still quite small but better

@akoshelev akoshelev merged commit 0481de7 into private-attribution:main Jan 9, 2023
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.

2 participants