In #2735199: Convert web tests to browser tests for help module a new method assertNoEscaped()
was added to the AssertLegacyTrait
trait. The method is currently public, but it should be protected to be consistent with the other assertions in these traits.
This is currently causing BTB tests to fail in contrib. See for example this test for Organic Groups, it fails with the following error now:
Fatal error: Access level to Drupal\simpletest\AssertContentTrait::assertNoEscaped() must be public (as in class Drupal\Tests\BrowserTestBase) in /home/travis/build/amitaibu/og/og_ui/tests/src/Functional/BundleFormAlterTest.php on line 24
Comment | File | Size | Author |
---|---|---|---|
#19 | 2752315-19-even-better.patch | 617 bytes | Eric_A |
#19 | 2752315-19.patch | 615 bytes | Eric_A |
#2 | 2752315-2.patch | 614 bytes | pfrenssen |
Comments
Comment #2
pfrenssenSimple 1 line fix, similar to the one from #2744089: Fix visibility of AssertLegacyTrait::assertLink().
Comment #3
dawehner+1 I've seen another issue about that though already
Comment #4
pfrenssenOh yes indeed this was already reported by @RoySegall, who was also hacking on OG: #2751711: Fix visibility of AssertContentTrait::assertNoEscaped(), I didn't see that.
I'm leaving this up since here the visibility is consistent with the others, while in Roy's patch it is inverted.
Comment #5
pfrenssenComment #6
pfrenssenIf this gets committed, please add commit credit to @RoySegall.
Comment #10
catchCommitted/pushed to 8.2.x, thanks!
Comment #11
Eric_A CreditAttribution: Eric_A commentedNow that #2735199: Convert web tests to browser tests for help module made it into 8.1.x, this is needed too.
Comment #12
Eric_A CreditAttribution: Eric_A commentedRTBC as per #3.
Comment #14
Eric_A CreditAttribution: Eric_A commentedComment #16
Eric_A CreditAttribution: Eric_A commentedSigh, I requested a standard retest for 8.1.x and it got queued against 8.2.x.
Comment #19
Eric_A CreditAttribution: Eric_A commentedOk, a cherry-pick worked, but the patch failed to apply with git apply because the white space context is not in 8.1.x, which did not get the coding standards fix that was added on commit to 8.2.x.
Here are two patches: a rerolled one with the actual context instead of the white space and one that additionally adds the coding standards fix to 8.1.x.
The first is RTBC per #3 and the other per the code snippet in #2735199-19: Convert web tests to browser tests for help module...
Comment #20
pfrenssenRTBC+1 for the 2nd patch of #19.
Comment #21
catchCommitted/pushed to 8.1.x, thanks!