[meta] Replace calls to ::expectError*() and ::expectWarning*()

Dizzy bot? The MR is mergeable.

#11 what we need here is to stop using TestCase::expectError*() methods. If you don’t find others, it should be ok.

Yeah the problem in some of the cases here is notifying ‘somehow’ that an error occurred. So I think we have the following options:

1. We keep throwing user errors, and we abide to PHPUnit 10 vision that is no longer going to test on such errors by expecting them. We do not implement a gap fill to expect errors. Therefore we drop any test that expects an error.
2. We keep throwing user errors, and we fill the gap vs PHPUnit 10 using our own error handler to test error expectations. We keep the tests as they are now. This is currently done in #3417066: Upgrade PHPUnit to 10, drop Symfony PHPUnit-bridge dependency.
3. We stop throwing user errors, and again abide to PHPUnit 10. We replace the user error, where we expect the user to be informed, with a log call like \Drupal::service(‘logger something’)->error(‘whatever’). Assuming the container is available.
4. ?

Each of the cases here will probably need to have its own solution.

Thoughts?

#16 yes, but I’d be more interested on reviews and input on #15 ATM.

IMHO, I would prefer NOT to reimplement expectError*() methods.

Between the lines, I read PHPUnit 10 approach as one where using trigger_error in SUT code is no longer seen as a good practice. We might debate whether this is right or not, but that’s OOS here.

So what is the practical effect of the PHPUnit 10 change is that triggering E_USER_ERROR in SUT is not expectable in test and therefore FAILS the test itself. In other words, triggering an error should not be in any covered code path. That’s a bold assumption, with its own meaning i.e. that you should develop code where triggering errors is really exceptional. If we backfill the expectation we’re swallowing the assumption.

Possibly we could take a different stance for expectWarning* but that’s for another issue.

#20 fine, but IMHO we need first to establish whether we will have expectError*() methods as our own stuff, or not. I am for NO.

OK, not sure about a meta, for the moment I created two spin-offs, #3419690: Find an alternative to trigger_error in Drupal\Core\Database\Query\Condition::compile and #3419693: Log error instead of trigger_error in Drupal\Core\EventSubscriber\RedirectResponseSubscriber::checkRedirectUrl, that are tackling a couple of cases that were reverted from the MR here.

I think we still need this MR to fix the clear bugs (for example, Statement objects trigger E_USER_WARNING but the test expects errors), and to cleanup the deprecation and warning ignores.

#24 thanks. For completeness: let's differentiate between triggering E_USER_ERROR and triggering E_USER_WARNING, though. PHPUnit 10 drops expect* methods for both. I'm on a NO to reintroduce expectError*() methods as our own implementation; more open to keep expectWarning*()methods.

See #3417066: Upgrade PHPUnit to 10, drop Symfony PHPUnit-bridge dependency for the current status of reimplementing deprecation, warning and error expectations, that were dropped in PHPUnit 10 or for which Symfony's PHPUnit-bridge has not come up with a solution yet. Decisions here will impact how much fill-gap code we will have to write and maintain.

Adjusted IS with latest direction discussed in Slack.