- Issue created by @joachim
- 🇬🇧United Kingdom joachim
Well, this is turning out to be way more complicated than I anticipated, because all the helper methods called by setUp() set properties on the test object, e.g.:
$this->siteDirectory = $test_db->getTestSitePath();
For all of these to be called by setUpBeforeClass(), they can't do that, because setUpBeforeClass() is a static method.
It's going to be complicated to unpick, but I think potentially beneficial - there is a mishmash of methods that return things and method that set properties. I've already found one thing that doesn't look right:
bootEnvironment() calls $this->getDatabaseConnectionInfo() twice, and $this->getDatabaseConnectionInfo() makes use of $this->databasePrefix, but during the first call, $this->databasePrefix is not yet initialised!
- 🇺🇸United States mile23 Seattle, WA
Maybe make issues about the db and prefix stuff if it's worrisome, and soft-postpone this on that...?
- 🇬🇧United Kingdom joachim
Agreed -- maybe several small issues that refactor the setUp() helpers.
- 🇷🇺Russia Chi
KernelTestBase
needs deep refactoring anyway. I think it's worth to create a new base class for kernel tests. Also$runTestInSeparateProcess
needs to be disabled as it will casesetUpBeforeClass
andtearDownAfterClass
hooks run after each test method. - 🇬🇧United Kingdom joachim
I discussed this with @Alex Pott at DrupalCon Lille a few months ago.
His comment IIRC was along the lines of 'you can usually just comment parts out of the code'.
Here's an example where AFAICT, you can't. I want to test all the possible combinations of 4 factors. So I'm using a cross product setup. To do so with a @dataProvider would mean a kernel test setup for each combination, which is needlessly expensive.
So I'm doing the cross product manually in the test method like this:
foreach ([FALSE, TRUE] as $permission_access) { foreach ([FALSE, TRUE] as $operand_access) { foreach ([FALSE, TRUE] as $operability) { foreach ([FALSE, TRUE] as $reachable) { $this->state->set('test_mocked_control:permission_access', $permission_access ? AccessResult::allowed() : AccessResult::forbidden()); $this->state->set('test_mocked_control:operand_access', $operand_access ? AccessResult::allowed() : AccessResult::forbidden()); $this->state->set('test_mocked_control:operability', $operability); $this->state->set('test_mocked_control:next_state', $reachable ? 'cake' : NULL); // Generate the links. $links = $action_link->getStateActionPlugin()->buildLinkArray($action_link, $user_no_access); // No access always means no links. if (!$permission_access || !$operand_access()) { $this->assertEmpty($links, implode(':', [$permission_access, $operand_access, $operability, $reachable])); }
But this has a number of drawbacks:
- I have to ensure each assertion has a custom message otherwise I will have no idea which set of values caused a failure
- furthermore, I have to use cross product values which are easily stringable, so the message can be easily made without complex code. This harms DX, as it would have been easier to read if the values for the access factors were AccessResult objects -- the checks further down would be easier to read than the boolean check for instance
- If there are any failures, I only see one. With a data provider, I get a complete overview of what's wrong with the system
- I can't run just a single combination for debugging. Or well, I can if I rewrite the cross product code like this so that values can be commented out, but it still requires a lot of faffing about to isolate a single case:$permission_access_values = [ FALSE, TRUE, ]; $operand_access_values = [ FALSE, TRUE, ]; $operability_values = [ FALSE, TRUE, ]; $reachable_values = [ FALSE, TRUE, ]; foreach ($permission_access_values as $permission_access) { foreach ($operand_access_values as $operand_access) { foreach ($operability_values as $operability) { foreach ($reachable_values as $reachable) {
All of this is reinventing the wheel which PHPUnit provides with the data provider system.
- 🇧🇪Belgium attiks
We ran into the same issue, and converted our code to do something similar but using a flag
inlineDataProvider
on the base class, so whenever needed we can easily switch back to the normal behavior.Taken from https://github.com/UN-OCHA/un_date/blob/main/tests/src/Kernel/DateRender...
/** * Test with UTC timezone. * * @x-dataProvider providerTestDataUtc * @x-dataProvider providerTestDataRandom */ public function testDateRangeUtc($expected = NULL, $start = NULL, $end = NULL, $timezone = NULL) { if ($this->inlineDataProvider) { $data = array_merge( $this->providerTestDataUtc(), $this->providerTestDataRandom(), ); foreach ($data as $name => $row) { $expected = $row['expected']; $start = $row['start']; $end = $row['end']; $timezone = $row['timezone']; // assertions } } else { // assertions }
I also looked into changing the code, but it ain't straightforward since everything has to be static
- 🇬🇧United Kingdom joachim
Yeah, the big problem is that PHPUnit's once-per-class code is all static, and that's now how KernelTestBase is written.
One potential avenue is the TestDatabase class -- I filed an issue to clean it up, then spoke to @AlexPott about it at DrupalCon Lille where it turned out I'd completely misunderstood the purpose of the class because it's badly named (his words :). It's more of a 'holder of stuff about the test site'. So we could move test site setup stuff to that (and also rename it!)
Another avenue would be to have a testing kernel.
In the meantime, I've refactored your approach to hybrid tests a bit so that the assertions code is DRY:
/** * Data provider */ public function dataProvider() { return [ 'alpha' => [1, 2], 'beta' => [3, 4], ]; } /** * For development, restore the annotation below and comment out the one * on the quickForCI() method. * * @NOTtest * @dataProvider dataProvider */ public function expensiveForUseInDevelopment(...$parameters) { $this->doTest(...$parameters); } /** * @test */ public function quickForCI() { foreach ($this->dataProvider() as $data_set) { $this->doTest(...$data_set); } } /** * Test the something thing. * * @param [type] $one * @param [type] $two */ protected function doTest($one, $two) { $this->assertIsNumeric($one); }