See #2735005: Convert all Simpletest web tests to BrowserTestBase (or UnitTestBase/KernelTestBase)
Tests moved:
EntityTypeWithoutLanguageFormTest.php
LanguageConfigurationElementTest.php
LanguageConfigurationTest.php
LanguageCustomLanguageConfigurationTest.php
LanguageListTest.php
LanguageLocaleListTest.php
LanguageSelectorTranslatableTest.php
LanguageSwitchingTest.php
LanguageUILanguageNegotiationTest.php
LanguageUrlRewritingTest.php
Tests not moved:
LanguageTourTest.php => #2871740: Convert web tests to browser tests LanguageTourTest for language module
LanguageSelectWidgetUpdateTest.php => For now #2808777: Research: Complex tests to convert
Comment | File | Size | Author |
---|---|---|---|
#46 | interdiff-2756059-43-46.txt | 284 bytes | GoZ |
#46 | convert_web_tests_to-2756059-46.patch | 23.33 KB | GoZ |
#43 | interdiff-2756059-41-43.txt | 3.6 KB | GoZ |
#43 | convert_web_tests_to-2756059-43.patch | 24.06 KB | GoZ |
#41 | convert_web_tests_to-2756059-41.patch | 20.76 KB | GoZ |
Comments
Comment #2
klausiContains #2750941: Additional BC assertions from WebTestBase to BrowserTestBase and some other refactorings that I will move to dedicated issues later.
Comment #4
naveenvalechaReroll after #2750941: Additional BC assertions from WebTestBase to BrowserTestBase
Also fixed couple of failures.
Comment #6
klausiAsserLegacyTrait contains a duplicated method now. Make sure to run at least one converted test to catch PHP fatal errors before posting the patch.
This change is not necessary. Let's leave those calls if they don't break.
Comment #7
klausiFixed one more test, this will still fail.
Comment #9
klausiAnd another round. There was a more complicated fix in a test case that worked by submitting invalid form data, which does not work with mink. Implemented that in a nicer way by creating the language entity over the UI and then silently deleting it in the background.
Still not done yet.
Comment #11
klausiThis should fix the rest. Integrated the Drupal javascript settings method from #2753179: Convert all color web tests to BrowserTestBase.
Comment #12
klausiExtracted the config trait change to #2759853: Create ConfigTestTrait to share code between PHPUnit and Simpletest.
Comment #13
klausiExtracted the JS settings method to #2759859: Implement getDrupalSettings() on BrowserTestBase for checking JS settings.
Comment #14
klausiExtracted the request header support to #2759863: Implement request header support for drupalGet() on BrowserTestBase.
Comment #15
klausiExtracted the assertion methods to #2759879: Additional assertions for WebAssert and AssertLegacyTrait, part 2.
Comment #16
Mile23Applying the patch gives us this, which is out of scope:
So I'm guessing the other trait issues will make those changes and this will eventually need re-work.
Should we postpone on those?
Comment #17
klausiAgreed, let's postpone on those.
Comment #19
Eric_A CreditAttribution: Eric_A commentedAll in, now.
Comment #20
klausiSimple reroll. We still need to wait for #2771547: In Browser and FunctionalJavascript tests SIMPLETEST_USER_AGENT cookie needs to be set every 5 seconds.
Comment #21
klausiComment #22
dawehnerThis change is a bit weird. This language wasn't needed before
Comment #23
dawehnerCertainly needs work based upon #22
Comment #25
jofitz CreditAttribution: jofitz at ComputerMinds commentedComment #26
GoZ CreditAttribution: GoZ at Centarro commentedLanguageTourTest will work with BTB after #2767275: Convert web tests to browser tests for tour module
I replace
by
Original code doesn't work anymore.
I can't do interdiff because #2770921: Feb 21st: Convert chunk of WTB to BTB by just moving classes, changing use statements adding traits already move some files and i cannot apply previous patch without fails.
Comment #27
jofitz CreditAttribution: jofitz at ComputerMinds commentedRemove changes that are not relevant to this issue.
Comment #28
jofitz CreditAttribution: jofitz at ComputerMinds commentedComment #31
jofitz CreditAttribution: jofitz at ComputerMinds commentedSorry for making this issue so messy. I tried. I failed.
Comment #32
jofitz CreditAttribution: jofitz at ComputerMinds commentedAddressed test failures.
Comment #33
dawehnerI went through the changes and they look alright!
I guess this was simply not needed before.
Comment #34
jofitz CreditAttribution: jofitz at ComputerMinds commented@dawehner FYI: That change is because the BTB version of drupalPostForm() has slightly different parameters to the WTB version.
Comment #35
dawehnerFair :)
Comment #36
klausino, we are not testing exceptions here. We want to test the actual page response here, so those annotations should not be added.
Comment #37
GoZ CreditAttribution: GoZ at Centarro commentedComment #39
GoZ CreditAttribution: GoZ at Centarro commentedAs done in #26, i try catch
\InvalidArgumentException
because we cannot send not existed site_default_language like before.I also keep @exception annotations removed which make tests success in case Exception is catched, which is not the purpose.
Comment #40
klausithis introduces long array syntax, but we only allow short array syntax now.
Hm, but this is changing the test? In the old test the last assertion was that the field is not checked, but now it is? That's probably because a POST request was never triggered (it was stopped by the InvalidArgumentException).
Can you go back to my original solution in #20? I know dawehner was against it, but I think it is still better than not doing the POST request. We could also issue an artificial POST request with Guzzle, but I like my approach better because it makes it clear what is going on from a user perspective.
Comment #41
GoZ CreditAttribution: GoZ at Centarro commentedI cannot generate interdiff since 8.4.x add to much changes in code.
I fix some array to short array.
I came back to solution in #20.
Comment #42
klausiLooks like you forgot to move the test files to their new destination. Can you make sure that all modified test files are in the correct place before you upload the next patch? Thanks!
Comment #43
GoZ CreditAttribution: GoZ at Centarro commentedSorry for the mess.
What to do with
core/modules/language/src/Tests/Update/LanguageSelectWidgetUpdateTest.php
?Does the conversion of this file should be done in another issue or in this one ?
Comment #44
klausiwe cannot move that file yet because the TourTestBase has not been converted yet.
Postponing on #2767275: Convert web tests to browser tests for tour module.
Yes, the UpdatePathTestBase are fine to leave untouched until we make progress with #2808777: Research: Complex tests to convert.
Comment #45
michielnugter CreditAttribution: michielnugter as a volunteer and at Synetic commentedWe can split the Tour test into a separate issue and try to get the rest for the Language module ready, setting back to needs work for that.
Comment #46
GoZ CreditAttribution: GoZ at Centarro commentedI revert LanguageTourTest conversion. Follow LanguageTourTest conversion on #2871740: Convert web tests to browser tests LanguageTourTest for language module
Comment #47
LendudeManually checked if everything was moved, updated the IS to reflect the changes done and the follow ups.
All previous feedback has been addressed and I see no issues with the current changes.
Comment #48
michielnugter CreditAttribution: michielnugter as a volunteer and at Synetic commentedComment #50
catchCommitted/pushed to 8.4.x, thanks!