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

Move integration tests to github actions #41139

Merged
merged 13 commits into from
Jan 26, 2024
Merged

Move integration tests to github actions #41139

merged 13 commits into from
Jan 26, 2024

Conversation

nickvergessen
Copy link
Member

@nickvergessen nickvergessen commented Oct 26, 2023 •
edited
Loading

Summary

Run all integration tests on GitHub instead of Drone.

TODO

  • Make integration-sqlite-summary required

Checklist

@nickvergessen nickvergessen added the 2. developing Work in progress label Oct 26, 2023
@nickvergessen nickvergessen added this to the Nextcloud 28 milestone Oct 26, 2023
@nickvergessen nickvergessen self-assigned this Oct 26, 2023
@nickvergessen nickvergessen marked this pull request as draft October 26, 2023 14:23
@nickvergessen nickvergessen force-pushed the techdebt/noid/move-integration-testing-to-actions branch from 6adc610 to 452687f Compare October 27, 2023 09:46
@susnux susnux mentioned this pull request Oct 29, 2023
Closed
13 tasks
@susnux susnux force-pushed the techdebt/noid/move-integration-testing-to-actions branch 5 times, most recently from 89fbe1b to db95484 Compare October 29, 2023 20:28
@susnux susnux force-pushed the techdebt/noid/move-integration-testing-to-actions branch 3 times, most recently from 82fb0d5 to 890ac98 Compare October 29, 2023 21:04
@susnux
Copy link
Contributor

susnux commented Oct 29, 2023

@nickvergessen Tests are now working. But maybe you can have a look and replace the fixups with better solutions?
Seems like tests rely on different assumptions, e.g. LDAP rely on data directory being /dev/shm while some other tests require real access to that files (`federated.features).

Maybe adjust on of those locations so the tests all use the same data directory?

@nickvergessen
Copy link
Member Author

Seems like tests rely on different assumptions, e.g. LDAP rely on data directory being /dev/shm while some other tests require real access to that files (`federated.features).

Yeah, let's keep it for now. The path is hardcoded for LDAP (maybe even into the LDAP config of the test docker, so changing would cause issues on stable branches, etc). Feel free to raise a follow up issue

@skjnldsv skjnldsv mentioned this pull request Nov 1, 2023
Merged
@blizzz blizzz mentioned this pull request Nov 6, 2023
Merged
@susnux susnux changed the base branch from master to chore/migrate-behat-to-github November 7, 2023 00:16
@susnux susnux force-pushed the techdebt/noid/move-integration-testing-to-actions branch 3 times, most recently from 84f5e0d to 69d23ac Compare November 7, 2023 00:42
@susnux susnux changed the title Move setup_features integration test to github actions Move integration tests to github actions Nov 7, 2023
@susnux susnux force-pushed the techdebt/noid/move-integration-testing-to-actions branch 2 times, most recently from a6adc6a to 1a987da Compare November 7, 2023 10:27
@susnux susnux force-pushed the chore/migrate-behat-to-github branch from 10cf7e0 to 016df6c Compare November 8, 2023 12:09
@nickvergessen
Copy link
Member Author

Rebased on master, ready to merge for less load on Drone

@nickvergessen nickvergessen requested a review from susnux January 25, 2024 22:04
@nickvergessen nickvergessen marked this pull request as ready for review January 25, 2024 22:04
@susnux
Copy link
Contributor

susnux commented Jan 25, 2024

Rebased on master, ready to merge for less load on Drone

Lets then also do #41003

Especially nodb would be useful

mejo-
mejo- approved these changes Jan 26, 2024
View reviewed changes
Copy link
Member

@mejo- mejo- left a comment

Choose a reason for hiding this comment

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

Overall looks good. Thanks so much for working on this 👾

Only minor comments, nothing blocking.

Bit by bit
stone by stone
the day will come
we'll sunset drone

