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

Further reduce test parameter downloads to reduce CI flakiness #2603

Open
wants to merge 2 commits into
base: staging
Choose a base branch
from

Conversation

vicsn
Copy link
Collaborator

@vicsn vicsn commented Jan 27, 2025

Motivation

A parameter test still failed recently: https://app.circleci.com/pipelines/github/ProvableHQ/snarkVM/13487/workflows/4c90c4b7-c758-4774-a323-6dd68c421a7f/jobs/597293?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-checks-link&utm_content=summary

Further reducing the downloads, as already proposed by @raychu86 in the past

Test Plan

Existing tests should suffice.

Related PRs

The previous reduction in the tests was implemented in #2594

@vicsn vicsn requested review from niklaslong and raychu86 January 27, 2025 08:50
@vicsn vicsn force-pushed the further_reduce_param_downloads branch from a8a8887 to bb88779 Compare January 27, 2025 12:58
@raychu86
Copy link
Contributor

Is this really the correct approach? I think at least we need a unit test to prove that these SRS can't be used based on the protocol limits.

Alternatively, we can disable the CI, but still allow the downloads of these SRS. (This might require more LoC changed)

@vicsn
Copy link
Collaborator Author

vicsn commented Jan 27, 2025

Is this really the correct approach? I think at least we need a unit test to prove that these SRS can't be used based on the protocol limits.

We already approved this approach in #2594 and the reasoning for why its safe is in there and in the code comments.

Alternatively, we can disable the CI, but still allow the downloads of these SRS. (This might require more LoC changed)

Yes that would be ideal and I think we should do so, or think of a better CI setup, when it becomes prioritiy.

@vicsn
Copy link
Collaborator Author

vicsn commented Jan 27, 2025

Happy to host a review session on the topic if you want.

@d0cd
Copy link
Collaborator

d0cd commented Feb 3, 2025

Is there a way to reduce the parameter downloads only during CI via some feature flags?

@niklaslong
Copy link
Collaborator

Is there a way to reduce the parameter downloads only during CI via some feature flags?

Yep! Feature flags in place of the commented out code should work fine—we could introduce a ci feature.

@vicsn
Copy link
Collaborator Author

vicsn commented Feb 3, 2025

Is there a way to reduce the parameter downloads only during CI via some feature flags?

@raychu86 does that sound acceptable to you?

@raychu86
Copy link
Contributor

raychu86 commented Feb 3, 2025

Is there a way to reduce the parameter downloads only during CI via some feature flags?

Yep! Feature flags in place of the commented out code should work fine—we could introduce a ci feature.

Yes let's do that!

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.

4 participants