ci: move phpunit-nodb to actions

[kesselb]

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.

admin_audit
encryption
files_external
user_ldap

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

• CI
• Decide: Run nodb tests for PHP 8.0, 8.1 and 8.2 or only PHP 8.0 (like on drone)
• Review
• Follow-up to remove nodb from drone
• Follow-up to exclude PRIMARY-azure,PRIMARY-s3,PRIMARY-swift
• Follow-up to add php 8.3 (after Allow PHP 8.3 #40630)
• Ask Côme if the additional apps should be enabled for phpunit-32bits too

Migration

Round 1:

• No changes to test suite

	Actions	Drone
Tests executed	10554	10861
Warnings	28	25
Skipped	76	96
PHPUnit	9.6.10	9.5.28

Warnings on Actions but not on Drone:

26) OCA\Theming\Tests\IconBuilderTest::testGetFaviconNotFound
Expecting E_WARNING and E_USER_WARNING is deprecated and will no longer be possible in PHPUnit 10.

27) OCA\Theming\Tests\IconBuilderTest::testGetTouchIconNotFound
Expecting E_WARNING and E_USER_WARNING is deprecated and will no longer be possible in PHPUnit 10.

28) OCA\Theming\Tests\IconBuilderTest::testColorSvgNotFound
Expecting E_WARNING and E_USER_WARNING is deprecated and will no longer be possible in PHPUnit 10.

expectWarning is deprecated since PHPUnit 9.6

Round 2:

• Exclude PRIMARY-azure,PRIMARY-s3,PRIMARY-swift (they are skipped anyway because the configuration is missing)
• Added redis extension

	Actions	Drone
Tests executed	10524	10861
Warnings	28	25
Skipped	46	96
PHPUnit	9.6.10	9.5.28

Round 3:

• Added ldap extension
• Print list of tests to be executed

	Actions	Drone
Tests executed	10524	10861
Warnings	28	25
Skipped	46	96
PHPUnit	9.6.10	9.5.28

Round 4:

• Enable additional apps: 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	Drone
Tests executed	10831	10861
Assertions	23081	23020
Warnings	28	25
Skipped	46	96
PHPUnit	9.6.10	9.5.28

Actions: https://github.com/nextcloud/server/actions/runs/5777400665/job/15657527803
Drone: https://drone.nextcloud.com/nextcloud/server/38222/9/4

Round 5:

• Include test suite PRIMARY-azure,PRIMARY-s3,PRIMARY-swift again because this cleanup should be done as follow up.

	Actions	Drone
Tests executed	10861	10861
Assertions	23081	23020
Warnings	28	25
Skipped	76	96
PHPUnit	9.6.10	9.5.28

Actions: https://github.com/nextcloud/server/actions/runs/5777508182/job/15657761644
Drone: https://drone.nextcloud.com/nextcloud/server/38223/9/4

Round 6:

• Squashed the commits
• Removed the --list-tests step

	Actions	Drone
Tests executed	10861	10861
Assertions	23081	23020
Warnings	28	25
Skipped	76	96
PHPUnit	9.6.10	9.5.28

Actions: https://github.com/nextcloud/server/actions/runs/5777613755/job/15657986497
Drone: https://drone.nextcloud.com/nextcloud/server/38224/9/4

Checklist

• Code is properly formatted
• Sign-off message is added to all commits
• Tests (unit, integration, api and/or acceptance) are included
• Screenshots before/after for front-end changes
• Documentation (manuals or wiki) has been updated or is not required
• Backports requested where applicable (ex: critical bugfixes)

[skjnldsv]

I did some work in this one: skjnldsv#13
If you want to have a look if anything is worth taking from

[kesselb]

The paths filter hopefully works like https://github.com/nextcloud/server/blob/master/tests/drone-run-php-tests.sh 🙈

[kesselb]

If you want to have a look if anything is worth taking from

Thanks for letting me know, looks very nice 👍

I added the paths filter and when-unrelated workflow, hopefully it will work.

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.

[come-nc]

I’m not sure if you can use ini-file: development along with ini-values: but if yes it would make sure the tests are running without php deprecation warnings.

[kesselb]

I’m not sure if you can use ini-file: development along with ini-values: but if yes it would make sure the tests are running without php deprecation warnings.

Thank you, good to know 👍

I don't mind the deprecation warnings, just documented them to explain the difference in the test outcome.
Do you want me to set ini-file to development?

[come-nc]

I’m not sure if you can use ini-file: development along with ini-values: but if yes it would make sure the tests are running without php deprecation warnings.

Thank you, good to know +1

I don't mind the deprecation warnings, just documented them to explain the difference in the test outcome. Do you want me to set ini-file to development?

Yes please, I tested in #39796 for 32 bits CI and it does work with ini-values.
The CI should run without deprecation warnings (so it should fail on those if there are any).

[kesselb]

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 error_reporting = E_ALL & ~E_DEPRECATED & ~E_STRICT.

[kesselb]

Ready to review 🎉

[nickvergessen]

You have to use the quirk from https://github.com/nextcloud/.github/pull/230/files otherwise this executes way too often

[kesselb]

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 🙈

[nickvergessen]

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.

[kesselb]

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.

[kesselb]

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

[come-nc]

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?

[kesselb]

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.
The memcache and redis tests are also skipped on drone.

[come-nc]

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. The memcache and redis tests are also skipped on 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
That would clean up the output of the job, making it easier to see failures/warnings.

[kesselb]

Yes but could they be added to a group that we add to the exclude-groups in #39734 (files)
That would clean up the output of the job, making it easier to see failures/warnings.

Done. You already added the groups in 2d8e696 🎉

[kesselb]

Replaced by #41003