Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 UTC on 18 March 2024, to get $100 off your ticket.
Problem/Motivation
ConditionFormTest fails currently with PostgreSQL as database backend.
Proposed resolution
Identify and fix the failing tests.
Remaining tasks
Write patch.
User interface changes
None.
API changes
None.
Comment | File | Size | Author |
---|---|---|---|
#5 | 2443653-5.patch | 1.96 KB | tstoeckler |
Comments
Comment #1
bzrudi71 CreditAttribution: bzrudi71 commentedOrder matters - at least for PostgreSQL. Let's see what MySQL thinks about this...
Comment #2
daffie CreditAttribution: daffie commentedIt all looks good to me.
I can confirm that the test fails for postgreSQL and with the patch the test passes for postgreSQL.
So for me it is RTBC.
Comment #3
catchThis looks really fragile, why does the order matter? Would prefer making the test not rely on the order.
Comment #4
webchickHuh. Sorry, but could you explain this one a bit more? Why does simply changing the order of the two lines matter to PostgreSQL? (Feel free to mark back to RTBC when this is answered, though maybe we should add a code comment here as well.)
Comment #5
tstoecklerI'm assuming the line that is currently failing is
where the message that is output depends on the order of the bundles in the configuration.
Because the order is otherwise irrelevant - especially for this test - I think it makes more sense to instead make the output message less fragile.
Attached patch solves this by simply outputting a separate message for each bundle. Entirely untested.
Comment #6
daffie CreditAttribution: daffie commentedIt all looks good to me.
I can confirm that the test fails for postgreSQL and with the patch the test passes for postgreSQL.
I think that this is a better solution then the patch from comment #1.
So for me it is RTBC.
Comment #7
bzrudi71 CreditAttribution: bzrudi71 commentedYep, #5 looks much better. Didn't know that we are allowed to change tests in beta phase. Thanks a tstoeckler!
Comment #9
webchickComment #10
webchickThat's better, thanks. :) And yep, tests aren't part of the public-facing API so we can still change those.
Committed and pushed to 8.0.x. Thanks!
(Huh. D.o ate this the first time?)