Problem/Motivation

A number of tests are marked ‘risky’ when PHPUnit is configured to fail in such case.

‘Risky’ tests are those that do not perform any assertion.

In the past, I think there was an assumption that risky tests would flag a failure in Drupal core tests. This is currently not the case since PHPUnit 10 adoption. The configuration file should explicitly have a failOnRisky attribute, which is not the case atm.

Steps to reproduce

Proposed resolution

- Configure PHPUnit to fail when risky tests are run
- Fix the failing tests

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3466788

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

mondrake created an issue. See original summary.

mondrake opened merge request !9126

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Related issues: +#2805849: WebAssert does not track assertions, Test marked as risky

#2805849: WebAssert does not track assertions, Test marked as risky is semi related, there were no functional fails here but only because BTB explicitly increments the assertion count to avoid risky tests.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Active » Needs review
mondrake’s picture

mondrake
Location 🇮🇹
commented
Issue summary: View changes
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Issue tags: +Needs change record

The changes look good but as this is a change to default PHPUnit config it can only go in a minor release and needs a change record.

We probably should backport the test changes without the config change to keep things in sync.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Issue tags: -Needs change record

Added draft CR.

smustgrave’s picture

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

CR seems straight forward enough but tests appear passing with change to core/phpunit.xml.dist

  • catch committed 2b255fa2 on 10.3.x 
    Issue #3466788 by mondrake, longwave: Fix ‘risky’ tests

(cherry picked...

  • catch committed c1de768e on 10.4.x 
    Issue #3466788 by mondrake, longwave: Fix ‘risky’ tests

(cherry picked...

  • catch committed b7168f97 on 11.0.x 
    Issue #3466788 by mondrake, longwave: Fix ‘risky’ tests

  • catch committed 3ea0221e on 11.x 
    Issue #3466788 by mondrake, longwave: Fix ‘risky’ tests
catch’s picture

catch
he/him
Primary language English
commented
Version: 11.x-dev » 10.3.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 11.x and cherry-picked without the phpunit.xml change to 11.0.x/10.4.x/10.3.x, thanks!

catch closed merge request !9126

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

Published the CR.

Status: Fixed » Closed (fixed)

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