[Meta] Perform set-up tasks in Browser tests using API calls rather than browser requests

Just done 📌 ContentTranslationContextualLinksTest should use API calls to set up translation Fixed and in the process of looking around language and translation tests, I found a LOT of tests that need this work!

We generally don't do per-file/module issue splits anymore unless there's a good reason to.

I would argue there is a good reason to here. Most of the tests are going to be unrelated to each other and the changes aren't just a matter of using a new coding style. Reviewing many different tests in a single issue will lead to review fatigue. Some tests may be harder to refactor than others and the sooner we can get some of these in the earlier we can reap the benefits. On the other hand, optimising all of them in one issue will make it easier to measure the impact.

See https://www.drupal.org/docs/develop/issues/issue-procedures-and-etiquett... → and @longwave's comments on 🌱 [meta] Use FileUrlGenerator::generate() everywhere, then deprecate generateString() and generateAbsoluteString() Active .

Given it's probably going to be me who does most of these (I mean, if people beat me to it, great, but I'm not counting on it), I've filed issues in the way that makes sense to me. I'm not going to have the brainpower to work on multiple tests at the same time.

As @mstrelan points out, some issues are going to require discussion and some will be straightforward. It was tedious enough filing issues for all of these (that the 'clone issue' link doesn't work as expected made it harder) without having to think about how to group the simple ones together and leave the complex ones separate.

I'm just pointing it out before you do the work, your issues might be closed by core maintainers or they might not, can't say.

Also, I can't help but point out that since you're going with the carbon footprint argument here, lets assume that this saves 0.1% of the total test execution(which I think is very generous) but at the same same time do it across 20 issues, it will require an extra 19'000 test runs to just break even on the actual carbon footprint of doing this change, assuming that each issue only needs a single test run. And that doesn't even consider the extra on-commit test runs, git commits, issues and comments ;)

For this the issue scoping is a bit of a grey area because the changed might be quite different, or they might be nearly identical - a huge patch might be too much, but individual issues might be too repetitive. A possibly half-way would be to do A-F, G-N, O-Z sort of thing too.

Could we try this - maybe do two individual patches in the existing issues to see how it goes, and we can commit them one by one regardless of scoping for the other issues, and figure out the rest once we've done those.

I'm reviving 📌 Add an automated way for slow tests to fail drupalci builds Needs work which should help to identify priorities here - i.e. NodeTranslationUiTest has an individual test method that takes 55 seconds on my machine and the whole test class takes 4 minutes. That's not necessarily the worst one, just picked it from the @group #slow.

Cross-posting my review from 📌 Use the API to set up languages in tests that are not specifically testing the language form Fixed :

I agree that the language configuration form is not what's under test, so it makes sense to do this with the API.

That said, there's also an entire freaking container rebuild after these lines, which is even more expensive. Makes one question if this should be done inside the test method at all?

It's also super a lot of API calls in a row for a single test method, and something that it seems like a single test trait or the like should provide.

Finally, I reviewed a different issue that was around lines with the same form-based translation setup in a test of not-the-translation-form elsewhere. In fact, core uses this exact pattern in 26 other places:

[ayrton:maintainer | Fri 16:18:56] $ grep -r "admin/config/regional/content-language" * | grep "drupalGet" | wc -l
27
So, if we're fixing this one spot, we should probably fix all the rest as well, in a consistent way. We should check whether there is an existing trait to set up a language via the API with a single method call, and if not, add one. We also should have that happen in setUp() rather than individual test methods. (We might need to split up some tests if there are other methods in the same class not requiring a language.)

NW to expand the scope of this. Thanks!

@joachim pointed out that the container rebuild happens regardless, so maybe we need a separate issue to make the container multilingual much earlier in the test setup process. However, one child issue per test for tests that all have the exact same problem of using the translation form via the UI to set up languages is not okay scoping. At a minimum, a test trait should be created in one issue and the tests with the same form problem converted in bulk in a separate issue.

Please close all the children doing this exact same thing WRT the language form as duplicates. I have already rescoped 📌 Use the API to set up languages in tests that are not specifically testing the language form Fixed to be the issue to address this. Thanks!

> However, one child issue per test for tests that all have the exact same problem of using the translation form via the UI to set up languages is not okay scoping.

I chose to scope it like this because when I was working on this, I imagined that it would mostly be me working on the issues. I did not want to be running one branch for AGES while I work on one test after another, with the risk of the work done so far rotting, and then having to potentially discuss all the different changes in the different tests. It felt like a too large piece of work.

@Berdir brought the same thing up as you did @xjm and frankly the nitpickiness totally demoralised me. I'm glad at least that other people have stepped up and done some of the work.

Another candidate, second longest test remaining in core: 📌 Try to optimize CKEditor5Test Needs review