Support from Acquia helps fund testing for Drupal Acquia logo

Comments

DamienMcKenna created an issue. See original summary.

DamienMcKenna’s picture

Status: Active » Postponed

Marking this "postponed" as I won't be looking at it until after 1.0.

DamienMcKenna’s picture

Status: Postponed » Active

8.x-1.0 is out, so this is fair game again.

DamienMcKenna’s picture

FileSize
10.92 KB

WIP.

DamienMcKenna’s picture

Status: Active » Needs review
FileSize
21.98 KB
11.79 KB

More WIP.

DamienMcKenna’s picture

Status: Needs review » Needs work

The last submitted patch, 7: metatag-n2820214-7.patch, failed testing.

DamienMcKenna’s picture

FYI these won't work with 8.2.x because the backwards compatibility layer is only in 8.3.

DamienMcKenna’s picture

A lot of the failures are related to changes in the ::xpath() return code.

KarenS’s picture

I did some digging into this, focusing just on one test and base, the tags test:

was:
isset($xpath[0][$xpath_value_attribute])
becomes:
null !== $xpath[0]->getAttribute($xpath_value_attribute)

was:
(string)$xpath[0]
becomes:
$xpath[0]->getText()

was:
$this->assertTrue($xpath[0][$xpath_value_attribute]);
becomes:
$this->assertTrue($xpath[0]->getAttribute($xpath_value_attribute));

The verbose() method doesn't work, and really throws up if you put the $xpath object in it, see issue.

Also some method names have changed:

assertEqual(): assertEquals(),
assertFalse(): assertEmpty(),
assertResponse: assertSession()->statusCodeEquals()

DamienMcKenna’s picture

Thanks Karen. After all of the recent changes we'll need to start over on the patch anyway, so your research is helpful.

Lendude’s picture

Status: Needs work » Needs review
FileSize
38.58 KB

Basic move of the top level tests (so not the sub-module ones). Lets see where the test bot fails.

I've copied the base classes and @deprecated the simpletest versions. Currently just with a doc block tag, no trigger_error.

Didn't find any ajax/fake-js emulation in here, which is nice and hopefully leaves out the really complicated conversion stuff.

Status: Needs review » Needs work

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

Lendude’s picture

Status: Needs work » Needs review
FileSize
46.85 KB
8.71 KB

// Use the helper functions from the Functional trait. This is pretty safe but
// remember to rewrite all of these WebTestBase tests using BrowserTestBase
// before the next millenium.

:-)

this should clear up a fair number of fails.

DamienMcKenna’s picture

Status: Needs review » Needs work
Parent issue: » #2996500: Plan for Metatag 8.x-1.10

Discussed this with @lendude in slack.

The end goal for this issue will be to not have any tests in the src/Tests directories, instead having replacements in the tests/src directories.

Let's get this into the next release.

Lendude’s picture

Status: Needs work » Needs review
FileSize
39.61 KB
35.84 KB

Hmm not sure if MetatagPageManagerTest was ever run, since it was in the wrong namespace. Might fail now that it is in the right spot.

Moved all the sub-module tests that needed moving, didn't do more then a small sample test to see if they pass, so lets see what the bot makes of this.

Lendude’s picture

124 pass in #16, 110 pass in #18, checked the jenkins logs and both times the same set of tests is detected, so not sure what is going on here.

edit:
Using Simpletest:
Drupal\metatag_twitter_cards\Tests\MetatagTwitterCardsTagsTe 50 passes

Using BrowserTestBase:
Drupal\Tests\metatag_twitter_cards\Functional\MetatagTwitter 2 passes

As far as I can tell, there are only 2 test cases in this test class, so seems to me that the PHPUnit count is right.

idebr’s picture

Status: Needs review » Reviewed & tested by the community

Patch applies cleanly 8.x-1.x, and converts all deprecated Simpletests to PHPUnit tests

  • DamienMcKenna committed b1c9a37 on 8.x-1.x
    Issue #2820214 by DamienMcKenna, Lendude, KarenS, idebr: Update tests to...
DamienMcKenna’s picture

Status: Reviewed & tested by the community » Fixed

Thanks everyone, this has been committed!

Status: Fixed » Closed (fixed)

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