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

test: remove the integration test #306

Merged
merged 1 commit into from
Aug 2, 2024
Merged

test: remove the integration test #306

merged 1 commit into from
Aug 2, 2024

Conversation

bajtos
Copy link
Member

@bajtos bajtos commented Aug 2, 2024

This test is evaluating one batch of real measurements. With the upcoming change introducing committees and majorities, a single batch of measurements is not enough to produce committees with the required size. As a result, most measurements are rejected with COMMITTEE_TOO_SMALL evaluation. In a recent test run, the number of accepted measurements dropped from 14,512 to 2,530.

We could fix this test by evaluating all batches in one round, but that would extremely slow down this test to the order of minutes.

Let's remove this test for now. If we find a need to test evaluation results of real data, then I'd like to rework the dry-run CI step to diff the dry-run output against a snapshot.

Links:

This test is evaluating one batch of real measurements. With the
upcoming change introducing committees and majorities, a single batch
of measurements is not enough to produce committees with the required
size. As a result, most measurements are rejected with
COMMITTEE_TOO_SMALL evaluation. In a recent test run, the number of
accepted measurements dropped from 14,512 to 2,530.

We could fix this test by evaluating all batches in one round,
but that would extremely slow down this test to the order of minutes.

Let's remove this test for now. If we find a need to test evaluation
results of real data, then I'd like to rework the `dry-run` CI step
to diff the dry-run output against a snapshot.

Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
Copy link
Member

@juliangruber juliangruber left a comment

Choose a reason for hiding this comment

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

reasoning sgtm!

@bajtos bajtos merged commit 61ab7d1 into main Aug 2, 2024
6 checks passed
@bajtos bajtos deleted the remove-integration-test branch August 2, 2024 09:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ done
Development

Successfully merging this pull request may close these issues.

2 participants