MNT Fix unit test #11442
MNT Fix unit test #11442
Conversation
There was a problem hiding this comment.
Can you please explain how this is resolving the problem? It looks like in the failed test we expect a replica to be used (even for unmatched routes), but the primary is used instead.
Is this not just masking the problem? Won't unmatched routes still be using the primary?
|
The test intended it to hit a matched route, but that route was never added to the config. The test was accidentally passing before because in non recipe-core setups it finds a route, I'm guessing something that leads to an error page? However in recipe-core this does not appear to happen. All this PR is doing is making the unit test work as intended. If you think this behavior is wrong, then it should be done as a new card |
|
My point is more that it shouldn't matter whether the route is matched or not - it should always be using the replica, shouldn't it? If so, this PR doesn't fix the test, it makes the test pass when the test is correctly pointing out there's a problem. |
emteknetnz
force-pushed
the
pulls/6/fix-unit-test
branch
from
aa9988d to
fb2cf8a
Compare
|
Looks like there's no DB calls made at all when doing a simulated GET to a missing route in recipe-core, have made an inline comment about this |
Issue silverstripe/.github#320
Fixes a couple of SilverStripe\ORM\Tests\DBReplicaTest::testRoutes tests failing in recipe-core specifically which won't handle unmatched routes.
https://github.com/silverstripe/recipe-core/actions/runs/11468619556/job/31914133643