Problem/Motivation

While we support test-specific overrides in the form of settings.testing.php and testing.services.yml for WebTestBase tests we only support testing.services.yml for KernelTestBase but not settings.testing.php.

Proposed resolution

Also support settings.testing.php for KernelTestBase tests.

Remaining tasks

- Reviews
- Tests

User interface changes

None.

API changes

None.

Comments

tstoeckler’s picture

Status: Active » Needs review
Related issues: +#2229011: Tests are no longer modifiable
StatusFileSize
new2.32 KB

Here we go. This still needs (automated) tests, but I verified that this works locally.

tstoeckler’s picture

So this was reported by @chx, who needs this to be able to properly test the MongoDB suite. As mentioned in #2229011: Tests are no longer modifiable there are other possible use-cases (those apply to KernelTestBase tests as much as WebTestBase).

Also note that it's totally my fault that this currently does not work. I implemented the testing.services.yml support for KernelTestBase tests in the referenced issue, but I cannot fathom why I stopped at the halfway mark.

Status: Needs review » Needs work

The last submitted patch, 1: 2395901-1.patch, failed testing.

tstoeckler’s picture

Assigned: Unassigned » tstoeckler

Don't want to keep anyone from working on this, but keeping this on my radar.

dawehner’s picture

--- nevermind everything is alright ---

dawehner’s picture

StatusFileSize
new2.47 KB

Tried to write some test but so far no success.

chx’s picture

Status: Needs work » Needs review
StatusFileSize
new2.59 KB
new28.73 KB

Broken reroll.

Status: Needs review » Needs work

The last submitted patch, 7: 2395901_7.patch, failed testing.

chx’s picture

Status: Needs work » Needs review
StatusFileSize
new2.63 KB

Rerolled at least. Needs review indeed.
Also this is what I get with mongodb:

Status: Needs review » Needs work

The last submitted patch, 9: 2395901_9.patch, failed testing.

tstoeckler’s picture

Assigned: tstoeckler » Unassigned

Thanks! This being assigned to me is a lie at this point... ..sadly. Hope I manage to get back to this soon.

chx’s picture

Status: Needs work » Needs review
StatusFileSize
new2.89 KB

OK this is the minimal disruption version: if there are no overrides, no change happens. At all.

chx’s picture

StatusFileSize
new3.02 KB

Aaaargh wrong patch. Too many development directories!

The last submitted patch, 12: 2395901_12.patch, failed testing.

dawehner’s picture

StatusFileSize
new4.45 KB
new2.43 KB

So what about something like that? Note: This is some logic more similar to the one in WebTestBase but yeah, we don't cover all lines of code.

tstoeckler’s picture

Issue tags: -Needs tests

The test looks great. It should generally be sufficient in terms of coverage IMO although SimpleTestTest contains

# Make sure that the $test_class variable is defined when this file is included.
if ($test_class) {
}

in the test settings.testing.php. I think that could easily be added here without any further assertions.

I have one minor nitpick:

+    file_put_contents($settings_testing_file, $settings_file);

The variable names are a bit confusing, because the first is a filepath and the second are the file contents but the names are very similar.

chx’s picture

StatusFileSize
new4.53 KB
new1023 bytes
dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Looks perfect now!

The last submitted patch, 6: 2395901-6.patch, failed testing.

tstoeckler’s picture

Awesome, thanks!

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Makes sense. That + $php = <<<'EOS' stuff is wackadoo, but apparently we do this elsewhere in HEAD according to grep.

Eyeballing WebTestBase.php, this seems to cover the same bases, and has been RTBC for awhile if anyone had any other concerns.

Committed and pushed to 8.0.x. Thanks!

  • webchick committed a701df0 on 8.0.x
    Issue #2395901 by chx, dawehner, tstoeckler: Allow the same test-...
chx’s picture

StatusFileSize
new451.58 KB

Status: Fixed » Closed (fixed)

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