Nightwatch testing against all opted in versions

#3397129: Allow modules to opt in to testing against Drupal previous major, previous minor, next minor → landed 👍

I think this can be worked on.

Done. This will make projects like decoupled_pages and ckeditor5_paste_filter (with only Nightwatch tests — see https://git.drupalcode.org/project/ckeditor5_paste_filter/-/tree/1.0.0-a...) a breeze to set up!

Really small nit.

Also, why would one of the nightwatch jobs runs for keycdn here?? https://git.drupalcode.org/issue/gitlab_templates-3397270/-/pipelines/43472

keycdn no longer has Nightwatch tests so that looks like a bug.

Because it seems that Moshe has chosen to adopt OPT_IN_TEST_PREVIOUS_MAJORin https://git.drupalcode.org/project/keycdn/-/commit/358ebe8277cf9373ff0eb... 😄

So why only nightwatch (previous major) and not also nightwatch? Because Moshe removed the Nightwatch test in https://git.drupalcode.org/project/keycdn/-/commit/48dbb2ea72ce2c96668c9... and … the rules I'm adding here were missing

    - exists:
        - tests/src/Nightwatch/**/*.js
        - modules/*/tests/src/Nightwatch/**/*.js

😇 Oops :P

P.S.: note that this is only happening because the phpunit job is assumed to always run unless one explicitly opts out by setting SKIP_PHPUNIT='1'. Arguably we should have similar exists logic for PHPUnit, then projects like Decoupled Pages or CKEditor 5 Paste Filter (which have only Nightwatch tests: https://git.drupalcode.org/project/ckeditor5_paste_filter/-/tree/1.0.x/t...), would no longer have to set SKIP_PHPUNIT.

I made a couple of comments in the MR.

The fact that I was rushing/multitasking is painfully clear 🙈

Really small change needed I think.

In addition to the MR feedback, we now have new reusable rules introduced here #3414391: Define extra rule to jobs that have "needs: composer" → that affect directly how these new jobs are run.

I don't think anyone is actively working on this right now, so I will fix the merge conflicts and incorporate any necessary additions from #3414391: Define extra rule to jobs that have "needs: composer" → . None of the contrib projects I main have any Nightwatch tests, so I may need some assistance in where to run tests. But first, I will get the conflicts resolved and address the MR feedback.

jonathan1055 → changed the visibility of the branch main to hidden.

I have merged in 'main', resolved the conflicts, and made changes to use the new rule snippets. But the target of the existing MR78 is still 1.0.x. Can someone with access edit the MR the target to 'main', please?

@wimleers from your comment in #10 I have opened #3419008: Add logic to avoid running PHPUNIT if the project has no phpunit tests →

@Wim Leers - as per your comment in the issue summary

Acceptance criteria: all customzied and newly added jobs in decoupled_pages/-/blob/8.x-1.x/.gitlab-ci.yml disappear and are replaced with a number of OPT_IN_* variables!

I have created a MR to test this - https://git.drupalcode.org/project/decoupled_pages/-/merge_requests/3

To assist readability and maintainability, I have moved all the Nightwatch job variants together into one block. Having each variant interspersed with the similar phpunit variant made it harder to check all nightwatch jobs together. Plus the merge conflicts were tedious to resolve. This should reduce future conflicts.

It also allows easy comparison between the nightwatch block of variants and the corresponding phpunit variant. And there is one difference. 'Nightwatch previous minor' has the variable _TARGET_PHP: $CORE_PHP_MIN which the 'Phpunit next minor' does not have. See screengrab. Was this added intentionally when first doing the new jobs? First thought would be that either they both need it or neither of them do.

I know it's not in the "Needs review", but wanted to see the progress.

Keycdn does not have nightwatch tests and we can see how "max php" and "previous major" nightwatch tasks are run (and fail because there are no tests). It's weird because I see the "exists" rules, but it's somehow happening.

Is this the pipeline you are referring to?
https://git.drupalcode.org/project/keycdn/-/pipelines/87722
I will investigate further.

Correct. That's the one.

I easily replicated the problem in Scheduler, which also has no nightwatch tests. The 'current core' nightwatch test was not run as expected, but when I set opt_in_ for any other variant the unrequired job was added to the pipeline and failed. But I've found the problem, and it was a simple coding error. The variant jobs all had:

    - exists:
        - tests/src/Nightwatch/**/*.js
        - modules/*/tests/src/Nightwatch/**/*.js
    - when: on_success

but the presence of that final '-' meant that it was the start of a new condition, and it was always true. Just needed to remove that '-' because the 'when: on_success' should be part of the 'exists' condition, like this:

    - exists:
        - tests/src/Nightwatch/**/*.js
        - modules/*/tests/src/Nightwatch/**/*.js
      when: on_success

Staring at the code it is very easy to see the '-' and not realise that it be there :-)

I can also see that most of the other jobs that have 'exists:' such as eslint, stylelint, pages also have '- when: never' as a final condition. This is unnecessary, and can add confusion, because it gives the impression that it does something, when in fact it never will. If the file exists then the 'exists' condition will cause the job to be added. If the file does not exist, then there will be no matching clauses and so the job wont get added. The final '- when: never' is redundant in these jobs. I think it would be good to remove them, maybe in another issue, not to get scope creep here.

I re-ran the pipeline on MR3 for decoupled_pages 📌 Remove customised nightwatch jobs from .gitlab-ci template Needs review . All OK

Ready for review. Can you re-try keycdn? Simpler than me opening a MR for it, as I can't run pipelines on that project directly.

Agree that if the - when: never is not needed in some places, it should be addressed in a follow-up issue.
Reviewing this and running pipelines.

Some really minor feedback in the MR.

Pipelines running for keycdn in the last iteration of the MR here: https://git.drupalcode.org/issue/gitlab_templates-3397270/-/pipelines/92480

I have addressed both MR thread comments. Ready for review.

Scheduler pipeline with opt_ins turned on but no Nightwatch test files. No jobs attempted.
https://git.drupalcode.org/project/scheduler/-/pipelines/92943

Scheduler pipeline with a dummy nightwatch file added. The jobs are added, showing that the reusable rule is working.
https://git.drupalcode.org/project/scheduler/-/pipelines/92946

As we are introducing new jobs, we need to update the table in the README.md file to reflect the jobs and when they will be run or not:
https://git.drupalcode.org/project/gitlab_templates#when-do-the-jobs-run

I have added the new jobs into the README table. Also took the oportunity to make two other improvements to the table:

(1) the first column was being pushed narrow due to the long unbroken conditions of the composer job. This meant that the job names were being wrapped and not so readable. So I added a fix break in those two conditions so that the column widths are more even. Not only does this prevent the job names being wrapped it also means better use of the available screen and more rows shown.
Before

After

(2) The table had two header rows, as above. The first row only had 'Rules' then the second header was treated like an ordinary row. Better to have one header with more detailed text.

I know these are a little out of scope, but while I was editting the table I thought it might as well be done here, does not need a separate issue.

I have also moved the nightwatch-tests-exist-rule rule as requested in the MR and added the comment about 'exists:'
Ready for review.

This looks great. RTBC.

Merging to "main" shortly.

Made one more change to README, with prior agreement on slack. Removing most of the hard <br> makes better use of the screen, not pre-judging what size or orientation the users screen is.

What we currently have

With latest change - job rows shown increases from 15 to 20 for the same size screen.
Also the sets of conditions are easier to read.

I think the readme changes look great. Thanks so much for the attention to the detail!

Merged, marking this issue as fixed. Note that as this includes new features the tag next tag will jump to 1.2.0.

Thank you so much, @jonathan1055, for pushing what I started across the finish line! 🤩🙏 Happily merged your MR for 📌 Remove customised nightwatch jobs from .gitlab-ci template Needs review 🚀

P.S.: @fjgarlin, in the future, please transfer credit from me to somebody who finished what I started — I wish @jonathan1055 was credited as the primary author in the git commit history 😇

Never sure which one to choose when somebody starts the work and then another person takes it to the finish line.
I wish we could do multiple assignees for the commit itself.

The Drupal-credit does not depend on commit author or commit message, but yeah, I get what you mean. In any case, I love that you started the idea and that Jonathan took it to the next level, so big thanks and big kudos to BOTH!

It feels like I started something kinda tricky but then @jonathan1055 took it to a WHOLE NEW LEVEL! 😄

Automatically closed - issue fixed for 2 weeks with no activity.