.github/workflows/integration-sqlite.yml Outdated Show resolved Hide resolved
build/integration/composer.json Show resolved Hide resolved
build/integration/features/ratelimiting.feature Show resolved Hide resolved
susnux
susnux approved these changes Jan 26, 2024
View reviewed changes
Copy link
Contributor

@susnux susnux left a comment

Choose a reason for hiding this comment

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

🚀

nickvergessen and others added 13 commits January 26, 2024 13:53
@nickvergessen
chore: Move integration tests from drone to github
058b69e 
* Improve grouping as setting up CI took 3* the time of the test in average

Signed-off-by: Joas Schilling <[email protected]>
@nickvergessen
fix: Video verification test by cloning talk
1ecdefd 
Signed-off-by: Joas Schilling <[email protected]>
@nickvergessen
Fix "Creation of dynamic property FeatureContext::$parts is deprecate…
736fb4c 
…d in WebDav.php line 757"

Signed-off-by: Joas Schilling <[email protected]>
@nickvergessen
Remove library that is unmaintained since 2015 and was merged in the …
a1e9c6f 
…behat core

Signed-off-by: Joas Schilling <[email protected]>
@nickvergessen
Ensure the testing app is enabled
d7e2498 
Signed-off-by: Joas Schilling <[email protected]>
@nickvergessen
Don't tail the log output, it's available separately
e7aff76 
Signed-off-by: Joas Schilling <[email protected]>
@nickvergessen
Reduce the output on github CI
cc4e2f9 
Signed-off-by: Joas Schilling <[email protected]>
@nickvergessen
Fix "Creation of dynamic property CommandLineContext::$…BaseUrl is de…
34ea298 
…precated in CommandLineContext.php on line 41"

Signed-off-by: Joas Schilling <[email protected]>
@nickvergessen
Skip the large download again
5ebc5be 
Signed-off-by: Joas Schilling <[email protected]>
@nickvergessen
Enable LDAP tests with redis
b64d032 
Signed-off-by: Joas Schilling <[email protected]>
@susnux @nickvergessen
fix(tests): Ensure ldap server can be reached in integration tests on…
52cc18e 
… GitHub Actions and other improvments

* Run integration tests for every pull request
* Also print docker logs of service containers (ldap, redis)
* Ensure consistent `datadir` for test assertions
* Test openldap features separatly
* Only the LDAP tests rely on `/dev/shm` while `federated.feature` rely on real directory access

Signed-off-by: Ferdinand Thiessen <[email protected]>
@susnux @nickvergessen
fix(integration-s3): Adjust integration test for changed test layout
a3d3517 
Signed-off-by: Ferdinand Thiessen <[email protected]>
@nickvergessen
fix(CI): Use ghcr.io/nextcloud/continuous-integration-redis image
da46be4 
Signed-off-by: Joas Schilling <[email protected]>
@nickvergessen nickvergessen force-pushed the techdebt/noid/move-integration-testing-to-actions branch from f45f11c to da46be4 Compare January 26, 2024 12:53
@nickvergessen nickvergessen merged commit 8d114c9 into master Jan 26, 2024
76 checks passed
@nickvergessen nickvergessen deleted the techdebt/noid/move-integration-testing-to-actions branch January 26, 2024 13:37
@szaimen
Copy link
Contributor

szaimen commented Jan 26, 2024

🎉🎉🎉🎉🎉

@blizzz blizzz mentioned this pull request Mar 5, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mejo- mejo- mejo- approved these changes

@susnux susnux susnux approved these changes

@AndyScherzinger AndyScherzinger AndyScherzinger approved these changes

@ArtificialOwl ArtificialOwl ArtificialOwl approved these changes

@sorbaugh sorbaugh Awaiting requested review from sorbaugh

Assignees

@nickvergessen nickvergessen

Labels
3. to review Waiting for reviews technical debt
Projects
None yet
Milestone
Nextcloud 29
Development

Successfully merging this pull request may close these issues.
7 participants
@nickvergessen @susnux @szaimen @ArtificialOwl @AndyScherzinger @mejo- @blizzz