Problem/motivation

The automated tests for #3573881: Fix coding standards violations fail. The failure is reproducible, but does not seem to be related to the changes in that issue. At least some failures seem related to #3535526: Deprecate block_content_add_body_field and stop automatic creation of body field. See the related change record https://www.drupal.org/node/3535528.

Let's also fix deprecations (in the code and in the tests) as part of this issue. The current test summary is

Tests: 19, Assertions: 344, Errors: 11, PHPUnit Deprecations: 34.

Steps to reproduce

Proposed resolution

  1. Replace BlockContentType::create() with Drupal\Tests\block_content\Traits\BlockContentCreationTrait::createBlockContentType(). Set the optional $create_body parameter to TRUE to add a Body field, replacing the deprecated block_content_add_body_field(). See the change record https://www.drupal.org/node/3535528.
  2. Add the #[RunTestsInSeparateProcesses] attribute to each concrete test class. See the change record https://www.drupal.org/node/3548485.
Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

benjifisher created an issue. See original summary.

benjifisher’s picture

I started with a single commit: update composer.json and README.md, adding myself as a maintainer. That is enough to generate a MR and trigger the CI pipeline. I see the same failures as on #3573881.

benjifisher’s picture

Issue summary: View changes

I fixed the failing tests and some of the deprecations by replacing BlockContentType::create() with Drupal\Tests\block_content\Traits\BlockContentCreationTrait::createBlockContentType(). That method has an optional parameter to add a Body field, the equivalent of the deprecated block_content_add_body_field().

Instead of that, I could have replaced block_content_add_body_field() with Drupal\Tests\field\Traits\BodyFieldCreationTrait::createBodyField(), which is what BlockContentCreationTrait::createBlockContentType() calls. That would be one fewer levels of indirection and a few more lines of code.

Either way, in order to use non-static, protected methods of a trait, I had to remove the static declaration from LayoutBuilderRestrictionsTestBase::generateTestBlocks(). I am not sure why that method was declared static, but it does not seem to matter since it is never called statically. (That is, it is always called as $this->generateTestBlocks(), not as self::generateTestBlocks() nor static::generateTestBlocks().)

Running the test locally, I saw additional deprecation notices. I added the #[RunTestsInSeparateProcesses] attribute to each concrete test class. See the change record https://www.drupal.org/node/3548485.

I also saw this deprecation notice:

/var/www/html/core/tests/Drupal/FunctionalJavascriptTests/WebDriverTestBase.php:140
The "w3c" option for Chrome is deprecated in drupal:11.4.0 and will be forced to TRUE in drupal:12.0.0. See https://www.drupal.org/node/3460567

I fixed this by updating the configuration of my local Docker containers, following the instructions in the change record. The change record also mentions several test methods that need to be changed. I searched for these methods and did not find any uses.

The tests use the @group annotation, but that does not (yet) generate a deprecation notice. Should I replace that with the #[Group()] annotation as part of this issue?

benjifisher’s picture

Assigned: benjifisher » Unassigned
Status: Active » Needs review
mark_fullmer’s picture

I fixed the failing tests and some of the deprecations by replacing BlockContentType::create() with Drupal\Tests\block_content\Traits\BlockContentCreationTrait::createBlockContentType()

Makes sense!

Either way, in order to use non-static, protected methods of a trait, I had to remove the static declaration from LayoutBuilderRestrictionsTestBase::generateTestBlocks(). I am not sure why that method was declared static, but it does not seem to matter

Yeah, the static declaration was most likely force of habit; I also don't see a reason it needs to be static.

I added the #[RunTestsInSeparateProcesses] attribute to each concrete test class.

Thanks for bringing this to my awareness. Looks like I'll be needing to do this in a number of other projects....

I also saw this deprecation notice: /var/www/html/core/tests/Drupal/FunctionalJavascriptTests/WebDriverTestBase.php:140
The "w3c" option for Chrome is deprecated in drupal:11.4.0

Our team also has this in our local tooling for test execution. Thanks for pointing this out.

Should I replace that with the #[Group()] annotation as part of this issue?

I'm comfortable with retiring the use of the annotations here in favor of attributes. I don't think we need to keep testing for compatibility with Drupal < 10 at this point.

benjifisher’s picture

Assigned: Unassigned » benjifisher
Status: Needs review » Needs work

I'm comfortable with retiring the use of the annotations here in favor of attributes. I don't think we need to keep testing for compatibility with Drupal < 10 at this point.

I am setting the status to NW for that, and assigning the issue to myself. It might be a few days before I have time for it, and I do not mind if someone else re-assigns it.

benjifisher’s picture

Status: Needs work » Needs review

I replaced the @group annotations in tests with #[Group()] attributes. Back to NR.

benjifisher’s picture

Assigned: benjifisher » Unassigned
Status: Needs review » Needs work
benjifisher’s picture

Status: Needs work » Needs review

Oops.

mark_fullmer’s picture

Status: Needs review » Reviewed & tested by the community

Everything looks great! Thanks for this work, and for future-facing changes related to PHPUnit 12. Marking this as RTBC. Since you're a maintainer, feel free to merge the MR. Then we can turn our attention to #3573881: Fix coding standards violations :)

benjifisher’s picture

Status: Reviewed & tested by the community » Fixed

@mark_fullmer:

Done. Thanks for working with me on these issues.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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