Comments

Sutharsan created an issue. See original summary.

sutharsan’s picture

Status: Active » Needs review
StatusFileSize
new35.98 KB

I found some abnormalities in ContentAccessBrowserTestBase for which I added TODOs.

gisle’s picture

This requires OOP style method $this->t() Adding as related: #2795683: Replace t() with this->t().

gisle’s picture

Status: Needs review » Needs work

Needs reroll against HEAD.

nitesh624’s picture

Assigned: Unassigned » nitesh624
gisle’s picture

Assigned: nitesh624 » Unassigned

No progress for two weeks – un-assigning nitesh624.

Before assigning yourself to an issue, please read the page about Issue Queue Etiquette and take note of recital #8 in the "things not to do"-list.

alexshipilov’s picture

Status: Needs work » Needs review
StatusFileSize
new24.19 KB

Status: Needs review » Needs work

The last submitted patch, 7: content_access-browsertest-2855968-7.patch, failed testing. View results

alexshipilov’s picture

Status: Needs work » Needs review

please review

Status: Needs review » Needs work

The last submitted patch, 7: content_access-browsertest-2855968-7.patch, failed testing. View results

gisle’s picture

Thanks for the patch, alexshipilov, I'll got some other commitments, but I'll review your patch ASAP. Don't pay any attention to the "Status" - this is set automatically.

alexshipilov’s picture

StatusFileSize
new48.14 KB

Sorry the patch was broken, going to apply new one

alexshipilov’s picture

gisle’s picture

Hi, alexshipilov,
thanks for the patch in #12.

It applies cleanly, and the tests now run, which is great!

However, they all fail 😞.

I must admit that I am new to browsertests, so the problem may not be with your patch, but with my way of testing.

I believe I have a working environment for browsertests.

Running one of the core tests:

vendor/bin/phpunit web/core/modules/datetime/tests/src/Unit/Plugin/migrate/field/DateFieldTest.php

works as one would expect.

However:

vendor/bin/phpunit web/modules/contrib/content_access/tests/src/Functional/ContentAccessAclTest.php

produces three errors from three tests.

I don't understand what causes this: "User warning: mkdir(): Permission Denied".
The directory "web/sites/simpletest/" exists and is world-writeable.

gisle’s picture

I've figured out the cause of "Permission denied".

It is explained on https://www.drupal.org/docs/automated-testing/phpunit-in-drupal/running-...

Functional tests have to be invoked with a user in the same group as the webserver user.

Doing that, the "Permission denied" user warning goes away.

However, most of the tests fail. For instance, the output from:

vendor/bin/phpunit web/modules/contrib/content_access/tests/src/Functional/ContentAccessAclTest.php

results in one pass and two errors:

PHPUnit 7.5.20 by Sebastian Bergmann and contributors.

Testing Drupal\Tests\content_access\Functional\ContentAccessAclTest
.EE                                                                 3 / 3 (100%)

Time: 5 minutes, Memory: 6.00 MB

There were 2 errors:

1) Drupal\Tests\content_access\Functional\ContentAccessAclTest::testEditAccess
Behat\Mink\Exception\ResponseTextException: The text "Access denied" appears in the text of this page, but it should not.

/var/www/example.com/vendor/behat/mink/src/WebAssert.php:785
/var/www/example.com/vendor/behat/mink/src/WebAssert.php:279
/var/www/example.com/web/modules/contrib/content_access/tests/src/Functional/ContentAccessAclTest.php:152

2) Drupal\Tests\content_access\Functional\ContentAccessAclTest::testDeleteAccess
Behat\Mink\Exception\ResponseTextException: The text "Access denied" appears in the text of this page, but it should not.

/var/www/example.com/vendor/behat/mink/src/WebAssert.php:785
/var/www/example.com/vendor/behat/mink/src/WebAssert.php:279
/var/www/example.com/web/modules/contrib/content_access/tests/src/Functional/ContentAccessAclTest.php:197

ERRORS!
Tests: 3, Assertions: 64, Errors: 2.

HTML output was generated
[… removed links to HTML output]

Remaining deprecation notices (6)

  3x: Drupal\Tests\BrowserTestBase::$defaultTheme is required in drupal:9.0.0 when using an install profile that does not set a default theme. See https://www.drupal.org/node/3083055, which includes recommendations on which theme to use.
    1x in ContentAccessAclTest::testViewAccess from Drupal\Tests\content_access\Functional
    1x in ContentAccessAclTest::testEditAccess from Drupal\Tests\content_access\Functional
    1x in ContentAccessAclTest::testDeleteAccess from Drupal\Tests\content_access\Functional

  3x: \Drupal\Core\Session\AccountInterface::getUsername() is deprecated in Drupal 8.0.0, will be removed before Drupal 9.0.0. Use \Drupal\Core\Session\AccountInterface::getAccountName() or \Drupal\user\UserInterface::getDisplayName() instead. See https://www.drupal.org/node/2572493
    1x in ContentAccessAclTest::testViewAccess from Drupal\Tests\content_access\Functional
    1x in ContentAccessAclTest::testEditAccess from Drupal\Tests\content_access\Functional
    1x in ContentAccessAclTest::testDeleteAccess from Drupal\Tests\content_access\Functional

The legacy simpletest suite produced no errors. I am afraid this still needs work, as we need to figure out why these errors are reported.

gisle’s picture

StatusFileSize
new38.9 KB

Replaced deprecated calls to getUsername() with getAccountName().
Added protected $defaultTheme = "classy".
This fixes all deprecated notices.

Added a new browsertest file: ContentAccessTintTest.php. This is just a tiny test just to debug the PHPunit configuration (it passes, and is quicker to run than the other browsertests).

As for the original tests:

  • ContentAccessAclTest.php: Two out of three tests fail.
  • ContentAccessModuleTest.php: Four out of four tests fail.

So status is still "Needs work".

kunal_singh’s picture

Assigned: Unassigned » kunal_singh
kunal_singh’s picture

Assigned: kunal_singh » Unassigned
Status: Needs work » Needs review
StatusFileSize
new39.44 KB

Please review this patch

@gisle

  1. ContentAccessAclTest.php: Two test were failing because the test is performed on the basis of 'Add user' button text and on node/' . $this->node1->id() . '/access page we have multiple buttons with same Label. So I have changed the test to assert on the basis of button id.
  2. ContentAccessModuleTest.php: 4 test were because of a debug statement that was there in the ContentAccessTestHelperTrait.php file.

  • kunal_singh authored 230adda on 8.x-1.x
    Issue #2855968 by alexshipilov, gisle, Sutharsan, kunal_singh: : Migrate...
gisle’s picture

Status: Needs review » Fixed

Thank you, kunal_singh, for finding the final two errors.

And a big thank you to all who participated! This has now been committed to the latest development snapshot of the 8.x-1.x branch.

Status: Fixed » Closed (fixed)

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