Problem/Motivation

Once all other tasks of #3376057: [META] Add declare(strict_types=1) to all tests are done we can enforce strict types in tests.

Steps to reproduce

N/A

Proposed resolution

Add the following to phpcs.xml.dist

  <rule ref="SlevomatCodingStandard.TypeHints.DeclareStrictTypes">
    <properties>
      <property name="spacesCountAroundEqualsSign" value="0" />
    </properties>
    <include-pattern>*/tests/*</include-pattern>
    <exclude-pattern>*/fixtures/*</exclude-pattern>
  </rule>

Remaining tasks

  • Review changes to ViewResultAssertionTrait and SortDateTest
  • Review changes to http.php and https.php

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3400434

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:

About issue forks

Comments

mstrelan created an issue. See original summary.

mstrelan opened merge request !5315

mstrelan’s picture

mstrelan commented

Current status (assuming Kernel tests gets in soon):

<include-pattern>*/tests/*</include-pattern>
<exclude-pattern>*/tests/modules/*</exclude-pattern>
<exclude-pattern>*/tests/themes/*</exclude-pattern>
<exclude-pattern>*/tests/*_test/*</exclude-pattern>
<exclude-pattern>*/tests/*fixture*/*</exclude-pattern>
  • #3400334: Add declare(strict_types=1) to all test modules should take care of the first two exclude patterns.
  • We need another issue for test themes, I think they are all in core/modules/system/tests/themes/
  • We need another issue for fixtures, although maybe we should continue to ignore them

That leaves the following that need to be categorised:

core/modules/comment/src/Tests/CommentTestTrait.php
core/modules/contact/tests/drupal-7.contact.database.php
core/modules/filter/tests/filter_test_plugin/src/Plugin/Filter/FilterSparkles.php
core/modules/filter/tests/filter_test_plugin/src/Plugin/Filter/FilterTestStatic.php
core/modules/menu_link_content/tests/menu_link_content_dynamic_route/src/Routes.php
core/modules/migrate_drupal/src/Tests/StubTestTrait.php
core/modules/node/tests/node_access_test_auto_bubbling/src/Controller/NodeAccessTestAutoBubblingController.php
core/modules/system/tests/http.php
core/modules/system/tests/https.php
core/modules/views/src/Tests/AssertViewsCacheTagsTrait.php
core/modules/views/src/Tests/TestHelperPlugin.php
core/modules/views/src/Tests/ViewResultAssertionTrait.php
core/modules/views/src/Tests/ViewTestData.php
core/tests/bootstrap.php
  • Possibly the Traits and Views classes should move to the tests dir in each module, unless there is some reason they need to live in src/Tests instead.
  • We should make sure to capture filter_test_plugin, menu_link_content_dynamic_route and node_access_test_auto_bubbling in #3400334: Add declare(strict_types=1) to all test modules, or move them to a test/modules directory first
  • Not too sure about the remaining non-class php files, I guess we should be ok to include them
mstrelan’s picture

mstrelan commented
Status: Postponed » Active
mstrelan’s picture

mstrelan commented
Issue summary: View changes
Status: Active » Needs review
mstrelan’s picture

mstrelan commented
Issue summary: View changes
smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Yes! Very happy to see this is going to be mandatory now.

Changes in MR seems fine to me.

  • catch committed f3a63b72 on 11.x 
    Issue #3400434 by mstrelan: Enforce strict types in tests
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 11.x, thanks!

catch closed merge request !5315

quietone’s picture

quietone commented

Since this issue enabled strict types I moved the CR from the parent to here and published it.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Nice! 😄

Status: Fixed » Closed (fixed)

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