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

Members fund testing for the Drupal project. Drupal Association Learn more

Comments

pfrenssen created an issue. See original summary.

pfrenssen’s picture

dawehner’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +Quickfix

+1 I've seen another issue about that though already

pfrenssen’s picture

Oh 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.

pfrenssen’s picture

pfrenssen’s picture

If this gets committed, please add commit credit to @RoySegall.

  • catch committed 58c3b96 on 8.2.x
    Issue #2752315 by pfrenssen, RoySegall: Fix visibility of...

catch credited RoySegall.

catch credited RoySegall.

catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.2.x, thanks!

Eric_A’s picture

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

Now that #2735199: Convert web tests to browser tests for help module made it into 8.1.x, this is needed too.

Eric_A’s picture

Status: Fixed » Reviewed & tested by the community

RTBC as per #3.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 2: 2752315-2.patch, failed testing.

Eric_A’s picture

Status: Needs work » Reviewed & tested by the community

18:33:40 Write error
18:33:40 An error was encountered while attempting to write https://www.drupal.org/files/issues/2752315-2.patch to /var/lib/drupalci/web/jenkins-default-160325

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 2: 2752315-2.patch, failed testing.

Eric_A’s picture

Sigh, I requested a standard retest for 8.1.x and it got queued against 8.2.x.

The last submitted patch, 2: 2752315-2.patch, failed testing.

The last submitted patch, 2: 2752315-2.patch, failed testing.

Eric_A’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
615 bytes
617 bytes

Ok, 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...

pfrenssen’s picture

RTBC+1 for the 2nd patch of #19.

catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.1.x, thanks!

  • catch committed b36b4f4 on 8.1.x
    Issue #2752315 by Eric_A, pfrenssen, RoySegall: Fix visibility of...

Status: Fixed » Closed (fixed)

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