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

Add full blob batch data #1374

Merged
merged 8 commits into from
Jul 22, 2024
Merged

Add full blob batch data #1374

merged 8 commits into from
Jul 22, 2024

Conversation

darth-cy
Copy link

Description

  • Add a batch data test vector that produces a full blob.
  • This test should pass after blob len check fix in PR1373 is merged.

Type of change

  • Add test vector that covers an edge case.

@darth-cy darth-cy requested a review from roynalnaruto July 22, 2024 13:31
@darth-cy darth-cy self-assigned this Jul 22, 2024
Copy link

@roynalnaruto roynalnaruto left a comment

Choose a reason for hiding this comment

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

@darth-cy approving and merging for now.

However, something to note:
In the future, we may make changes to the zstd encoder configs (for instance, increasing the window log for better compression ratio). When such changes happen, "batch274" will not encode down to exactly 126976 bytes.

I think we must add some kind of an assertion just for this test (probably separate it out into its own unit test), to assert that the blob data config does in fact have 126976 bytes post encoding.

@roynalnaruto roynalnaruto merged commit 61841f1 into develop Jul 22, 2024
17 checks passed
@roynalnaruto roynalnaruto deleted the test/full_blob branch July 22, 2024 21:39
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