CommentFileSizeAuthor
#56 2742995-56.patch7 KBJo Fitzgerald
#53 2742995-53.patch6.45 KBJo Fitzgerald
#50 interdiff-48-to-50.txt827 bytesclaudiu.cristea
#50 2742995-50.patch6.99 KBclaudiu.cristea
#48 interdiff-44-to-48.txt3.87 KBclaudiu.cristea
#48 2742995-48.patch7.07 KBclaudiu.cristea
#44 2742995-44.patch5.64 KBJo Fitzgerald
#44 interdiff-42-44.txt5.88 KBJo Fitzgerald
#42 2742995-42.patch12.48 KBJo Fitzgerald
#42 interdiff-40-42.txt2.2 KBJo Fitzgerald
#40 2742995-40.patch15.22 KBJo Fitzgerald
#40 interdiff-35-40.txt1.03 KBJo Fitzgerald
#36 2742995-35.patch12.73 KBJo Fitzgerald
#29 browsertest-contact-2742995-29.patch21.21 KBklausi
#27 interdiff.txt15.5 KBklausi
#27 browsertest-contact-2742995-27.patch20.19 KBklausi
#22 add-assert-legacy-2742995-22-assertCacheContext.patch1.11 KBnaveenvalecha
#22 2742995-22.patch16.68 KBnaveenvalecha
#18 2742995-18.patch20.46 KBclaudiu.cristea
#14 2742995-14.patch122.56 KBclaudiu.cristea
#14 interdiff.txt5.6 KBclaudiu.cristea
#10 interdiff-2742995-6-9.txt6.29 KBnaveenvalecha
#10 2742995-9.patch39.64 KBnaveenvalecha
#6 2742995-6.patch35.62 KBnaveenvalecha
Members fund testing for the Drupal project. Drupal Association Learn more

Comments

naveenvalecha created an issue. See original summary.

naveenvalecha’s picture

Component: help.module » contact.module
naveenvalecha’s picture

Assigned: Unassigned » naveenvalecha

assigning to myself. will work on it tonight

naveenvalecha’s picture

dawehner’s picture

Assigned: naveenvalecha » Unassigned

Let's unassign, so someone else could pick it up.
@naveenvalecha of course you can pick it up at any point in time :)

naveenvalecha’s picture

Assigned: Unassigned » naveenvalecha
Status: Active » Needs review
FileSize
35.62 KB

Sorry for being late here. I was busy with my practicals exams. Assigning back to myself.

This test also includes the patch #2750941: Additional BC assertions from WebTestBase to BrowserTestBase as it depends on couple of assertion helpers of that module.
This patch will fails let's see what ci throws.
How about the tests that are extending viewsTestBase, should we cover this in chunks OR rely on views module conversion first ? how are you dealing with those ? cc / @dawehner / @klausi ?

dawehner’s picture

How about the tests that are extending viewsTestBase, should we cover this in chunks OR rely on views module conversion first

Let's do that in their own subissue. They will probably take some time, given the speciality of views.

Status: Needs review » Needs work

The last submitted patch, 6: 2742995-6.patch, failed testing.

dawehner’s picture

How about the tests that are extending viewsTestBase, should we cover this in chunks OR rely on views module conversion first ? how are you dealing with those ? cc / @dawehner / @klausi ?

I totally believe that we should have a dedicated issue for those issues.

    $i = 0;
    foreach ($this->xpath('//table/tbody/tr') as $row) {
      if (((string) $row->td[0]->a) == $label) {
        break;
      }
      $i++;
    }

    $this->clickLink(t('Manage fields'), $i);

