Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Wrong message being asserted against. Not sure why this is not being caught when running all the tests. Ah because submodules can not have unit tests.
This is a major bug because we have test coverage that is not being run.
Comment | File | Size | Author |
---|---|---|---|
#23 | 2548061-23.patch | 1.39 KB | klausi |
Comments
Comment #2
alexpottComment #3
BerdirLooks like something else crept into the patch :)
Comment #6
hussainwebFixing the error.
Comment #7
hussainwebAlso, as pointed out by @Berdir in #3, there seems to be a lot of other things in the patch. After studying the patch, it seems to me that this (attached) is the only change required to bring the class into test. Please point out if this is wrong.
Comment #8
dawehnerThis doesn't make sense IMHO ... we are testing the test implementation, not something else ... so moving that test is kinda wrong, if you ask me.
Comment #9
Crell CreditAttribution: Crell as a volunteer commentedWhy are we testing a test implementation of a matcher...? Insert Yo Dawg meme here...
I skimmed through #6 and I'm not entirely sure what we're doing here.
Also moving to the routing system, as it's about matcher coverage.
Comment #10
Crell CreditAttribution: Crell as a volunteer commentedAh! Wait, now I see. This is the code we had to verify that after the switch to GET parameter format detection it would still be possible for contrib to re-add Accept-header based matching. That's why we have a Yo Dawg situation. :-)
So then, the problem here is JUST that the test is getting skipped, because testbot? Why was this picked up in PHP 7 testing, then?
Comment #11
stefan.r CreditAttribution: stefan.r commentedI think the problem is just the string, the dot disappeared in PHP7
Comment #12
BerdirIt's not related to PHP7 at all, I just found it when running unit tests (which I always do with phunit core/modules, which *does* pick this one up).
The expected exception message simply doesn't match the actual exception and we didn't notice because testbot doesn't run the test.
Comment #13
alexpott@Berdir it's not just testbot... go to core/ and run phpunit from there. This picks up phpunit.xml.dist and does not run the test either. It a known bug / feature.
Comment #14
Crell CreditAttribution: Crell at Palantir.net commentedOK, so side effect of a known issue. Then do we:
1) Move the test so that it gets run (and apparently seems to pass per #7)
2) Postpone on sorting out the known issue?
Comment #15
dawehnerWell, its not the worst to do this here, but yeah
needs to be changed as well.
Comment #16
tim.plunkettComment #17
tim.plunkettComment #19
tim.plunkettOh, missed #15.
Comment #23
klausiSystem module is the wrong place for unit tests, we should not add any there.
Moved to core/tests.
#2859538: system/tests/modules/accept_header_routing_test/tests are in the wrong place, never get executed was marked as duplicate of this.
Comment #24
Wim LeersComment #25
alexpottCommitted and pushed 97d9a7b to 8.4.x and 9f1d3c4 to 8.3.x. Thanks!
Backported to 8.3.x cause this is a test only.
removed unused use...