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

ci: add cloud storage driver integration tests to CI #4121

Merged
merged 1 commit into from Oct 26, 2023
Merged

ci: add cloud storage driver integration tests to CI #4121

merged 1 commit into from Oct 26, 2023

Conversation

milosgajdos
Copy link
Member

@milosgajdos milosgajdos commented Oct 23, 2023 •
edited

This PR:

  • includes storage integration tests in the CI build matrix
  • adds a new CI job that runs E2E tests backed by S3 storage driver

@milosgajdos milosgajdos marked this pull request as ready for review October 23, 2023 14:04
@milosgajdos milosgajdos requested review from Jamstah, thaJeztah and wy65701436 and removed request for Jamstah October 23, 2023 14:04
@milosgajdos
Copy link
Member Author

milosgajdos commented Oct 23, 2023 •
edited

NOTE: this is seemingly "blocked" on run-e2e-test task, but that's because I've renamed run-e2e-test to run-e2e-test-inmem-storage to make it more explicit what storage driver is used

Options:

  • I can rename the GH workflow job name back to the original name (currently opened PRs don't have to be rebased)
  • I can change the GH settings so the new workflow name is required (currently opened PRs will need to be rebased)

@thaJeztah
Copy link
Member

oh! had the tab open, and was indeed considering to suggest what you did in that last commit; feel free to make that a squashed commit (we can always rename later if we think that makes things less confusing)

@milosgajdos
Copy link
Member Author

@thaJeztah I will squash. Just made a few more cosmetic changes 💅 Let's wait for CI. Then I'll squash.

@milosgajdos
Copy link
Member Author

Squashed, PTAL @thaJeztah @Jamstah

Jamstah
Jamstah approved these changes Oct 24, 2023
View reviewed changes
Copy link
Collaborator

@Jamstah Jamstah left a comment

Choose a reason for hiding this comment

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

All LGTM, great to have proper S3 testing, I get the impression there are a lot of users.

@milosgajdos
Copy link
Member Author

PTAL @corhere @squizzi

It seems impossible to get anyone's attention to a few lines of change 🙃

@milosgajdos
ci: add cloud storage driver integration tests to CI
da92c34 
* include storage integration tests in the build matrix
* add a new CI job that runs E2E tests backed by S3 storage driver

Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>
@milosgajdos
Copy link
Member Author

Squashed and cleaned up PTAL @thaJeztah

@milosgajdos milosgajdos merged commit 6c694cb into distribution:main Oct 26, 2023
15 checks passed
@milosgajdos milosgajdos deleted the ci-storage-integration-tests branch October 26, 2023 15:39
thaJeztah
thaJeztah reviewed Oct 26, 2023
View reviewed changes
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

post-merge LGTM (sorry, thought I already LGTM'd)

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

@thaJeztah thaJeztah thaJeztah left review comments

@corhere corhere corhere approved these changes

@Jamstah Jamstah Jamstah approved these changes

@wy65701436 wy65701436 Awaiting requested review from wy65701436

@squizzi squizzi Awaiting requested review from squizzi

@deleteriousEffect deleteriousEffect Awaiting requested review from deleteriousEffect

Assignees
No one assigned
Labels
area/ci
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@milosgajdos @thaJeztah @corhere @Jamstah