This code in ContactSidewideTest is now problematic, as clickLink() no longer supports $i.

  1. +++ b/core/modules/contact/tests/src/Functional/ContactAuthenticatedUserTest.php
    --- a/core/modules/contact/src/Tests/ContactLanguageTest.php
    +++ b/core/modules/contact/tests/src/Functional/ContactLanguageTest.php
    
    +++ b/core/modules/contact/tests/src/Functional/ContactLanguageTest.php
    +++ b/core/modules/contact/tests/src/Functional/ContactLanguageTest.php
    @@ -1,8 +1,8 @@
    
    @@ -13,7 +13,7 @@
      */
    -class ContactLanguageTest extends WebTestBase {
    +class ContactLanguageTest extends BrowserTestBase {
    

    Note: There are methods which no longer support a $message, like.

    $this->assertResponse(200, 'The page exists');

    Let's drop those unused parameters.

  2. +++ b/core/modules/contact/tests/src/Functional/ContactPersonalTest.php
    @@ -1,19 +1,24 @@
    +class ContactPersonalTest extends BrowserTestBase {
    +
    +  use AssertMailTrait {
    +    getMails as drupalGetMails;
    +  }
     
    

    Let's rather rename the usages

PS: @naveenvalecha Do you mind including review only patches? They are really helpful to focus.

naveenvalecha’s picture

Included the patch https://www.drupal.org/node/2750941#comment-11320591
Addressed :
#9.2

This assertion is failing
Failed asserting that false is true. Other ContactPersonalTest.php 111 Drupal\Tests\contact\Functional\ContactPersonalTest->testPersonalContactAccess()
Did not find the exact reason of this failure.

The conversion of assertUniqueTextHelper is quite confusing here.
Mink does not provide any helper function out of the box to find the unique text on the page and we can't access the getPage() method of Webassert directly in AssertLegacyTrait. So not sure how to proceed with that.We have only responsePageContains/pageTextContains method exists in it and we have getTextContent method exists in BrowserTestBase

Here's the chunks of code that I tried :

  /**
   * Passes if the text is found MORE THAN ONCE on the text version of the page.
   *
   * The text version is the equivalent of what a user would see when viewing
   * through a web browser. In other words the HTML has been filtered out of
   * the contents.
   *
   * @param string|\Drupal\Component\Render\MarkupInterface $text
   *   Plain text to look for.
   * @param string $message
   *   (optional) A message to display with the assertion. Do not translate
   *   messages: use \Drupal\Component\Utility\SafeMarkup::format() to embed
   *   variables in the message text, not t(). If left blank, a default message
   *   will be displayed.
   * @param string $group
   *   (optional) The group this message is in, which is displayed in a column
   *   in test output. Use 'Debug' to indicate this is debugging output. Do not
   *   translate this string. Defaults to 'Other'; most tests do not override
   *   this default.
   *
   * @return bool
   *   TRUE on pass, FALSE on fail.
   *
   * @deprecated Scheduled for removal in Drupal 9.0.0.
   *   Use  $this->assertSession()->responseContains() or
   *   $this->assertSession()->pageTextContains() instead.
   */
  protected function assertNoUniqueText($text, $message = '', $group = 'Other') {
    return $this->assertUniqueTextHelper($text, $message, $group, FALSE);
  }

  /**
   * Helper for assertUniqueText and assertNoUniqueText.
   *
   * It is not recommended to call this function directly.
   *
   * @param string|\Drupal\Component\Render\MarkupInterface $text
   *   Plain text to look for.
   * @param string $message
   *   (optional) A message to display with the assertion. Do not translate
   *   messages: use \Drupal\Component\Utility\SafeMarkup::format() to embed
   *   variables in the message text, not t(). If left blank, a default message
   *   will be displayed.
   * @param string $group
   *   (optional) The group this message is in, which is displayed in a column
   *   in test output. Use 'Debug' to indicate this is debugging output. Do not
   *   translate this string. Defaults to 'Other'; most tests do not override
   *   this default. Defaults to 'Other'.
   * @param bool $be_unique
   *   (optional) TRUE if this text should be found only once, FALSE if it
   *   should be found more than once. Defaults to FALSE.
   *
   * @return bool
   *   TRUE on pass, FALSE on fail.
   *
   * @deprecated Scheduled for removal in Drupal 9.0.0.
   *   Use  $this->assertSession()->responseContains() or
   *   $this->assertSession()->pageTextContains() instead.
   */
  protected function assertUniqueTextHelper($text, $message = '', $group = 'Other', $be_unique = FALSE) {
    // Cast MarkupInterface objects to string.
    $text = (string) $text;
    if (!$message) {
      $message = '"' . $text . '"' . ($be_unique ? ' found only once' : ' found more than once');
    }
    $content_type = $this->getSession()->getResponseHeader('Content-type');
    // In case of a Non-HTML response (example: XML) check the original
    // response.
    if (strpos($content_type, 'html') === FALSE) {
      $this->assertSession()->responseContains($text);
    }
    else {
      $this->assertSession()->pageTextContains($text);
    }

    $first_occurrence = strpos($this->getTextContent(), $text);
    if ($first_occurrence === FALSE) {
      return $this->assert(FALSE, $message, $group);
    }
    $offset = $first_occurrence + strlen($text);
    $second_occurrence = strpos($this->getTextContent(), $text, $offset);
    return $this->assert($be_unique == ($second_occurrence === FALSE), $message, $group);
  }

Unassinging from myself until i get some clue/feedback on how to achieve / proceed with this.

naveenvalecha’s picture

Status: Needs work » Needs review
dawehner’s picture

@naveenvalecha
I think the right strategy is to add the helper function to WebAssert in Drupal itself ...

Status: Needs review » Needs work

The last submitted patch, 10: 2742995-9.patch, failed testing.

claudiu.cristea’s picture

Status: Needs work » Needs review
FileSize
5.6 KB
122.56 KB

Partial work. Still fails because it seems that \Behat\Mink\WebAssert::pageTextNotContains() is not correctly extracting the text from the HTML page.

Status: Needs review » Needs work

The last submitted patch, 14: 2742995-14.patch, failed testing.

dawehner’s picture

Thank you for this patch!

Please put

[diff]
  renames = copies

into your git config, see https://www.drupal.org/documentation/git/configure

Also note: This patch includes a book module change, even this issue is just about contact module :)

claudiu.cristea’s picture

@dawehner, yes I noticed. The patch is not usable. Sorry, I'll get back.

claudiu.cristea’s picture

Status: Needs work » Needs review
FileSize
20.46 KB

Here's the correct patch. interdiff is OK from #14

Status: Needs review » Needs work

The last submitted patch, 18: 2742995-18.patch, failed testing.

The last submitted patch, 18: 2742995-18.patch, failed testing.

The last submitted patch, 18: 2742995-18.patch, failed testing.

naveenvalecha’s picture

The 2742995-22.patch also used assertUniqueText from this patch and its also baked into it. https://www.drupal.org/node/2759879#comment-11419115

Also it added a new method assertCacheContext to AssertLegacytrait.php

The last submitted patch, 22: 2742995-22.patch, failed testing.

klausi’s picture

Status: Needs review » Needs work
  1. +++ b/core/modules/contact/tests/src/Functional/ContactPersonalTest.php
    @@ -67,7 +70,7 @@ function testSendPersonalContactMessage() {
    -    $mails = $this->drupalGetMails();
    +    $mails = $this->getMails();
    

    we should rename the methods same as SimpletTest does so that we don't need to change this line.

  2. +++ b/core/modules/contact/tests/src/Functional/ContactPersonalTest.php
    @@ -204,10 +207,10 @@ function testPersonalContactAccess() {
    -    $this->assertNoFieldChecked('edit-contact--2');
    +    $this->assertSession()->checkboxNotChecked('edit-contact--2');
    

    we should add the assertNoFieldChecked() method to the AssertLegacy trait instead and not change this line.

  3. +++ b/core/modules/contact/tests/src/Functional/ContactPersonalTest.php
    @@ -204,10 +207,10 @@ function testPersonalContactAccess() {
    -    $this->assertFieldChecked('edit-contact--2');
    +    $this->assertSession()->checkboxChecked('edit-contact--2');
    

    same here.

  4. +++ b/core/modules/contact/tests/src/Functional/ContactSitewideTest.php
    @@ -17,9 +18,10 @@
    -class ContactSitewideTest extends WebTestBase {
    +class ContactSitewideTest extends BrowserTestBase {
     
       use FieldUiTestTrait;
    +  use AssertMailTrait;
    

    I think for maximum compatibility we should add AssertMailTRait to BrowserTestBase instead to keep parity with SimpleTest.

  5. +++ b/core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php
    @@ -424,4 +577,18 @@ protected function buildXPathQuery($xpath, array $args = array()) {
    +    $this->assertTrue(in_array($expected_cache_context, $cache_contexts), "'" . $expected_cache_context . "' is present in the X-Drupal-Cache-Contexts header.");
    

    use assertCotains() instead.

dawehner’s picture

we should rename the methods same as SimpletTest does so that we don't need to change this line.

Well we should have some BC layer in place, but IMHO putting Drupal on there, just conceptually doesn't make sense.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Status: Needs review » Needs work

The last submitted patch, 27: browsertest-contact-2742995-27.patch, failed testing.

klausi’s picture

Status: Needs work » Needs review
FileSize
21.21 KB

And we also need #2784537: Add legacy assertFieldByXPath()/assertNoFieldByXPath() method for browser tests for this conversion patch, rolled that in.

Now all contact module tests are converted, except for Views tests and Update Path tests.

Status: Needs review » Needs work

The last submitted patch, 29: browsertest-contact-2742995-29.patch, failed testing.

claudiu.cristea’s picture

Status: Needs work » Postponed
  1. +++ b/core/modules/simpletest/tests/src/Functional/BrowserTestBaseTest.php
    @@ -86,4 +94,22 @@ public function testError() {
    +  /**
    +   * Tests that legacy assertions work.
    +   */
    +  public function testAssertions() {
    
    +++ b/core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php
    @@ -464,6 +479,103 @@ protected function assertNoFieldChecked($id) {
    +  protected function assertFieldByXPath($xpath, $value = NULL, $message = '') {
    ...
    +  protected function assertNoFieldByXPath($xpath, $value = NULL, $message = '') {
    ...
    +  protected function assertFieldsByValue($fields, $value = NULL, $message = '') {
    

    Shouldn't be here. Part of #2784537: Add legacy assertFieldByXPath()/assertNoFieldByXPath() method for browser tests.

  2. +++ b/core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php
    @@ -57,6 +58,9 @@ protected function assertElementNotPresent($css_selector) {
       protected function assertText($text) {
    +    // Cast MarkupInterface to string.
    +    $text = (string) $text;
    
    @@ -64,6 +68,10 @@ protected function assertText($text) {
    +      // Mink/BrowserKit uses PHP's DOMElement to get the value of an HTML tag.
    +      // It will provide the value HTML entity decoded, so we need to do the
    +      // same here for the text to actually match.
    +      $text = html_entity_decode($text);
    
    @@ -82,6 +90,9 @@ protected function assertText($text) {
       protected function assertNoText($text) {
    +    // Cast MarkupInterface to string.
    +    $text = (string) $text;
    
    @@ -89,6 +100,10 @@ protected function assertNoText($text) {
    +      // Mink/BrowserKit uses PHP's DOMElement to get the value of an HTML tag.
    +      // It will provide the value HTML entity decoded, so we need to do the
    +      // same here for the text to actually match.
    +      $text = html_entity_decode($text);
    
    +++ b/core/tests/Drupal/Tests/Core/Assert/AssertLegacyTraitTest.php
    @@ -154,6 +155,52 @@ public function testAssertOptionSelectedFail() {
       /**
    +   * @covers ::assertText
    +   */
    +  public function testAssertText() {
    ...
    +  /**
    +   * @covers ::assertNoText
    +   */
    +  public function testAssertNoText() {
    

    These changes were deferred to #2773733: Fix AssertLegacyTrait::assert(No)Text to handle html encoded strings (part 3).

  3. +++ b/core/tests/Drupal/Tests/DocumentElement.php
    @@ -0,0 +1,46 @@
    +/**
    + * Overrides the Mink document to better handle page text.
    + */
    +class DocumentElement extends MinkDocumentElement {
    +
    +  /**
    +   * {@inheritdoc}
    +   */
    +  public function getText() {
    

    So, the parent getText() doesn't remove style and script? Also here the output will contain encoded entities while parent::getText() return decoded entities (as a human would read them). See #2773733: Fix AssertLegacyTrait::assert(No)Text to handle html encoded strings (part 3). I think we should get first parent::getText() and apply changes against that. I'm not sure about the solution and, as we did in every conversion, this should be split in other issue.

Postponing on #2784537: Add legacy assertFieldByXPath()/assertNoFieldByXPath() method for browser tests and #2773733: Fix AssertLegacyTrait::assert(No)Text to handle html encoded strings (part 3).

naveenvalecha’s picture

Title: Convert web tests to browser tests for contact module » [PP-2]Convert web tests to browser tests for contact module

better title

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

jibran’s picture

Title: [PP-2]Convert web tests to browser tests for contact module » Convert web tests to browser tests for contact module
Status: Postponed » Needs work
Jo Fitzgerald’s picture

Issue tags: +Needs reroll
Jo Fitzgerald’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
12.73 KB

Re-roll. I'm not 100% certain of the file locations - the previous patch was noticeably different to 8.4.x.

klausi’s picture

Status: Needs review » Needs work

Looks like you forgot to move the files to the tests directory?

Jo Fitzgerald’s picture

As I said, I'm uncertain of the correct file locations, can you give me a hand @klausi? Which files should be where in this case? That should help me for future issues too. Thanks

klausi’s picture

they should be in tests/src/Functional of the module. See for example https://www.drupal.org/commitlog/commit/2/4c64f7b840ed4cbd1d075b3f409225...

Jo Fitzgerald’s picture

Status: Needs work » Needs review
FileSize
1.03 KB
15.22 KB

How about this then?

klausi’s picture

Status: Needs review » Needs work
  1. +++ b/core/modules/contact/tests/src/Functional/ContactSitewideTest.php
    @@ -386,14 +388,14 @@ function testAutoReply() {
    diff --git a/core/modules/contact/src/Tests/ContactStorageTest.php b/core/modules/contact/tests/src/Functional/ContactStorageTest.php
    
    diff --git a/core/modules/contact/src/Tests/ContactStorageTest.php b/core/modules/contact/tests/src/Functional/ContactStorageTest.php
    similarity index 98%
    
    similarity index 98%
    rename from core/modules/contact/src/Tests/ContactStorageTest.php
    
    rename from core/modules/contact/src/Tests/ContactStorageTest.php
    rename to core/modules/contact/tests/src/Functional/ContactStorageTest.php
    

    A rename is not enough, you also need to change the namespace in this file.

  2. +++ b/core/modules/contact/tests/src/Functional/ContactStorageTest.php
    @@ -1,6 +1,6 @@
    diff --git a/core/modules/contact/src/Tests/Update/ContactUpdateTest.php b/core/modules/contact/tests/src/Functional/Update/ContactUpdateTest.php
    
    diff --git a/core/modules/contact/src/Tests/Update/ContactUpdateTest.php b/core/modules/contact/tests/src/Functional/Update/ContactUpdateTest.php
    similarity index 97%
    
    similarity index 97%
    rename from core/modules/contact/src/Tests/Update/ContactUpdateTest.php
    
    rename from core/modules/contact/src/Tests/Update/ContactUpdateTest.php
    rename to core/modules/contact/tests/src/Functional/Update/ContactUpdateTest.php
    

    I think the update path tests are special and we might not be able to convert them in this issue.

  3. +++ b/core/tests/Drupal/FunctionalTests/BrowserTestBaseTest.php
    @@ -36,11 +36,19 @@ public function testGoTo() {
         // Test page contains some text.
         $this->assertSession()->pageTextContains('Test page text.');
    +    // Make sure CSS <style> content is not considered as part of the page text.
    +    $this->assertSession()->pageTextNotContains('@import');
    +    // Make sure JS setting values are not part of the page text.
    +    $this->assertSession()->pageTextNotContains('"baseUrl"');
     
    

    unrelated change, we should not change BrowserTestBaseTest in this issue.

  4. +++ b/core/tests/Drupal/Tests/Core/Assert/AssertLegacyTraitTest.php
    index 0000000..323bde2
    --- /dev/null
    
    --- /dev/null
    +++ b/core/tests/Drupal/Tests/DocumentElement.php
    

    same here, this is all coming from other patches form other issues and should not be included in this patch.

Jo Fitzgerald’s picture

Status: Needs work » Needs review
FileSize
2.2 KB
12.48 KB
  1. The namespace was changed as part of #35, is namespace Drupal\Tests\contact\Functional; not correct?
  2. I have moved Update/ContactUpdateTest.php back to where is was.
  3. These changes came from the patch in #29, as mentioned on another ticket, should I not be basing my patches on the existing patches? Or should I just start from scratch and ignore previous patches?
  4. Ditto.

Status: Needs review » Needs work

The last submitted patch, 42: 2742995-42.patch, failed testing.

Jo Fitzgerald’s picture

Status: Needs work » Needs review
FileSize
5.88 KB
5.64 KB
  • Removed non-relevant code.
  • Returned Views/ tests back to Tests/Views/.

Status: Needs review » Needs work

The last submitted patch, 44: 2742995-44.patch, failed testing.

bighappyface’s picture

I can reproduce the exception for Drupal\Tests\contact\Functional\ContactSitewideTest

$ php ./core/scripts/run-tests.sh --url http://127.0.0.1:8888 --verbose --color --class "Drupal\Tests\contact\Functional\ContactSitewideTest"

Drupal test run
---------------

Tests to be run:
  - Drupal\Tests\contact\Functional\ContactSitewideTest

Test run started:
  Friday, February 24, 2017 - 11:25

Test summary
------------

Drupal\Tests\contact\Functional\ContactSitewideTest            0 passes             1 exceptions             
FATAL Drupal\Tests\contact\Functional\ContactSitewideTest: test runner returned a non-zero error code (2).
Drupal\Tests\contact\Functional\ContactSitewideTest            0 passes   1 fails                            

Test run duration: 1 min 21 sec

Detailed test results
---------------------


---- Drupal\Tests\contact\Functional\ContactSitewideTest ----


Status    Group      Filename          Line Function                            
--------------------------------------------------------------------------------
Exception Other      phpunit-9.xml        0 Drupal\Tests\contact\Functional\Con
    PHPunit Test failed to complete; Error: PHPUnit 4.8.27 by Sebastian
    Bergmann and contributors.
    
    EE
    
    Time: 1.34 minutes, Memory: 7.00Mb
    
    There were 2 errors:
    
    1) Drupal\Tests\contact\Functional\ContactSitewideTest::testSiteWideContact
    Undefined property: Behat\Mink\Element\NodeElement::$td
    
    /workspace/drupal-8.4.x/core/modules/contact/tests/src/Functional/ContactSitewideTest.php:272
    
    2) Drupal\Tests\contact\Functional\ContactSitewideTest::testAutoReply
    PHPUnit_Framework_Exception: Fatal error: Call to undefined method
    Drupal\Tests\contact\Functional\ContactSitewideTest::drupalGetMails() in
    /workspace/drupal-8.4.x/core/modules/contact/tests/src/Functional/ContactSitewideTest.php
    on line 409
    

bighappyface’s picture

Same exception verified for Drupal\Tests\contact\Functional\ContactStorageTest

$ php ./core/scripts/run-tests.sh --url http://127.0.0.1:8888 --verbose --color --class "Drupal\Tests\contact\Functional\ContactStorageTest"

Drupal test run
---------------

Tests to be run:
  - Drupal\Tests\contact\Functional\ContactStorageTest

Test run started:
  Friday, February 24, 2017 - 11:30

Test summary
------------

Drupal\Tests\contact\Functional\ContactStorageTest             0 passes             1 exceptions             
FATAL Drupal\Tests\contact\Functional\ContactStorageTest: test runner returned a non-zero error code (2).
Drupal\Tests\contact\Functional\ContactStorageTest             0 passes   1 fails                            

Test run duration: 1 min 45 sec

Detailed test results
---------------------


---- Drupal\Tests\contact\Functional\ContactStorageTest ----


Status    Group      Filename          Line Function                            
--------------------------------------------------------------------------------
Exception Other      phpunit-10.xml       0 Drupal\Tests\contact\Functional\Con
    PHPunit Test failed to complete; Error: PHPUnit 4.8.27 by Sebastian
    Bergmann and contributors.
    
    .EE
    
    Time: 1.73 minutes, Memory: 7.25Mb
    
    There were 2 errors:
    
    1) Drupal\Tests\contact\Functional\ContactStorageTest::testSiteWideContact
    Undefined property: Behat\Mink\Element\NodeElement::$td
    
    /workspace/drupal-8.4.x/core/modules/contact/tests/src/Functional/ContactSitewideTest.php:272
    
    2) Drupal\Tests\contact\Functional\ContactStorageTest::testAutoReply
    PHPUnit_Framework_Exception: Fatal error: Call to undefined method
    Drupal\Tests\contact\Functional\ContactStorageTest::drupalGetMails() in
    /workspace/drupal-8.4.x/core/modules/contact/tests/src/Functional/ContactSitewideTest.php
    on line 409
claudiu.cristea’s picture

Status: Needs review » Needs work

The last submitted patch, 48: 2742995-48.patch, failed testing.

claudiu.cristea’s picture

Status: Needs work » Needs review
FileSize
6.99 KB
827 bytes

More clear.

bighappyface’s picture

Status: Needs review » Reviewed & tested by the community

+1 LGTM - #50 verified

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 50: 2742995-50.patch, failed testing.

Jo Fitzgerald’s picture

Status: Needs work » Needs review
FileSize
6.45 KB

Re-rolled.

Jo Fitzgerald’s picture

Status: Needs review » Reviewed & tested by the community

Setting back to RTBC in expectation.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 53: 2742995-53.patch, failed testing.

Jo Fitzgerald’s picture

Status: Needs work » Needs review
FileSize
7 KB

I forgot to move the files.

klausi’s picture

Status: Needs review » Reviewed & tested by the community

Looks good, thanks!

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed 9e85f54 to 8.4.x and 6755fc9 to 8.3.x. Thanks!

  • alexpott committed ac7313d on 8.4.x
    Issue #2742995 by Jo Fitzgerald, claudiu.cristea, naveenvalecha, klausi...

  • alexpott committed 380b52d on 8.3.x
    Issue #2742995 by Jo Fitzgerald, claudiu.cristea, naveenvalecha, klausi...

Status: Fixed » Closed (fixed)

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

RajabNatshah’s picture