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.

Sign up for GitHub

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

Jump to bottom

Add thorough tests for debug file chunk uploading #2194

Closed
szokeasaurusrex opened this issue Oct 23, 2024 · 1 comment
Closed

Add thorough tests for debug file chunk uploading #2194

szokeasaurusrex opened this issue Oct 23, 2024 · 1 comment
Assignees
@szokeasaurusrex

Comments

@szokeasaurusrex
Copy link
Member

Our current chunk uploading tests don't appear to actually verify that inputting a given file to Sentry CLI for chunk upload produces the expected chunks.

Before refactoring the chunk uploading code to add support for chunk uploading Proguard files, we should add tests that ensure that for a given debug information file, Sentry CLI produces the correct chunks for upload. This way, we can more easily detect if our refactor introduces a change in behavior
This was referenced Oct 23, 2024
Closed
Closed
szokeasaurusrex added a commit that referenced this issue Nov 6, 2024
@szokeasaurusrex
ref(tests): Decouple test env vars from trycmd code
f11a5b1 
As a part of #2194, I intend to introduce the [`assert_cmd` crate](https://docs.rs/assert_cmd/latest/assert_cmd/) to have more control over how the assertions for the chunk upload tests are run. This control would be difficult to achieve with `trycmd`.

So, here, I am splitting off the logic for setting environment variables to make it also reusable for `assert_cmd` tests
@szokeasaurusrex szokeasaurusrex mentioned this issue Nov 6, 2024
Merged
szokeasaurusrex added a commit that referenced this issue Nov 6, 2024
@szokeasaurusrex
ref(tests): Decouple test env vars from trycmd code
3a0b0e6 
As a part of #2194, I intend to introduce the [`assert_cmd` crate](https://docs.rs/assert_cmd/latest/assert_cmd/) to have more control over how the assertions for the chunk upload tests are run. This control would be difficult to achieve with `trycmd`.

