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

Convert:
- Only ResponsiveImageAdminUITest

CommentFileSizeAuthor
#13 2863984-13.patch2.16 KBjofitz
#13 interdiff-2-13.txt3.04 KBjofitz
#2 convert_web_tests_to-2863984-2.patch7.03 KBgoz

Comments

GoZ created an issue. See original summary.

goz’s picture

goz commented
Status: Active » Needs review
Parent issue: » #2735005: Convert all Simpletest web tests to BrowserTestBase (or UnitTestBase/KernelTestBase)
StatusFileSize
new convert_web_tests_to-2863984-2.patch7.03 KB
dawehner’s picture

dawehner
Primary language German
commented

It feels like we should maybe have some JS test coverage for these forms, just wondering ...

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Needs work

Do you mind actually converting it to a JS test?

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs work » Closed (duplicate)

I guess its the same kind of case as #2780063: Convert web tests to browser tests for datetime and datetime_range modules

goz’s picture

goz commented

@dawehner are you sure about duplicate with #2780063: Convert web tests to browser tests for datetime and datetime_range modules ?
this seems to have nothing to do with datetime module.

michielnugter’s picture

michielnugter commented
Issue summary: View changes
Status: Closed (duplicate) » Postponed
Related issues: +#2809513: Convert AJAX part of \Drupal\responsive_image\Tests\ResponsiveImageFieldUiTest to JavascriptTestBase and the rest to BrowserTestBase

I don't think it's a duplicate either. There are some tests that should be Browser tests. I have postponed this one on the JavascriptTestBase conversion though: #2809513: Convert AJAX part of \Drupal\responsive_image\Tests\ResponsiveImageFieldUiTest to JavascriptTestBase and the rest to BrowserTestBase. We need to get that in first and then finish up this issue.

dawehner’s picture

dawehner
Primary language German
commented

I have no idea what I was thinking :P

goz’s picture

goz commented
Assigned: goz » Unassigned

@dawehner there is some days like that.

As other postponed WTB to BTB issues, may be we could create an issue for the postponed JavascriptTestBase and at least convert what can be converted right now ?

michielnugter’s picture

michielnugter commented
Issue summary: View changes
Status: Postponed » Needs work

I looked again at the module and there are 3 tests in the module:

- 1 can be converted directly without dependencies (ResponsiveImageAdminUITest)
- 1 has at least a partial conversion to JTB (ResponsiveImageFieldUiTest)
- 1 has a dependency on image (ResponsiveImageFieldDisplayTest).

I changed the scope for this issue to only the AdminUITest, increased the scope for the JavascriptTestBase conversion and moved ResponsiveImageFieldUiTest out of this scope and into the follow-up for image (yet to be created).

Can you reroll the patch with only the ResponsiveImageAdminUITest test?

dawehner’s picture

dawehner
Primary language German
commented
Title: Convert web tests to browser tests for responsive_image module » Convert [1/3] web tests to browser tests for responsive_image module

I'm wondering whether we should make it clear in the title how much of the tests are gonna be converted?

michielnugter’s picture

michielnugter commented
Issue tags: +phpunit initiative
jofitz’s picture

jofitz
he/him
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff-2-13.txt3.04 KB
new 2863984-13.patch2.16 KB

Rerolled the patch with only the ResponsiveImageAdminUITest test, as requested in #10.

michielnugter’s picture

michielnugter commented
Status: Needs review » Reviewed & tested by the community

Thanks, looks good to me!

dawehner’s picture

dawehner
Primary language German
commented

Nice. One step towards it!

  • catch committed 3a6a71a on 8.4.x 
    Issue #2863984 by Jo Fitzgerald, GoZ, michielnugter: Convert [1/3] web...
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.4.x, thanks!

Status: Fixed » Closed (fixed)

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