Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Problem/Motivation
Based on this change record: https://www.drupal.org/node/2913019
...it is important that build tests use the .ht.router.php file provided by Drupal when they are serving requests via the PHP HTTP server.
The php built-in server has a small "bug" where it assumes all URLs w/ a period it them are for static assets. The .ht.router.php file overrides this functionality and looks for a resolvable route. If it finds one, it uses it instead of falling back on a static file. In the test case added here, .well-known/change-password
is a route in user.module. And its pretty easy to see it fails.
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Comment | File | Size | Author |
---|---|---|---|
#11 | 3085728-10.patch | 4.66 KB | heddn |
#11 | 3085728-tests_only.patch | 3.56 KB | heddn |
Comments
Comment #2
heddnAutomatic updates module uses a version strings (with periods in them) for its testing. Here's the diff of what I do over top of BTB to get those tests to pass.
Comment #3
heddnLet's start the discussion.
Comment #4
Mile23This looks fine. We should also document in the docblock that .ht.router.php will be used if it's present.
Comment #5
heddnComment #6
Mile23Excellent. :-)
Comment #7
webchickGiven 8.5.x is already not supported, and definitely not by the time 8.8.x ships, do we need the conditional?
Also, this may be a silly question, but is there anyway to test this? That's a pretty glaring bug to not use a file we're depending on, no?
Comment #9
heddnWe can't always be sure how folks will use build tests. They might want to use it with 1) non-drupal install 2) very old drupal install 3) something else I can't think of where the router doesn't exist.
There is a way to test it and I can add a test.
Comment #10
heddnInterdiff is the tests only patch. Note, this pulls in quickstart base from #3082230: [meta] Convert some tests to Build tests. Which just means that meta will need a re-roll if it lands here first.
Comment #11
heddnI really muffed up that test only patch. Here are updated patches.
Comment #13
heddnYeah, tests passed.
Comment #14
Mile23This counts as a test improvement for a new testing feature in 8.8.0, so we can target for 8.8.x.
Comment #15
heddnComment #16
Mile23LGTM.
Comment #17
alexpottCommitted and pushed f6b2481aee to 9.0.x and 2a58c7ef79 to 8.9.x. Thanks!
Will backport this to 8.8.x once the code freeze is over.
Comment #20
alexpottDiscussed with @catch we agreed this was good to backport to 8.8.x