So, here, I am splitting off the logic for setting environment variables to make it also reusable for `assert_cmd` tests
szokeasaurusrex added a commit that referenced this issue Nov 6, 2024
@szokeasaurusrex
build: Add assert_cmd as a dev dependency
9fef5c2 
The [`assert_cmd`](https://docs.rs/assert_cmd/latest/assert_cmd/) crate provides an interface which allows more fine-grained control over how an individual integration test is run compared with the `trycmd` crate we currently use everywhere. `assert_cmd` will help us write better integration tests for chunk uploading, which not only assert command output, but also ensure that the correct data is sent to the Sentry server (see #2194).
@szokeasaurusrex szokeasaurusrex mentioned this issue Nov 6, 2024
Merged
szokeasaurusrex added a commit that referenced this issue Nov 6, 2024
@szokeasaurusrex
ref(tests): Decouple test env vars from trycmd code
c46bb3c 
As a part of #2194, I intend to introduce the [`assert_cmd` crate](https://docs.rs/assert_cmd/latest/assert_cmd/) to have more control over how the assertions for the chunk upload tests are run. This control would be difficult to achieve with `trycmd`.

So, here, I am splitting off the logic for setting environment variables to make it also reusable for `assert_cmd` tests
szokeasaurusrex added a commit that referenced this issue Nov 6, 2024
@szokeasaurusrex
build: Add assert_cmd as a dev dependency
6f354d2 
The [`assert_cmd`](https://docs.rs/assert_cmd/latest/assert_cmd/) crate provides an interface which allows more fine-grained control over how an individual integration test is run compared with the `trycmd` crate we currently use everywhere. `assert_cmd` will help us write better integration tests for chunk uploading, which not only assert command output, but also ensure that the correct data is sent to the Sentry server (see #2194).
@szokeasaurusrex szokeasaurusrex mentioned this issue Nov 7, 2024
Closed
@szokeasaurusrex szokeasaurusrex self-assigned this Nov 18, 2024
szokeasaurusrex added a commit that referenced this issue Nov 27, 2024
@szokeasaurusrex
test(chunk-upload): Add a test for uploading multiple debug files
05cf43a 
This test ensures that the correct chunks are sent to the server when multiple debug files are being uploaded.

Note that our chunk uploading code does not guarantee that the chunks appear in any particular order within the request. Only the invariant that all chunks get uploaded (in any arbitrary order) is guaranteed. Because of this, we need to parse the request body into an unordered set of all chunks, and compare this against the set of chunks we expect to receive.

ref #2194
@szokeasaurusrex szokeasaurusrex mentioned this issue Nov 27, 2024
Merged
szokeasaurusrex added a commit that referenced this issue Nov 27, 2024
@szokeasaurusrex
test(chunk-upload): Add a test for uploading multiple debug files (#2274
e6e8bf3 
)

This test ensures that the correct chunks are sent to the server when
multiple debug files are being uploaded.

Note that our chunk uploading code does not guarantee that the chunks
appear in any particular order within the request. Only the invariant
that all chunks get uploaded (in any arbitrary order) is guaranteed.
Because of this, we need to parse the request body into an unordered set
of all chunks, and compare this against the set of chunks we expect to
receive.

ref #2194
szokeasaurusrex added a commit that referenced this issue Nov 27, 2024
@szokeasaurusrex
test(chunk-upload): Test chunk upload with many chunks
c679edc 
Add a test that verifies that chunk uploading works as expected for an upload which requires many (500+) chunks, split accross multiple (10) requests. This test also verifies that the chunk uploading code works for smaller chunks, since the way we achieve simulating a many-chunk upload without requiring a very large file to upload is by reducing the chunk size (in this case to 2 KiB).

ref #2194
@szokeasaurusrex szokeasaurusrex mentioned this issue Nov 27, 2024
Merged
szokeasaurusrex added a commit that referenced this issue Nov 27, 2024
@szokeasaurusrex
test(chunk-upload): Test upload where only some files need upload
21fe3a8 
When only some debug files need to be uploaded (because some are already on the server), we should only upload the files which are missing.

Ref #2194
@szokeasaurusrex szokeasaurusrex mentioned this issue Nov 27, 2024
Merged
szokeasaurusrex added a commit that referenced this issue Nov 27, 2024
@szokeasaurusrex
test(chunk-upload): Test chunk upload with many chunks
7cfeb7c 
Add a test that verifies that chunk uploading works as expected for an upload which requires many (500+) chunks, split accross multiple (10) requests. This test also verifies that the chunk uploading code works for smaller chunks, since the way we achieve simulating a many-chunk upload without requiring a very large file to upload is by reducing the chunk size (in this case to 2 KiB).

ref #2194
szokeasaurusrex added a commit that referenced this issue Nov 27, 2024
@szokeasaurusrex
test(chunk-upload): Test upload where only some files need upload
e4a5ad6 
When only some debug files need to be uploaded (because some are already on the server), we should only upload the files which are missing.

Ref #2194
szokeasaurusrex added a commit that referenced this issue Nov 27, 2024
@szokeasaurusrex
test(chunk-upload): Test upload where only some files need upload
31ed94d 
When only some debug files need to be uploaded (because some are already on the server), we should only upload the files which are missing.

Ref #2194
@szokeasaurusrex szokeasaurusrex mentioned this issue Nov 27, 2024
Closed
szokeasaurusrex added a commit that referenced this issue Nov 28, 2024
@szokeasaurusrex
test(chunk-upload): Test chunk upload with many chunks (#2275)
166be86 
Add a test that verifies that chunk uploading works as expected for an
upload which requires many (500+) chunks, split accross multiple (10)
requests. This test also verifies that the chunk uploading code works
for smaller chunks, since the way we achieve simulating a many-chunk
upload without requiring a very large file to upload is by reducing the
chunk size (in this case to 2 KiB).

ref #2194
szokeasaurusrex added a commit that referenced this issue Nov 28, 2024
@szokeasaurusrex
test(chunk-upload): Test upload where only some files need upload
391cd67 
When only some debug files need to be uploaded (because some are already on the server), we should only upload the files which are missing.

Ref #2194
szokeasaurusrex added a commit that referenced this issue Nov 28, 2024
@szokeasaurusrex
test(chunk-upload): Test upload where only some files need upload (#2276
26c6352 
)

When only some debug files need to be uploaded (because some are already
on the server), we should only upload the files which are missing.

Ref #2194
@szokeasaurusrex szokeasaurusrex mentioned this issue Nov 28, 2024
Merged
@szokeasaurusrex
Copy link
Member Author

Tests were added in several PRs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@szokeasaurusrex szokeasaurusrex

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@szokeasaurusrex