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

bug-1906139: systemtests download API improvements #2971

Merged
merged 2 commits into from
Jul 26, 2024
Merged

Conversation

biancadanforth
Copy link
Contributor

@biancadanforth biancadanforth commented Jul 25, 2024

Because:

  • We will rely on systemtests as a primary way to validate the Tecken GCP migration.
  • We were not testing HEAD requests or verifying response headers in the systemtests for the download API.

This commit:

  • Tests the HTTP response for HEAD requests.
  • Checks response headers for both the Tecken redirect response and the storage backend response.

Copy link
Contributor Author

@biancadanforth biancadanforth left a comment

Choose a reason for hiding this comment

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

Added some specific questions inline for the review. Thanks in advance!

systemtests/bin/download_sym_files.py Show resolved Hide resolved
systemtests/bin/download_sym_files.py Outdated Show resolved Hide resolved
Because:
* We will rely on systemtests as a primary way to validate the Tecken GCP migration.
* We were not testing HEAD requests or verifying response headers in the systemtests for the download API.

This commit:
* Tests the HTTP response for HEAD requests.
* Checks response headers for both the Tecken redirect response and the storage backend response for all envs except local.
  * We don't check in local, because the emulators don't currently return the correct response headers, and for the Tecken redirect response headers, the headers being tested are CSP headers that are added by nginx which is not present in the local environment.
  * What headers to check for and their values is based largely on the specifications in the [Tecken GCP migration plan](https://docs.google.com/document/d/1tLoGhMJQrP1m6A5H3D5zElNbTHQ9_vzYbOE9QnrxLUc/edit?usp=sharing).
Copy link
Contributor

@smarnach smarnach left a comment

Choose a reason for hiding this comment

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

This is looking good to me. I've just had one tiny comment.

systemtests/bin/download_sym_files.py Outdated Show resolved Hide resolved
systemtests/bin/download_sym_files.py Show resolved Hide resolved
systemtests/bin/download_sym_files.py Outdated Show resolved Hide resolved
@biancadanforth biancadanforth merged commit 5acbc21 into main Jul 26, 2024
2 checks passed
@biancadanforth biancadanforth deleted the bug-1906139 branch July 26, 2024 18:08
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