Add support for 'test only' changes to gitlab CI

Added screenshot to issue summary

Would never have believed this would be easier than patches. There will be cases that this doesn't cover, but it should cover 90% of issues where we'd ask for a 'test only' patch or more than that.

Yeah this is also better on the $$ than patches because we just run the changed tests and not the whole suite.

Pushed some changes to split out how I revert deletions and modifications from additions using diff-filter.

The cases it won't catch are nightwatch tests. Still thinking about how to handle that, but they're nowhere near as common.

Sample pipeline showing this works - https://git.drupalcode.org/project/drupal/-/pipelines/19253

MR currently has patch from 🐛 "Enforced" Dependencies of Optional Configs Overwrite Other Dependencies RTBC as a proof of concept

I guess we postpone this on the parent?

Let's postpone, but also try to get it in asap once the main issue lands. The output looks really good!

Wonder if this could include a CR or note how to do a test-only in gitlab? is it automatic?

Since it was merged, we need a rebase since this includes some changes that now live in core.

Answering #10. Yes, the idea is that it is automatic. No need to upload test-only branches or patches, the Gitlab CI job will do it for you (and the reviewers).

Yeah there'll be a button to press on the pipeline UI to get the test only run, but no extra MR to create, no patch to upload etc.

That. Is. AMAZING!!

So it just automatically picks up test files?

Yes so what the new pipeline actually does is:

1. For every change that's not to an test file, checks it out from origin.
2. Lists the remaining files - which is just the tests now.
5. Runs phpunit against those files - so if there is one method addition to a test class, it'll run that test class, if a patch updates 15 tests, it runs those 15.

It's an individual pipeline job which is manually triggered alongside the other pipeline jobs, and it is set not to cause the entire pipeline to fail. So you can get a passing pipeline, + a test only job which you manually start, and when it fails, that means the tests failed.

There are occasionally issues where this approach won't work - say you need half a change to make the tests run at all but omit something else to show the test failure, but those are pretty rare and we could handle it ad-hoc issue by issue - possibly with extra 'draft' MRs, possibly something else - like reverting a commit, running a pipeline, taking a screenshot, reinstating the change.

Rebased on top of 11.x and removed the code from 🐛 "Enforced" Dependencies of Optional Configs Overwrite Other Dependencies RTBC

I think this is a great start. We might need to consider that there might be changes in something like composer.json. What is a change is also adding/updating a package since that broke. Then we need to composer install. I don't think we'd want to do this manually, but perhaps some sort of check for that is a good idea.

This could also be a follow up, to not run tests if [insert list of files] is changed.

Addressed reviews and fixed the out of date partial names

Don't want to interrupt the work here but may need a rebase? But there are some open threads from what I can tell.

Picked 🐛 Generic Revision UI's Revision overview page generates wrong operations/view links for a translation Fixed for testing

Got a failure when I triggered the test-only build

fatal: ambiguous argument 'origin/11.x': unknown revision or path not in the working tree.

Think I've fixed that, didn't revert 🐛 Generic Revision UI's Revision overview page generates wrong operations/view links for a translation Fixed yet

Still not working, something wrong with the bash commands, pausing for today

https://git.drupalcode.org/project/drupal/-/jobs/102286 didn't checkout the non test changes so the tests passed

Do we have a better way of identifying tests? Not all of them end in Test.php, although maybe we should have a policy and test coverage to ensure this is consistent.

Do we have many tests in core that use the @test annotation? Short of that I thought the file name had to end in Test.php to be found by phpunit?

Yeah discovery looks for Test.php so I think everything actually does end with that, or at least the class name has to finish with Test which is the same thing.

I think I fixed the test-only script part.

https://git.drupalcode.org/issue/drupal-3386566/-/jobs/105712

Looks like this might need review again?

I see that the pipeline failed in a few spots. Kernel, FunctionalJavascript.

Also looking at the test-only spot the failures it's reporting I don't believe are from this MR. HttpExceptionNormalizerTest was not something that was changed?

The last pipeline fails were widespread across all MRs, with GitLab not being able to spin up runners, those should be fixed already.

I rebased the branch. The pipelines are running again.

Regarding the files, if the branch is not rebased it can show different files which are not in this branch but made it to the HEAD of 11.x.
I added an informative message after listing the "changed" files prompting users to rebase the branch if this happens.

Back to Needs Review (will keep an eye on the pipeline).

Regarding the files, if the branch is not rebased it can show different files which are not in this branch but made it to the HEAD of 11.x.

This also seems like a feature for making sure that changes won't cause test failures against HEAD :)

So the pipeline is showing all green, tests are showing all green.

When I run the test-only though I get errors https://git.drupalcode.org/issue/drupal-3386566/-/jobs/124477 from the last ticket to be merged.

I think it just needed a rebase, MRs aren't like patches in so far as they don't apply to HEAD, they apply to the point in time

This looks good to me, there's no test changes in the branch so we can't check the latest MR's output (and I realised this after pushing the button, not before), but there are successful examples from earlier in the issue. Once it's merged we can try it out on core MRs and will find any issues that way, it's only running on-demand so shouldn't introduce instability.

Fantastic to see this working, we might need some tweaks in followups but let's see how we get on with this - and hopefully it should make it much easier than uploading separate test-only patches.

Committed and pushed 3f49e8cdca to 11.x and d60aab72b3 to 10.1.x. Thanks!

This is awesome!

However, it needs:

1. To be documented in the hanbdook somewhere, with the screenshot from the IS (probably along with everything else about GitLab CI for issue users, like the little green check/red x, how to find the specific results of failed jobs because half the time a text search gets "stuck" in the results output and there's no clear build status with a fail message that I can find, etc. etc.)
2. To be added to the core gate item about test-only patches, linking the above docs.

We need an issue, not sure if one is open or not, for better failed result summaries. i.e. at least at the job level but ideally at the pipeline level we should be able to show which tests failed and with what message etc. Right now I am relying on ctrl-f. Keep forgetting to search/open issues though.

In reference to #26 we need to fix this issue 📌 Rename test classes not ending with Test to end with Test Needs work

catch in#28:

> so I think everything actually does end with that, or at least the class name has to finish with Test

That is still not the case there are still test classes without ending with Test.php

Agreed with #41; despite all the fast simplicity of GitLab CI, I find it really difficult to actually see the relevant results.

I opened 📌 [GitLab CI] Improve failure reporting summary Active .

I have created 🐛 Remove variables export from test-only job Needs review , as it seems like that we have reintroduced variables export in this commit.

      if [[ $(git diff refs/heads/${TARGET_BRANCH} --diff-filter=DM --name-only|grep -Ev "Test.php$"|grep -v .gitlab-ci|grep -v scripts/run-tests.sh) ]]; then

We are filtering *Test.php.
IMHO we should use */tests/* instead. When we move this to the gitlab contrib templates, I've used to have mock data too in the tests folder.
This might actually be relevant for core too if there are fixes to e.g. Upgrade tests with db dumps.

Didn't create a new issue, but we might if you agree?

Came here to comment on that as well. test-only tests often also require changes to/new test modules and other information that is most of the time found in tests folders. I guess there are exceptions to that, but they are probably pretty rare?

That seems like a good idea to me too, will catch more changes that way.