Support from Acquia helps fund testing for Drupal Acquia logo

Comments

yobottehg created an issue. See original summary.

yobottehg’s picture

Issue summary: View changes
yobottehg’s picture

Issue tags: +DevDaysMilan
GoZ’s picture

Issue summary: View changes
GoZ’s picture

Issue summary: View changes
jmuzz’s picture

FieldUiTestTrait uses assertFieldByXPath so the tests using this trait may need to wait for either #2784537: Add legacy assertFieldByXPath()/assertNoFieldByXPath() method for browser tests to be done or the field_ui module to get converted to unit tests for #2735005: Convert all Simpletest web tests to BrowserTestBase (or UnitTestBase/KernelTestBase). field_ui doesn't seem to have its own issue currently.

jmuzz’s picture

I made a patch for field_ui. Together the patches in these two issues should make it possible to convert the paragraphs tests that are using using FieldUiTestTrait.

#2784537: Add legacy assertFieldByXPath()/assertNoFieldByXPath() method for browser tests

#2794347: Convert web tests to browser tests for field_ui module

chr.fritsch’s picture

Status: Active » Needs review
FileSize
129.04 KB

Here is an patch, to convert all tests with one big bang. Currently there are some issues:

  • ParagraphsAdministrationTest and ParagraphsExperimentalAdministrationTest are failing because of #2794347: Convert web tests to browser tests for field_ui module.
  • ParagraphsTranslationTest and ParagraphsExperimentalTranslationTest are failing due to an unknown reason to me.
  • ParagraphsUninstallTest, seems that the batch to delete paragraphs is currently not working in test

But that should be the only issues. I would like to push this issue forward, to have more possibilities in future tests.

Status: Needs review » Needs work

The last submitted patch, 8: 2752659.patch, failed testing.

Lendude’s picture

Status: Needs work » Needs review
FileSize
117.81 KB

#8 doesn't apply more (at all), this is a start from scratch since core moved quite a bit forward since this was last done. This just moves everything to the right place and namespace, removes some uses of traits that are already in the parent class, and renames drupalPostAjaxForm to drupalPostForm (pretty much what happened in #8 I think).

Lets see what breaks now.

Lendude’s picture

Ok moved the sub module tests too, lets see if it actually runs now....

Lendude’s picture

Ok..moved ParagraphsCoreVersionUiTestTrait to a spot where it can actually be found during test runs (I hope).

Lendude’s picture

Missed one

Status: Needs review » Needs work

The last submitted patch, 13: 2752659-13.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Berdir’s picture

Thanks for starting this. Maybe we should split it up a bit, make it easier to review and get the easy parts in first?

Lendude’s picture

Status: Needs work » Needs review
FileSize
15.94 KB
161.15 KB

@Berdir absolutely. This still feels like the easy part though :)

This renames drupalGetTestFiles to getTestFile which is made available in one of the parent classes, so lets see if that caches everything.
And removes one instance of asXML

So far the only changes are:

  • Move and change namespaces
  • rename drupalPostAjaxForm to drupalPostForm
  • rename drupalGetTestFiles to getTestFile
  • clean up some trait usage
  • remove use of asXML

There seems to be an issue with the settings of entity_usage (leads to config fails) and I think the messages that this shows, haven't looked into why this does work in simpletest.

Status: Needs review » Needs work

The last submitted patch, 16: 2752659-16.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Lendude’s picture

Status: Needs work » Needs review
FileSize
76.39 KB
97.32 KB

Ok so this reverts some of the failing tests back to simpletest so we can work on those separately.

Only other changes are a fix for Cannot use object of type Behat\Mink\Element\NodeElement as array.

Interdiff might not be that useful due to all the reverting.

Lets see what I missed..

Status: Needs review » Needs work

The last submitted patch, 18: 2752659-18.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Berdir’s picture

  1. +++ b/modules/paragraphs_demo/tests/src/Functional/ParagraphsDemoTest.php
    @@ -76,7 +76,7 @@ class ParagraphsDemoTest extends WebTestBase {
         $this->drupalGet('admin/structure/types/manage/paragraphed_content_demo/form-display');
    -    $this->drupalPostAjaxForm(NULL, [], "field_paragraphs_demo_settings_edit");
    +    $this->drupalPostForm(NULL, [], "field_paragraphs_demo_settings_edit");
    

    I wasn't aware that this actually works in so many cases :)

    I usually converted them to click() and similar things, this is nice.

  2. +++ b/tests/src/Traits/ParagraphsCoreVersionUiTestTrait.php
    @@ -0,0 +1,58 @@
    +/**
    + * Provides helper methods for Drupal 8.3.x and 8.4.x versions.
    + */
    +trait ParagraphsCoreVersionUiTestTrait {
    

    Hm..

    Probably off-topic to do something about this one here. And I guess it's used a lot.

    Maybe just drop the actual logic of the version check and just always do that? Maybe even add a @deprecated and follow-up to remove it?

I'll want to apply it and check what/how much is left, but this seems to be a really good start. Maybe we could create a child issue for this first chunk as this is a meta? There are also various existing per-class child issues, we can probably clean that up and close the duplicate ones, maybe keep some if they happen to be not-yet-converted and complex.

Lendude’s picture

#20.1 yeah it works but you lose even the pseudo js coverage, so it should really get a follow up to add some basic real JS coverage, but mich less hassle then trying to convert everything to real JS now
#20.2 yeah probably better handled in a follow up

Continuing the work in #2753889: Convert ParagraphsTestBase and the easy-to-convert tests extending it to BrowserTestBase which seems to fit the bill nicely

Lendude’s picture

Issue summary: View changes

Made a new list of child issues since some earlier issues are no longer relevant.

Added #3077720: Convert ParagraphsExperimentalTypesTest to PHPUnit

Lendude’s picture

Issue summary: View changes
Lendude’s picture

Issue summary: View changes

Added issues for the remaining tests, grouped them together if there was a classic and experimental version

Lendude’s picture

Berdir’s picture

Issue summary: View changes
Status: Needs work » Fixed

Committed all of them. Leaving the base classes for now, we still have subclasses of them in paragraphs_collection for example.

Status: Fixed » Closed (fixed)

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