Problem/Motivation

drupal-check output:

drupal-check web/modules/contrib/paragraphs_features 
 10/10 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%
                                                             
 [OK] No errors

deprecations after running tests:

Remaining deprecation notices (7)

  7x: Drupal\Tests\BrowserTestBase::$defaultTheme is required in drupal:9.0.0 when using an install profile that does not set a default theme. See https://www.drupal.org/node/3083055, which includes recommendations on which theme to use.
    1x in ParagraphsFeaturesAddInBetweenTest::testAddInBetweenFeature from Drupal\Tests\paragraphs_features\FunctionalJavascript
    1x in ParagraphsFeaturesDeleteConfirmationTest::testDeleteConfirmation from Drupal\Tests\paragraphs_features\FunctionalJavascript
    1x in ParagraphsFeaturesDeleteConfirmationTest::testCkEditorAfterCancel from Drupal\Tests\paragraphs_features\FunctionalJavascript
    1x in ParagraphsFeaturesExtendedParagraphsWidgetTest::testAddInBetweenFeature from Drupal\Tests\paragraphs_features\FunctionalJavascript
    1x in ParagraphsFeaturesSingleActionTest::testSingleActionOption from Drupal\Tests\paragraphs_features\FunctionalJavascript
    1x in ParagraphsFeaturesSingleActionTest::testAddAboveSupport from Drupal\Tests\paragraphs_features\FunctionalJavascript
    1x in ParagraphsFeaturesSplitTextTest::testSplitTextFeature from Drupal\Tests\paragraphs_features\FunctionalJavascript

Remaining tasks

  • Add core_version_requirement to paragraphs_features.info.yml.
  • Add defaultTheme to the tests.
CommentFileSizeAuthor
#13 interdiff_10-13.txt755 bytestdnshah
#13 3120474-13.patch2.4 KBtdnshah
#10 3120474-10.patch1.93 KBsuresh prabhu parkala
#7 3120474-7.patch1.65 KBsuresh prabhu parkala
#4 3120474-4.patch3.65 KBsuresh prabhu parkala
#2 3120474-2.patch416 bytessuresh prabhu parkala

Comments

Suresh Prabhu Parkala created an issue. See original summary.

suresh prabhu parkala’s picture

suresh prabhu parkala commented
Status: Active » Needs review
StatusFileSize
new 3120474-2.patch416 bytes

Please review!

jeroent’s picture

jeroent
Primary language Dutch
Location 🇧🇪
commented
Issue summary: View changes
Status: Needs review » Needs work
Issue tags: +Drupal 9 compatibility, +Novice

$defaultTheme is missing from the tests.

suresh prabhu parkala’s picture

suresh prabhu parkala commented
Status: Needs work » Needs review
StatusFileSize
new 3120474-4.patch3.65 KB

Please review!

Status: Needs review » Needs work

The last submitted patch, 4: 3120474-4.patch, failed testing. View results

jeroent’s picture

jeroent
Primary language Dutch
Location 🇧🇪
commented

It seems the test ParagraphsFeaturesSplitTextTest is failing because some css classes are missing.

I would suggest add protected $defaultTheme = 'stark'; to ParagraphsFeaturesJavascriptTestBase since all other tests are extending that class and overriding $defaultTheme with 'classy' for ParagraphsFeaturesSplitTextTest.

suresh prabhu parkala’s picture

suresh prabhu parkala commented
Status: Needs work » Needs review
StatusFileSize
new 3120474-7.patch1.65 KB

Please review the updated patch. Thank you.

jeroent’s picture

jeroent
Primary language Dutch
Location 🇧🇪
commented

Patch looks good to me. and is passing on Drupal 8.8 and Drupal 8.9.

jeroent’s picture

jeroent
Primary language Dutch
Location 🇧🇪
commented
Status: Needs review » Needs work

ah, I missed this one. The module is still not installable on Drupal 9 because of this line in composer.json:

"drupal/core": "^8.5",
suresh prabhu parkala’s picture

suresh prabhu parkala commented
Status: Needs work » Needs review
StatusFileSize
new 3120474-10.patch1.93 KB

Please review the patch.

jeroent’s picture

jeroent
Primary language Dutch
Location 🇧🇪
commented
Status: Needs review » Reviewed & tested by the community

Looks good to me.

jeroent’s picture

jeroent
Primary language Dutch
Location 🇧🇪
commented
Status: Reviewed & tested by the community » Needs work

Patch is failing since #2753889: Convert ParagraphsTestBase and the easy-to-convert tests extending it to BrowserTestBase was committed to paragraphs.

The trait used in Drupal\Tests\paragraphs_features\FunctionalJavascript\ParagraphsFeaturesJavascriptTestBase should be updated from Drupal\paragraphs\Tests\Classic\ParagraphsCoreVersionUiTestTrait to
Drupal\Tests\paragraphs\Traits\ParagraphsCoreVersionUiTestTrait.

tdnshah’s picture

tdnshah commented
StatusFileSize
new 3120474-13.patch2.4 KB
new interdiff_10-13.txt755 bytes

Updating as per suggestion in #12
attaching patch for the same

tdnshah’s picture

tdnshah commented
Status: Needs work » Needs review
jeroent’s picture

jeroent
Primary language Dutch
Location 🇧🇪
commented
Status: Needs review » Reviewed & tested by the community
manav’s picture

manav commented
Assigned: Unassigned » manav

1. Upgrade status: Paragraphs Features No known errors
2. drupal-check: 

vendor/bin/drupal-check -ad web/modules/contrib/paragraphs_features/
3/3 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

 [OK] No errors

#13 patch working fine.
RTBC

manav’s picture

manav commented
Assigned: manav » Unassigned

chr.fritsch credited volkerk.

chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
commented

@volkerk approved https://github.com/thunder/paragraphs_features/pull/28

  • chr.fritsch authored b82d324 on 8.x-1.x 
    Issue #3120474 by Suresh Prabhu Parkala, tdnshah, volkerk, chr.fritsch:...
chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Done

manav’s picture

manav commented

Status: Fixed » Closed (fixed)

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