Problem/Motivation
In #3264764-8: PhpUnitCliTest::testFunctionalTestDebugHtmlOutput fails if BROWSERTEST_OUTPUT_DIRECTORY is an empty string @alexpott said:
Imo we should move testFunctionalTestDebugHtmlOutput to a functional test because it runs functional tests and so has even more requirements than just BROWSERTEST_OUTPUT_DIRECTORY - it'll fail with Exception: You must provide a SIMPLETEST_BASE_URL environment variable to run some PHPUnit based functional tests. too.
On top of that:
In \Drupal\Tests\Core\Test\PhpUnitCliTest::testFunctionalTestDebugHtmlOutput we run \Drupal\Tests\image\Functional\ImageDimensionsTest twice to test Functional test either creates debug HTML output or warns if it can't.
I don't think its a good pattern to use an actual Functional test to test our tests. For instance if the test moves or is somehow altered to not produce HTML output we have a problem.
\Drupal\Tests\image\Functional\ImageDimensionsTest makes 10 HTTP requests, so 10 HTML pages, where we basically only need one.
Steps to reproduce
Proposed resolution
- Turn \Drupal\Tests\Core\Test\PhpUnitCliTest::testFunctionalTestDebugHtmlOutput into a Functional test.
and
- Introduce a new Functional test with one HTTP request, so 1 HTML page and use this in \Drupal\Tests\Core\Test\PhpUnitCliTest::testFunctionalTestDebugHtmlOutput.
Merge request link
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Issue fork drupal-3415321
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:
- 3415321-stop-using-actual
changes, plain diff MR !6213
Comments
Comment #3
spokjeComment #4
mondrakeThanks @Spokje! Actually there is also #3265459: Move testFunctionalTestDebugHtmlOutput to a functional test that is itself a follow-up of #3264764-8: PhpUnitCliTest::testFunctionalTestDebugHtmlOutput fails if BROWSERTEST_OUTPUT_DIRECTORY is an empty string. It'd probably make sense to enlarge the scope here and move "the test that tests the other test" itself to a functional one as suggested by @alexpott.
Comment #5
spokjeCrap, should have searched before going mental in here, thanks @mondrake.
Comment #6
mondrakeOr I may have been less lazy earlier... :)
Comment #7
spokjeHeh, so much for our new years resolutions this year...
Comment #8
mondrakeGave up a couple of weeks ago already :)
Comment #9
spokjeComment #10
spokjeClosed #3265459: Move testFunctionalTestDebugHtmlOutput to a functional test as a duplicate of this one, although that one is older.
This issue expands the scope and already has an MR for a part of the extended scope.
Comment #11
spokjeHeh, so much for our new years resolutions this year...
Comment #12
spokjeComment #13
mondrakeOne comment inline, other than that I think that’s all folks!
Comment #14
spokjeAnswered (or rather "questioned") in MR thread.
Comment #15
mondrakeIMHO the failure you get is absolutely legit, because if you set the env variable to nil, this https://git.drupalcode.org/project/drupal/-/blob/11.x/core/tests/Drupal/... kicks in and skips any output. Try passing a valid directory to the env variable and the test should pass.
I think @alexpott original comments stems from the consideration that functional tests usually run in an env context where all the required variables are set (think CI environments), otherwise they would fail massively.
Comment #16
spokjeAh, I see, you're obsoletely right.
Changed in the MR, thanks for explaining.
Comment #17
mondrakeLGTM now
Comment #18
longwaveOK, this makes total sense, and will confuse me less next time when this fails somehow as I won't get confused as to why ImageDimensionsTest is in the output.
Backported to 10.2.x to keep things in sync.
Committed and pushed 9926ebb005 to 11.x and f33f0a179c to 10.2.x. Thanks!