Title: Add Drupal 12 support

Problem/Motivation

3.0.x declares ^10.3 || ^11.0. There is no automated
compatibility issue for this project, so nothing has flagged it.

Four separate things are in the way, and only the first is the sort of change
a version bump implies.

1. The token hooks throw

Drupal 12 no longer loads MODULE.tokens.inc before invoking the
hook. It registers dfp_tokens from the file scan and then cannot
resolve it:

InvalidArgumentException: Class "dfp_tokens" does not exist.
  in Drupal\Core\DependencyInjection\ClassResolver->getInstanceFromDefinition()

This is not only a test failure. It is any request that replaces a token, once
dfp is installed. Core made the same move and now ships
no .tokens.inc files at all — every one became a
*TokensHooks class, for example NodeTokensHooks.php.

2. HtmlResponseAttachmentsProcessor gained an argument

Constructing HtmlResponseAttachmentsProcessor without a file url generator is
deprecated in drupal:11.4.0 and the argument will be required in drupal:12.0.0

DfpHtmlResponseAttachmentsProcessor subclasses it and does not
pass one, so the parent falls back to \Drupal::service(). In the
unit tests, where there is no container, that is a
ContainerNotInitializedException.

3. The test suite is not discovered

MissingGroupException: Missing group metadata in test
Drupal\Tests\dfp\Functional\AlterTagTest::testDfpTagAltering

Drupal 12 reads the PHPUnit Group attribute rather than the
@group annotation, so nothing runs at all. Separately, the
@dataProvider annotations in TagViewTest are ignored
by PHPUnit 12, which fails the four tests that use them with
ArgumentCountError.

4. submitForm() and TranslatableMarkup

TypeError: WebAssert::buttonExists(): Argument #1 ($button) must be of type
string|int|float|bool, Drupal\Core\StringTranslation\TranslatableMarkup given

Proposed resolution

  • Move the token hooks into src/Hook/DfpTokensHooks.php, keeping
    the procedural functions behind LegacyHook so 10.3 and 11 are
    unaffected.
  • Pass the file URL generator through to the parent constructor, and add it to
    the service definition and the unit test. Passing the extra argument is
    harmless on older core — PHP ignores it.
  • Add the Group attribute to all five test classes and
    RunTestsInSeparateProcesses to the functional ones. Add
    DataProvider attributes alongside the existing
    annotations rather than replacing them: PHPUnit 9 on Drupal 10 reads only
    the annotation, PHPUnit 12 only the attribute. Dropping the annotation
    breaks Drupal 10.
  • Pass a plain string as the submit button label.
  • core_version_requirement: ^10.3 || ^11 || ^12, in
    dfp.info.yml and the test module.

Remaining tasks

Review. 27 of 27 tests pass on 10.6.15, 11.4.5 and 12.0-dev with the above.
The phpcs and phpstan findings that remain — two "String concat is not
required" in the test files, and some new static() and entity
storage warnings — all predate this and are untouched.

Worth noting separately: the repository's default branch is
8.x-1.x, while releases come from 3.0.x. Anyone
cloning lands on the wrong branch.

Comments

marcelovani created an issue. See original summary.

  • marcelovani committed 31c96ca2 on 3619663-drupal-12
    Issue #3619663 by marcelovani: Make the blocking lint jobs pass
    
    phpcs...

  • marcelovani committed d37b5c65 on 3619663-drupal-12
    Issue #3619663 by marcelovani: Turn the next major CI jobs on...
marcelovani’s picture

>MR !46 >
is ready. Could another maintainer review? I would rather not merge my own
work here.

Four separate things were in the way, and only the last is the sort of change
a version bump implies.

The token hooks throw

Drupal 12 no longer loads MODULE.tokens.inc before invoking the
hook. It registers dfp_tokens from the file scan and then cannot
resolve it:

InvalidArgumentException: Class "dfp_tokens" does not exist.

This is not a test failure — it is any request that replaces a token, once dfp
is installed. Core made the same move and ships no
.tokens.inc files at all now, every one having become a
*TokensHooks class. The implementations move to
src/Hook/DfpTokensHooks.php and the procedural functions stay
behind LegacyHook, so 10.3 and 11 are unaffected.

The parent constructor gained an argument

HtmlResponseAttachmentsProcessor::__construct() takes a file URL
generator, required from 12 and deprecated without since 11.4.
DfpHtmlResponseAttachmentsProcessor subclasses it and did not
pass one, so the unit tests died with
ContainerNotInitializedException. Passing the extra argument is
harmless on older core — PHP ignores it.

The suite was not discovered

MissingGroupException, because 12 reads the
Group attribute rather than @group. Nothing ran at
all. Five classes gained it, and the functional ones
RunTestsInSeparateProcesses.

TagViewTest needed DataProvider attributes
alongside its annotations rather than instead of them: PHPUnit 9 on
Drupal 10 reads only the annotation, PHPUnit 12 only the attribute. Replacing
them passes on 12 and breaks 10 — I did that first and only caught it by
running 10.

Results

  • 10.6.15 — 27 pass
  • 11.4.5 — 27 pass
  • 12.0-dev on PHP 8.5.8 — 27 pass

Before this the suite did not run on 12 at all.

Two other changes worth calling out

phpcs and phpstan are allow_failure: false here, so findings that
predate this branch were turning the pipeline red. They are fixed rather than
left: three "String concat is not required" in the test files, and
TagBlock's deriver, which injected the dfp_tag
storage directly and held it as a property. It takes the entity type manager
now.

OPT_IN_TEST_NEXT_MAJOR was 0, so there was no next
major job at all and CI would not have built against Drupal 12 even after this
lands. It is 1 now, with _AUTORUN_NEXT_MINOR and
_AUTORUN_NEXT_MAJOR set — opting in only declares the jobs, the
autorun variables are what run them, and the template defaults those to
none. That is why composer (next minor) has been
sitting at manual with everything downstream skipped.

Separately, and needing a maintainer rather than a patch: the repository's
default branch is 8.x-1.x, which is 26 commits behind
3.0.x and not in the supported branches. Raised in
#3619700.

marcelovani’s picture

Status: Active » Needs review
Related issues: +#3597160: Automated Drupal 12 compatibility fixes for dfp 3.0.x-dev
marcelovani’s picture

  • marcelovani committed 738a470b on 3619663-drupal-12
    Issue #3619663 by marcelovani: Fix what the next major job found...

  • marcelovani committed 250dc567 on 3619663-drupal-12
    Issue #3619663 by marcelovani: Use stubs for the token mocks too
    
    Same...