Closed (fixed)
Project:
Drupal core
Version:
9.4.x-dev
Component:
phpunit
Priority:
Minor
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
27 Feb 2022 at 07:14 UTC
Updated:
9 May 2022 at 16:34 UTC
Jump to comment: Most recent
Problem/Motivation
The docs say:
* This method is private as it must only be called once by
* BrowserTestBase::setUp() (multiple invocations for the same test would have
* unpredictable consequences) and it must not be callable or overridable by
* test classes.
but the function is not private, it's merely protected:
> protected function prepareEnvironment() {
Proposed resolution
Unclear whether it should be changed to private, or whether the docs need to be changed.
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Issue fork drupal-3266739
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #2
joachim commentedWrong status.
Comment #3
beatrizrodriguesComment #5
beatrizrodriguesI changed the documentation to "protected" because this method is used in a lot of classes inherited from InstallerTestBase, which inherits from BrowserTestBase, which is the class that the Trait is found indeed. So I think that "protected" is the best option here. Please, kindly review.
Comment #6
joachim commentedIf it's protected, then that paragraph in the docs isn't needed at all:
- firstly, it's not unusual for methods to be protected, so it doesn't need to be explained
- second, "it must not be callable or overridable by test classes." no longer makes any sense, since it IS callable and overridable.
Comment #7
beatrizrodrigues@joachim that's a good point. I didn't think about that. So, it seems to me that this part of the documentation can be removed, right? As I said, the method is being used in a lot of other test classes. I will apply that approach in the next commit.
Comment #8
beatrizrodriguesComment #9
joachim commentedComment #10
alexpottI think this was a c&p from Simpletest - the last
private function prepareEnvironment()was in the TestBase from simpletest removed in #3110862: Remove simpletest module from coreIt got made private in #2170023: Use exceptions when something goes wrong in test setup which is where this documentation was added. It's fair to say that for tests using FunctionalTestSetupTrait this has not been considered private.
The issue that added BrowserTestBase added it as protected but with the odd docs - see #2232861: Create BrowserTestBase for web-testing on top of Mink. I'm guessing that this was due to rerolls. It looks like it could have been private at that point but now that is just a dream.
We could open a follow-up to try to make this private but we'd potentially break contrib tests and we have plenty of working core tests which do override it.
Committed and pushed 317d85842e to 10.0.x and 40615d8f3c to 9.4.x. Thanks!