Problem/Motivation

The fix merged in Outbound HTTP requests fail with KernelTestBase #51 added test coverage that does not perform any assertion, which is considered an antipattern.

PHPUnit would complain about it, but as @alexpott wrote in #53:

@mxr576 good point - can you create a follow-up issue to add an assertion. It doesn't fail because we have an assertion in \Drupal\KernelTests\KernelTestBaseTest::tearDown()

This is true: https://github.com/drupal/core/blob/325813c889288bd6bd6b8e7590c43e704eb6...

Isn't this a code smell too? It hides issues with useless, incorrect tests as the test in the subject example perfectly represents it.

Steps to reproduce

Proposed resolution

* Add missing assert to KernelTestBaseTest::testOutboundHttpRequest().
* Remove asserts from tearDown() ?

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3196470

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
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mxr576 created an issue. See original summary.

mxr576 opened merge request !294

mxr576’s picture

mxr576
Primary language Hungarian
Location Hungary
CreditAttribution: mxr576 at Pronovix commented
Status: Active » Needs review
mxr576’s picture

mxr576
Primary language Hungarian
Location Hungary
CreditAttribution: mxr576 at Pronovix commented

I did not change anything related to

Isn't this a code smell too? It hides issues with useless, incorrect tests as the test in the subject example perfectly represents it

Probably that should be solved in another follow-up issue...

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented

Opened #3196507: KernelTestBaseTest::tearDown() should not perform assertions as followup.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Status: Needs review » Reviewed & tested by the community

MR still applies, no reason to hold this back.

  • catch committed 3e5c6f5 on 9.3.x 
    Issue #3196470 by mxr576, longwave: KernelTestBaseTest::...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

Committed 3e5c6f5 and pushed to 9.3.x. Thanks!

Status: Fixed » Closed (fixed)

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