Move helpers in entity_test.module and delete it

Looks like need to split this to several issues.

• move entity_test_create_bundle to EntityTestHelper::createBundle
• move entity_test_entity_types to EntityTestHelper::createBundle, convert cont to enum.

Reviewed a bit, I'd suggest to postpone this on the two child issues, we won't be able to remove the .module file due to the BC layer anyway though.

MR rebased

The children are in this can be updated.

We can't remove the deprecated but we can make it easier in the future.

MR updated

Verified that there's nothing left in entity_test.module except the two deprecated functions.

I'm not sold yet on the Callbacks class/pattern used here in general, but for this module, it's an existing pattern that's just used here too.

Yeah I don't love the callbacks either but also can't get overly concerned about them considering this is test modules.

catch → closed merge request !10677

A long shot but could these helpers be committed on D10 or how do we use them in contrib to support D10? I tried copying search_api in 🐛 Fix reliance on deprecated .MODULE.inc files for hooks Active inside og but found D10 didn't have them https://git.drupalcode.org/project/og/-/jobs/4465178

DeprecationHelper is the standard way https://www.drupal.org/node/3379306 →

@nicxvan What should I do when the class doesn't exist, yet like in this case in my test for this change 📌 Move entity_test_create_bundle from entity_test.module Needs work ?

@joelpittet: A function/method exists check for either the old or the new one, combined with a phpstan-ignore-next-line for the BC path.

FWIW, this is exactly why I think having phpstan set to not allow fail is a a problem, especially on next minor. This is a deprecation, an early warning that this will stop working in ~2 years. There is absolutely no reason to break CI now and disrupt contribution workflows. You can disable phpstan on next major or allow it to fail. See https://git.drupalcode.org/project/diff/-/blob/2.x/.gitlab-ci.yml?ref_ty....

If phpstan is used purely as a code quality tool it's one thing, and it's also different in custom projects where you only care about one specific core version, but considering that we also use phpstan as this early-warning-system for deprecations, it is IMHO just not a good fit. Maybe there could be separate jobs that filter things depending on whether or not it's a deprecation message. But that's getting way off topic.

@berdir, Thanks for your insight! I ended up just copying the helper method into the test class that was using it, bypassing both the original helper function and the missing helper class altogether. It might come back to bite me later, but that’s a future problem. 😉 If the phpstan on next major causes too much of this, I will do as you say and allow fails, though gotta feel the pain first.

Your decision, but I think that's the wrong call. Those helpers are only one side of the coin. The other side is \Drupal\entity_test\Hook\EntityTestHooks::entityBundleInfo(). If you only copy one side, then if the implementation changes then it will break. Those two are tightly coupled.

My suggestion isn't that complicated, it's essentially something like this:

            if (method_exists(Error::class, 'logException')) {
              Error::logException(\Drupal::logger('pathauto'), $e);
            }
            else {
              /* @phpstan-ignore-next-line */
              watchdog_exception('pathauto', $e);
            }

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