Problem/Motivation
A simple grep shows sometimes we use browser output directory as absolute path and sometimes relative. We should consolidate how we want to use it.
grep -inr browser_output core/
core/tests/Drupal/Tests/Listeners/HtmlOutputPrinterTrait.php:37: $this->browserOutputFile = tempnam($html_output_directory, 'browser_output_');
core/tests/Drupal/Tests/BrowserHtmlDebugTrait.php:121: $uri = $GLOBALS['base_url'] . '/sites/simpletest/browser_output/' . $html_output_filename;
core/tests/Drupal/Tests/BrowserHtmlDebugTrait.php:132: $browser_output_file = getenv('BROWSERTEST_OUTPUT_FILE');
core/tests/Drupal/Tests/BrowserHtmlDebugTrait.php:133: $this->htmlOutputEnabled = is_file($browser_output_file);
core/tests/Drupal/Tests/BrowserHtmlDebugTrait.php:135: $this->htmlOutputFile = $browser_output_file;
core/tests/Drupal/Tests/BrowserHtmlDebugTrait.php:137: $this->htmlOutputDirectory = DRUPAL_ROOT . '/sites/simpletest/browser_output';
core/phpunit.xml.dist:29: <!-- Example BROWSERTEST_OUTPUT_DIRECTORY value: /path/to/webroot/sites/simpletest/browser_output -->
Also see https://gitlab.com/weitzman/drupal-test-traits/merge_requests/39
the html files are not going into the BROWSERTEST_OUTPUT_DIRECTORY. instead they are going into /sites/simpletest/browser_output regardless.
Proposed resolution
We can convert BROWSERTEST_OUTPUT_DIRECTORY to always be relative webroot and convert to absolute/url when needs be that would mean \Drupal\Tests\Listeners\HtmlOutputPrinterTrait::setUpHtmlOutput has to adapt.
Remaining tasks
- Agree on the approach.
- Create patch.
- Commit it.
- Rejoice.
User interface changes
None
API changes
None
Data model changes
None.
| Comment | File | Size | Author |
|---|---|---|---|
| #17 | 2992069-nr-bot.txt | 168 bytes | needs-review-queue-bot |
| #6 | browser_output_directory-2992069-6.patch | 870 bytes | chi |
Issue fork drupal-2992069
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:
Comments
Comment #2
mile23BROWSERTEST_OUTPUT_DIRECTORYis where BTB stores the HTML it gets from guzzle, before processing it to be browsable fromsites/simpletest/browser_output/. It's supposed to be a temp directory which is cleaned up sometime after the test run.The artifacts *always* live in
sites/simpletest/browser_output/, regardless of whatBROWSERTEST_OUTPUT_DIRECTORYsays.We do that because we use multisite for testing anyway, so the HTML artifacts can live there, too. That way your web server configuration will allow you to browse the artifacts, and all the paths to assets will work and you can see CSS and images.
Basically:
BROWSERTEST_OUTPUT_DIRECTORYis poorly named. :-) It's really browsertest_temp_output_directory.Comment #3
jibranHow do we fix it for existing site testing?
Comment #4
mile23It seems to me that if you're still using all the traits that BTB uses, you'll be using guzzle for requests, and that
Drupal\Tests\BrowserHtmlDebugTraitcould be modified to put the files somewhere else.I'd guess you want to serve HTML with URLs to all your assets form the installed site. You don't want evidence of multisites at all.
HtmlOutputPrinterdoes this:That's the signal to
BrowserHtmlDebugTrait::initBrowserOutputFile()that it should put HTML somewhere.Unfortunately,
initBrowserOutputFile()has lines like this:So it's a hardcoded path.
The solution would be to reproduce or override
BrowserHtmlDebugTraitin your project and have it store the HTML files in the place you want. It can still useHtmlOutputPrinterandBROWSERTEST_OUTPUT_DIRECTORYto get a temp storage location, but then it would put the HTML files in some public place.You can see that
BTB::getResponseLogHandler()calls$this->htmlOutput($html_output);as a guzzle middleware, which connects BTB to the trait. That's where the HTML logging happens, soExistingSiteBasewould have to set that up, much likeBTB::initMink()does.If this is an easy change to
BrowserHtmlDebugTraitthat would be easy to accommodate in core without too much complexity, that'd be great. But I'm thinking this solution lives in Drupal Test Traits.Comment #5
pieterdcThis was a big WTF moment; realising BROWSERTEST_OUTPUT_DIRECTORY isn't as configurable as it seems to be from what you can read at phpunit.xml(.dist).
If found my way around it for our use case - GitLab CI artifacts - by copying the browser output from the Drupal directory to the GitLab CI directory. Much like what is illustrated here: https://forum.gitlab.com/t/gitlab-ci-artifacts-not-found/7588/2
Nevertheless, it would be very useful to
Comment #6
chi commentedQuick fix.
Comment #7
chi commentedAnother confusing thing here is that setting BROWSERTEST_OUTPUT_DIRECTORY may reset BROWSERTEST_OUTPUT_FILE variable.
https://github.com/drupal/drupal/blob/8.6.4/core/tests/Drupal/Tests/List...
Comment #13
dave reidOh my gosh yes this is poorly named environment variable. I just ran into this assumption in our CI that this directory would contain the artifacts I want to store for review and it was always empty. Using /web/sites/simpletest/browser_output worked as expected, but you also need this variable set in order to store results, ugh.
Comment #17
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #19
tstoecklerJust hit this as well. Noticed that
\Drupal\Tests\BrowserHtmlDebugTrait::htmlOutput()needs to be updated, as well.Comment #23
jibranThis needs a rebase.