Follow-up to #2742995: Convert web tests to browser tests for contact module

See #2735005: Convert all Simpletest web tests to BrowserTestBase (or UnitTestBase/KernelTestBase)

CommentFileSizeAuthor
#4 interdiff-2889352-2-4.txt759 bytesnaveenvalecha
#4 2889352-4.patch2.25 KBnaveenvalecha
#2 2889352.patch2.43 KBnaveenvalecha

Comments

naveenvalecha created an issue. See original summary.

naveenvalecha’s picture

naveenvalecha
He/Him
commented
Status: Active » Needs review
StatusFileSize
new 2889352.patch2.43 KB

Here we go

//Naveen

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented 
+++ b/core/modules/contact/tests/src/Functional/Views/ContactLinkTest.php
@@ -39,10 +39,12 @@ class ContactLinkTest extends ViewTestBase {
+    if ($import_test_views) {

Why do we need this?

naveenvalecha’s picture

naveenvalecha
He/Him
commented
StatusFileSize
new 2889352-4.patch2.25 KB
new interdiff-2889352-2-4.txt759 bytes

@jibran,
we don't need this :) Removed this. Thanks for your review.

//Naveen

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Status: Needs review » Reviewed & tested by the community

Thanks!

dawehner’s picture

dawehner
Primary language German
commented

Maybe a child class want's to defer the importing to do some questions, but yeah I guess in this case we don't need it.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/modules/contact/tests/src/Functional/Views/ContactFieldsTest.php
@@ -28,8 +28,8 @@ class ContactFieldsTest extends ViewTestBase {
+  protected function setUp($import_test_views = TRUE) {
+    parent::setUp($import_test_views);

+++ b/core/modules/contact/tests/src/Functional/Views/ContactLinkTest.php
@@ -39,8 +39,8 @@ class ContactLinkTest extends ViewTestBase {
+  protected function setUp($import_test_views = TRUE) {
+    parent::setUp($import_test_views);

Hm, we did not do this before, why do we need it now?

naveenvalecha’s picture

naveenvalecha
He/Him
commented
Status: Needs work » Needs review

To fix the warning b/c ViewTestBase::setup requires the argument $import_test_views

Back to N/R
//Naveen

lendude’s picture

lendude
Primary language Dutch
Location Amsterdam
commented
Status: Needs review » Reviewed & tested by the community

The child class setUp() needs to match the parent, so the change is correct.

  • Gábor Hojtsy committed b6f1820 on 8.3.x 
    Issue #2889352 by naveenvalecha, jibran, dawehner, Lendude: Convert...

  • Gábor Hojtsy committed 09ee23b on 8.4.x 
    Issue #2889352 by naveenvalecha, jibran, dawehner, Lendude: Convert...
gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Reviewed & tested by the community » Fixed

Makes sense. Thanks for setting me straight :) Committed :)

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Version: 8.4.x-dev » 8.3.x-dev

Status: Fixed » Closed (fixed)

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