Skip to content

[Tests] E2E File upload/download #3214

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

Merged
merged 6 commits into from
May 22, 2024
Merged

[Tests] E2E File upload/download #3214

merged 6 commits into from
May 22, 2024

Conversation

CarsonF
Copy link
Member

@CarsonF CarsonF commented May 22, 2024

Previously we were faking the upload/download process by talking to the assumed LocalBucket storage.
Where as in actual usage we are expecting PUT/GET HTTP requests.

This changes the tests to actually use that code path.

The reasoning for this previous decision was because we weren't actually serving the GQL API over a server, but executing the operations directly. So without a web server setup, we couldn't do these requests either. That has since changed.

The last hard part here was that we use a unix socket for the test web server which assigns a random port when we start listening.
The app needs to know where it is hosting/listening on so it can generate those PUT/GET urls.
But app boot happens before listening so the port can't be known ahead of time.
I switched the ConfigService.hostUrl to be a BehaviorSubject so we can adjust the url later, after listening starts.
Here's where it is set in the test suite setup.
It's worth noting that Nest's usage of this, like global prefix, does not listen for changes to this value, and only uses the initial one. I've updated all of our usages to respect the current value.

@CarsonF CarsonF requested a review from a team May 22, 2024 16:18
@CarsonF CarsonF merged commit 67c7170 into develop May 22, 2024
15 checks passed
@CarsonF CarsonF deleted the tests/real-file-handling branch May 22, 2024 16:41
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