Move helpers in ajax_forms_test.module and delete it

Ready for review.

It's a test but wonder if we should start using a Callback folder?

Maybe a conversation for another issue. Since there are so many modules in system I just all the ones that started with ajax and these were the ones that needed conversion.

Any reason why these can't be static methods?

You are right.
It looks like most of the callbacks methods should be converted to static.

Existing methods at Callbacks are not static, this is the reason I decide to use non static methods, to keep code consistent, I think we can create follow up to convert methods to static and add return type hint. I revert recent changes I made. And set back to NR.

Since we are dealing with just tests that think it could be in scope of during the conversion to make static here vs another issue.

All methods at Drupal\ajax_forms_test\Callbacks converted to static.

Thanks, looks good to me.

Only used internally to the test module so we don't need to worry about a bc layer. Approach looks fine to me. Committed/pushed to 11.x, thanks!

catch → closed merge request !10676

Looks like this broke HEAD:https://git.drupalcode.org/project/drupal/-/jobs/4115231

Reverted for now.

nicxvan → changed the visibility of the branch 3495959-ajax_forms_test to active.

nicxvan → changed the visibility of the branch 11.x to hidden.

nicxvan → changed the visibility of the branch 3495959-ajax_forms_test to hidden.

I don't think I have permission to reopen the MR so I pulled it down, rebased on 11.x and pushed up a copy so I could create the MR.

Since I was doing that I also reviewed the MR. If it's green I think I can RTBC since the rebase was clean and I didn't write this one.

@nicxvan, just ask in #core-development for help re-opening an MR. Someone is often around.

Why did this break HEAD and has it been fixed?

@quietone not sure but it may have just been a cross-commit with something else where they passed individually and failed when combined. We have a lot of 1+ month MRs in the RTBC queue so the chances of that happening are quite high. It happened with at least one other issue too. When we're using gitlab issues, we'll be able to set up merge trains so that each merge gets a full pipeline run before it actually goes in against the current state of HEAD.

Since the rebase is green, going to try this again but also keep an eye on gitlab in case we need to revert again.

catch → closed merge request !10993

HEAD seems to be broken again with 38de8a6c.

Where are you seeing that?

In my MR for another issue: https://git.drupalcode.org/issue/drupal-3416819/-/pipelines/405059/test_...

Also testing locally latest 11.x fails. I have pushed up MR with the fix, but it's concerning that tests were passing earlier.

Pushed up MR 11001 with the fix. The test only job is passing erroneously, which is probably 🐛 Functional Javascript test false postive and missing browser output Active .

Oh good catch, went was it passing without it though?

Oh good catch, went was it passing without it though?

Yes, it's very concerning if functional javascript tests have false negatives on MR builds.

@godotislate I experienced this on another issue and @berdir had a solution.

I did another careful review of the merged MR and confirmed that this was incorrectly converted, and that MR 11001 fixes that.

RTBC

Also @quietone, I'll keep that in mind in the future! Honestly it really was not trouble, it's just a couple of extra commands to create a new branch and push it up.

I mad MR 10993. MR 11001 looks like good fix. @godotislate thanks.
+1 for RTBC

Yes, it's very concerning if functional javascript tests have false negatives on MR builds.

I think this false positive. Tests passes but it should fail.

@nicxvan thanks for double check original MR, so do I and also found nothing.

catch → closed merge request !11001

Bumped 🐛 Functional Javascript test false postive and missing browser output Active to critical, tests were passing multiple times both on here and branch runs when they should have failed.

Committed/pushed the latest MR to 11.x, thanks for tracking it down.

Thanks!

Godotislate should be credited I think though.

Ahh sorry too many things, also didn't mark it fixed...