Problem/Motivation

CommentUninstallTest makes no HTTP requests but is a functional test

Proposed resolution

Convert CommentUninstallTest into a Kernel test. Merge the 2 method tests.

Remaining tasks

None.

User interface changes

None.

API changes

None.

Data model changes

None.

Release notes snippet

N/A

CommentFileSizeAuthor
#4 3049637-4.patch6.46 KBclaudiu.cristea
#4 3049637-4.interdiff.txt2.89 KBclaudiu.cristea
#2 3049637-2.patch5.22 KBclaudiu.cristea
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

claudiu.cristea created an issue. See original summary.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
CreditAttribution: claudiu.cristea as a volunteer and at Webikon commented
Status: Active » Needs review
FileSize
3049637-2.patch5.22 KB

This patch decreases the locally test run from 15.7 to 1.6 seconds.

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude as a volunteer and at ezCompany commented
Status: Needs review » Needs work 
+++ b/core/modules/comment/tests/src/Kernel/CommentUninstallTest.php
@@ -1,83 +1,84 @@
+    $this->expectException(ModuleUninstallValidatorException::class);
+    $this->expectExceptionMessage('The following reasons prevent the modules from being uninstalled: The <em class="placeholder">Comments</em> field type is used in the following field: node.comment');
+    $module_installer->uninstall(['comment']);

This detects the exception but still stops execution when the exception is thrown, so everything after this is never actually run.
So this needs the original try/catch or the split into two separate methods to run the second part of the test.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
CreditAttribution: claudiu.cristea as a volunteer and at Webikon commented
Status: Needs work » Needs review
FileSize
3049637-4.interdiff.txt2.89 KB
3049637-4.patch6.46 KB

Splitted in 2 test methods.

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude as a volunteer and at ezCompany commented
Status: Needs review » Reviewed & tested by the community

Looks good now. No loss of coverage that I see.

catch’s picture

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

Committed/pushed to 8.8.x, thanks!

  • catch committed 684c23c on 8.8.x 
    Issue #3049637 by claudiu.cristea, Lendude: Convert CommentUninstallTest...

Status: Fixed » Closed (fixed)

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