Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[WIP] Move Integration Tests to NodeJS#194

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Draft
4141done wants to merge6 commits intomain
base:main
Choose a base branch
Loading
fromje-js-integration-tests

Conversation

4141done
Copy link
Collaborator

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

  1. A basic test set handling encoding, basic usage, etc.
  2. Use-case based tests handling (subject to change)
    1. File/directory browsing
    2. Caching specific
    3. Single page application/asset server
    4. object path acrobatics

Design

The basic execution flow for each test will be:

  1. Ensure a clean state in Minio
  2. Create a bucket using the Minio Client
  3. Write the required test files to the bucket
  4. Build and start an s3-gateway container using the supplied settings for the scenario
  5. Use supertest to send and verify requests against the running container.
  6. Clean up the Minio bucket.

The principles we will try to follow are:

  • Test file structure specific to each use-case based test suite
  • Starting a general transition to thinking about use cases rather than individual config (although that will always be an option)
  • Decoupling general container build from integration test run

Copy link

@ciroqueciroque left a 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!

Copy link

@ciroqueciroque left a 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.

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

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.

@dekobon
Copy link
Collaborator

I would do an explicit call out for the need to install node and/or npm modules.

@4141done4141done changed the base branch frommaster tomainApril 25, 2024 21:39
@4141done4141doneforce-pushed theje-js-integration-tests branch from5b21005 toabff552CompareApril 25, 2024 21:42
@alessfgalessfgforce-pushed themain branch 2 times, most recently from6d2bced to119c0e0CompareApril 26, 2024 21:15
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@ciroqueciroqueciroque left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

3 participants
@4141done@dekobon@ciroque

[8]ページ先頭

©2009-2025 Movatter.jp