Problem/Motivation
Proposed resolution
Fix it.
Remaining tasks
Review.
User interface changes
None.
API changes
None.
Data model changes
None.
| Comment | File | Size | Author |
|---|---|---|---|
| #13 | 2650362-9.patch | 564 bytes | heatherwoz |
| #8 | 2650362-8.patch | 558 bytes | dawehner |
| #2 | 2650362-2.patch | 829 bytes | wim leers |
Comments
Comment #2
wim leersLet's see if this breaks any tests.
Comment #6
wim leersBump…
Comment #7
dawehnerI think using $_SERVER removes some level of determinism. What about creating a hostname manually, like
localhost/exampleor so?Comment #8
dawehnerThis is a test which ensures it is always there.
Comment #12
mile23Comment #13
heatherwoz commentedRerolled patch. There was a conflict due to changes in the function in the line above. Readded the change below that.
Comment #14
dawehnerGiven that the test doesn't fail it looks like we no longer need the initial patch. I guess adding this additional test coverage could be helpful.
Comment #22
smustgrave commentedMoving to a minor as its been many years.
As mentioned in #14 since the parent issue landed would it be worth adding this additional assertion or not needed?
Comment #23
quietone commentedI don't see a bug here, changing to a task. The priority to set an issue to is defined on Issue Priority field (which does not refer to the number of years). As an example there is a Critical issue that is now 16 years old.
A parent issue is not mentioned in #14. What is the parent issue being referred to here?
This is retesting the patch on an unsupported version of Drupal instead of a supported version.
Comment #24
catchComment #25
longwaveArguably this code would be better in testSetUp() where we already check the path:
Comment #26
longwaveI was trying to understand where this is actually set up. run-tests.sh explicitly sets it:
but if I bypass this and run tests directly via PHPUnit, the test still passes. I think this is just because Symfony's
Request::create()method does this:I suppose this is still worth adding a test for if we have expectations on it elsewhere.