-
Notifications
You must be signed in to change notification settings - Fork 134
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
[WIP] Move Integration Tests to NodeJS #194
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Definitely a fan of moving away from shell-based test suites. This seems logical and approachable. Curious what the Contributor Journey will be.
Do new tests go in the __tests__/basic.test.js
file, or new files based on the functionality being tests? I'm guessing new files. If that's correct, will there he a template or generator for creating new test files?
Overall this looks good. I have not written Jest / JavaScript tests in a long time so details are new again for me. Regardless, this is a good start!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
There are two options here: | ||
1. In the `afterAll` block, comment out the code that runs the container teardown. After the test fails, you can run `docker logs -f nginx-s3-gateway-test-base` | ||
|
||
2. Before the code that is erroring, add `timeout(3000)` then quickly run `docker logs -f nginx-s3-gateway-test-base` in another tab after the container starts. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe something like this: How to redirect Docker logs to file?, or using a mount to keep the logs around after the container closes?
tsconfig.json
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For me all these commented lines are quite distracting. Though I cannot imagine I'd be in this file very often.
I would do an explicit call out for the need to install node and/or npm modules. |
5b21005
to
abff552
Compare
6d2bced
to
119c0e0
Compare
Note
This PR is a work in progress but feedback on direction is welcome.
What
This PR moves the existing integration test suite into a Jest + Superagent-based suite.
The goal is to mostly replicate the behavior of the current test suite while also accounting for the large number of potential configurations for the project.
Details
Organization
Design
The basic execution flow for each test will be:
The principles we will try to follow are: