\Drupal\Tests\Core\Extension\InfoParserUnitTest::testInfoParserMissingKey() will pass even without expected exception [#3078781]

Problem/Motivation

follow up to #2313917: Core version key in module's .info.yml doesn't respect core semantic versioning

in that issue we made sure the parse threw the expected exception in even it was called twice to make sure \Drupal\Core\Extension\InfoParserDynamic::isConstraintSatisfiedByPreviousVersion() wasn't incorrectly caching result(as a previous version of the patch ones.

This involved surrounding the first call to parse() in a try and checking the expected exception manually. then calling the parse() the second time from within the catch.

For this to work

// Set the expected exception for the 2nd call to parse().
    $this->expectException('\Drupal\Core\Extension\InfoParserException');
    $this->expectExceptionMessage('Missing required keys (type) in vfs://modules/fixtures/missing_key-duplicate.info.txt');

Needs to be before the try block for the first call to parse

This was done correctly for all test methods except testInfoParserMissingKey()(my bad 😅)

Proposed resolution

Fix testInfoParserMissingKey()

Remaining tasks

patch, review

User interface changes

None

API changes

None

Data model changes

None

Release notes snippet

None

Comment	File	Size	Author
#9	3078781-9.patch	1.19 KB	tedbow
#9	8.8.x: PHP 7.1 & MySQL 5.7 Patch Failed to Apply
#4	3078781-4.patch	1.27 KB	tedbow
#4	8.8.x: PHP 7.1 & MySQL 5.7 27,141 pass
#4	interdiff-2-4.txt	817 bytes	tedbow
#2	3078781-2.patch	1.29 KB	tedbow
#2	8.8.x: PHP 7.1 & MySQL 5.7 27,140 pass

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

2 September 2019 at 18:25

tedbow created an issue. See original summary.

Comment #2

tedbow

he/him

English

Ithaca, NY, USA

CreditAttribution: tedbow at Acquia commented 2 September 2019 at 18:26

Title:	\Drupal\Tests\Core\Extension\InfoParserUnitTest::testInfoParserMissingKey() will pass even with expected exception	» \Drupal\Tests\Core\Extension\InfoParserUnitTest::testInfoParserMissingKey() will pass even without expected exception
Issue summary:	View changes
Status:	Active	» Needs review

File	Size
3078781-2.patch	1.29 KB
8.8.x: PHP 7.1 & MySQL 5.7 27,140 pass

Comment #3

Wim Leers

Ghent 🇧🇪🇪🇺

CreditAttribution: Wim Leers at Acquia commented 3 September 2019 at 14:04

Status:

Needs review

» Needs work

+++ b/core/tests/Drupal/Tests/Core/Extension/InfoParserUnitTest.php
@@ -328,14 +328,15 @@ public function testInfoParserMissingKey() {
+    $this->expectException('\Drupal\Core\Extension\InfoParserException');

Let's use FQCN::class if we're changing this anyway.

+++ b/core/tests/Drupal/Tests/Core/Extension/InfoParserUnitTest.php
@@ -328,14 +328,15 @@ public function testInfoParserMissingKey() {
+    $this->expectExceptionMessage('Missing required keys (type) in vfs://modules/fixtures/missing_key-duplicate.info.txt');
...
       $this->infoParser->parse(vfsStream::url('modules/fixtures/missing_key.info.txt'));

How can an exception be thrown for a file that is not being parsed? :O

Comment #4

tedbow

he/him

English

Ithaca, NY, USA

CreditAttribution: tedbow at Acquia commented 3 September 2019 at 14:41

Status:

Needs work

» Needs review

File	Size
interdiff-2-4.txt	817 bytes
3078781-4.patch	1.27 KB
8.8.x: PHP 7.1 & MySQL 5.7 27,141 pass

@Wim Leers thanks for review!

fixed
And yet it works!😉

+++ b/core/tests/Drupal/Tests/Core/Extension/InfoParserUnitTest.php
@@ -328,14 +328,15 @@ public function testInfoParserMissingKey() {
+    $this->expectExceptionMessage('Missing required keys (type) in vfs://modules/fixtures/missing_key-duplicate.info.txt');
...
       $this->infoParser->parse(vfsStream::url('modules/fixtures/missing_key-duplicate.info.txt'));

The expected exception is for the 2nd call to parse. This has to be set before the first call or if the first call has no exception the test will pass.

This works because \PHPUnit\Framework\TestCase::expectException() and \PHPUnit\Framework\TestCase::expectExceptionMessage() are not for the next call but are checked for any uncaught exception.

Comment #5

Wim Leers

Ghent 🇧🇪🇪🇺

CreditAttribution: Wim Leers at Acquia commented 3 September 2019 at 15:34

Status:

Needs review

» Reviewed & tested by the community

D'oh, of course!

Comment #6

alexpott

he/they

English

🇪🇺🌍

CreditAttribution: alexpott at Acro Commerce, Thunder commented 4 September 2019 at 14:18

Version:	8.8.x-dev	» 8.7.x-dev
Status:	Reviewed & tested by the community	» Patch (to be ported)

Committed 42962f7 and pushed to 8.8.x. Thanks!

Comment #7

4 September 2019 at 14:19

alexpott committed 42962f7 on 8.8.x

Issue #3078781 by tedbow, Wim Leers: \Drupal\Tests\Core\Extension\...

Comment #8

alexpott

he/they

English

🇪🇺🌍

CreditAttribution: alexpott at Acro Commerce, Thunder commented 9 September 2019 at 11:00

Version:	8.7.x-dev	» 8.8.x-dev
Status:	Patch (to be ported)	» Fixed

This no longer applies to 8.7.x since #3079805: expectedException() usage in two pre-8.7.7 commits has broken PHP 5 testing for 8.7.x - so closing as fixed because at least this is in 8.8.x. If someone feels it is important that this is in 8.7.x then we can backport but I think we can expend effort elsewhere.

Comment #9

tedbow

he/him

English

Ithaca, NY, USA

CreditAttribution: tedbow at Acquia commented 9 September 2019 at 19:20

Version:	8.8.x-dev	» 8.7.x-dev
Status:	Fixed	» Needs review

File	Size
3078781-9.patch	1.19 KB
8.8.x: PHP 7.1 & MySQL 5.7 Patch Failed to Apply

@alexpott thanks for committing to 8.8.x.

Here is the 8.7.x patch.

Comment #10

9 September 2019 at 19:20

Version:

8.7.x-dev

» 8.8.x-dev

Drupal 8.7.9 was released on November 6 and is the final full bugfix release for the Drupal 8.7.x series. Drupal 8.7.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.8.0 on December 4, 2019. (Drupal 8.8.0-beta1 is available for testing.)

Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Comment #11

9 September 2019 at 19:20

Version:

8.8.x-dev

» 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.