-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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
ci: move phpunit-nodb to actions #39734
Conversation
c671e18
to
79a085a
Compare
I did some work in this one: skjnldsv#13 |
ee8f13e
to
58c960c
Compare
.github/workflows/nodb.yml
Outdated
|
||
on: | ||
pull_request: | ||
paths: |
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.
The paths filter hopefully works like https://github.com/nextcloud/server/blob/master/tests/drone-run-php-tests.sh 🙈
Thanks for letting me know, looks very nice 👍 I added the paths filter and Also tried adding the run script to composer.json, but that didn't work (in the workflow) because phpunit-autotest.xml must be called from the tests/ folder because the paths are relative. |
58c960c
to
ae936e5
Compare
I’m not sure if you can use |
Thank you, good to know 👍 I don't mind the deprecation warnings, just documented them to explain the difference in the test outcome. |
Yes please, I tested in #39796 for 32 bits CI and it does work with ini-values. |
ae936e5
to
1f47fa9
Compare
Make sense 👍 I first thought you were referring to the PHPUnit deprecation warnings, but you wanted to let me know that setup-php uses the php.ini-production by default and therefore |
1f47fa9
to
0745186
Compare
Ready to review 🎉 |
paths: | ||
- '!**.json' | ||
- 'package.json' | ||
- 'package-lock.json' | ||
- '!**.sh' | ||
- '!**.yml' | ||
- '!**.xml' | ||
- '!**.php' | ||
- '!**/tests/**' | ||
- '!3rdparty' | ||
- '!apps/theming/css' |
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.
You have to use the quirk from https://github.com/nextcloud/.github/pull/230/files otherwise this executes way too often
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.
Good point.
I think the paths filter for the nodb-when-unrelated workflow is wrong anyway.
It only excludes paths, but does not include anything and will therefore never run 🙈
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.
but does not include anything and will therefore never run 🙈
Yeah, I debugged this last week in https://github.com/nickvergessen/turbo-lamp/pulls and the pattern in https://github.com/nextcloud/.github/pull/230/files is what we need to use. So you need to paths-ignore the same thing in unrelated you require in the actual job and then add the work around with the second exclude list, to make sure the fake summary does not run when the real summary should be used.
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.
What do you think about 44b5b7a?
https://github.com/marketplace/actions/paths-changes-filter is similar to our drone-run-php-tests.sh script.
The workflow is more complicated because we have a changes job, a conditional "main" job and the summary job with a check if the conditional job was required or not. This approach works without the when-unreated workflow.
I think not having two workflows is a good trade-off for the higher complexity.
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.
Examples
- Vue/Js change without phpunit: https://github.com/nextcloud/server/actions/runs/6364565736/job/17280917479?pr=40729
- 3rdparty bump: https://github.com/nextcloud/server/actions/runs/6364580850/job/17280944950?pr=40730
b92b257
to
55407bb
Compare
44b5b7a
to
c7a98a3
Compare
memcache and redis tests are skipped, not sure if we should ignore or configure those, or if they should be tested in their own job? |
I think memcache and redis have another job in drone. |
Yes but could they be added to a group that we add to the exclude-groups in https://github.com/nextcloud/server/pull/39734/files#diff-2c10cc6204050ae8a99af53744687db4d3690d7157685a9a221fba3bb78e6946R86 |
c7a98a3
to
b40b75a
Compare
Done. You already added the groups in 2d8e696 🎉 |
b40b75a
to
a67d110
Compare
Signed-off-by: Daniel Kesselberg <[email protected]>
Signed-off-by: Daniel Kesselberg <[email protected]>
Signed-off-by: Daniel Kesselberg <[email protected]>
Signed-off-by: Daniel Kesselberg <[email protected]>
Signed-off-by: Daniel Kesselberg <[email protected]>
a67d110
to
adad1e8
Compare
Replaced by #41003 |
Learnings
Learning 1
phpunit -c autotest-external
only runs tests for enable apps.This seems to make sense, but it is not obvious and does not make the CI configuration any easier.
The following apps are part of the server repository and are not enabled by default.
Therefore, they must be enabled for CI runs.
Learning 2
phpunit --list-tests
lists the available tests.Unfortunately, it does not list tests from dynamically enabled apps.
I assume that's related to the way the tests are added.
TODO
Migration
Round 1:
Warnings on Actions but not on Drone:
expectWarning is deprecated since PHPUnit 9.6
Round 2:
Round 3:
Round 4:
admin_audit encryption federatedfilesharing federation files_sharing files_trashbin files_versions provisioning_api user_ldap
(cf. https://github.com/nextcloud/server/blob/master/tests/enable_all.php)Actions: https://github.com/nextcloud/server/actions/runs/5777400665/job/15657527803
Drone: https://drone.nextcloud.com/nextcloud/server/38222/9/4
Round 5:
Actions: https://github.com/nextcloud/server/actions/runs/5777508182/job/15657761644
Drone: https://drone.nextcloud.com/nextcloud/server/38223/9/4
Round 6:
Actions: https://github.com/nextcloud/server/actions/runs/5777613755/job/15657986497
Drone: https://drone.nextcloud.com/nextcloud/server/38224/9/4
Checklist