Refactored reference update #231
Conversation
|
the tests fail because travis is using development builds of phpunit and there was a hickup with something (there was a discussion somewhere else but can't remember where) - the solution was to install our own phpunit. |
dbu
force-pushed
the
allow_duplicate_multivalue_properties
branch
from
ad1690c to
7caa4c9
Compare
|
removed the offending phpunit option from the phpunit.xml.dist and rebased this on master, hope the fail goes away. |
|
now its a lot less errors. @dantleech can you check? |
dantleech
force-pushed
the
allow_duplicate_multivalue_properties
branch
from
7caa4c9 to
77b53b5
Compare
|
Updated. The |
|
See the associated test PR for this issue: phpcr/phpcr-api-tests#146 |
|
should we maybe not allow dev dependencies? |
|
@lsmith77 I guess we could disallow them by default |
|
do we have a flaky test case? |
|
yeah .. I retriggered the test .. should have copied the failure here .. |
|
merged phpcr/phpcr-api-tests#146 and now restarting the build |
|
this is the travis build that fails: https://travis-ci.org/jackalope/jackalope-doctrine-dbal/builds/48041560 i now reverted the phpcr-api-tests to get rid of the failure. can you please have another look @dantleech and create a new PR on the api tests that works with this branch? |
|
Do we know why the tests are flaky even with the reversion on the tests? |
|
hm, now everything is green. maybe i was too impatient with restarting the tests. |
|
but seems it needs a rebase |
dantleech
force-pushed
the
allow_duplicate_multivalue_properties
branch
from
3d3435b to
6fa2c42
Compare
|
Rebased.. lets see. |
|
okay. looks green again. can you try to rebuild the tests that i reverted, so that we actually test the feature? |
|
the test change that i removed is phpcr/phpcr-api-tests@7494a2a - reverting that commit and then cleaning them up would be a start. |
|
@dantleech can you adjust the phpcr api test dep to point to the branch of the PR so that we can see things running through? we can then revert that change before merging. |
dantleech
force-pushed
the
allow_duplicate_multivalue_properties
branch
from
6fa2c42 to
4158818
Compare
|
Updated composer (to link to the phpcr-api-tests branch) and also enabled container builds. Tests seem to pass - although there seems to be two builds, one of which fails with a seemingly unrelated error (/container/idExample not found). |
|
did somebody restart the tests? they are green now. only failing thing is hhvm because of DTD |
| @@ -1,5 +1,7 @@ | |||
| language: php | |||
|
|
|||
| sulu: false | |||
There was a problem hiding this comment.
do we need this? would be a bit awkward imho
There was a problem hiding this comment.
It should force container builds, which are faster: http://docs.travis-ci.com/user/workers/container-based-infrastructure/#Routing-your-build-to-container-based-infrastructure
|
merged the tests and reverted the dep in composer.json. changed if the tests are green, this can be merge imo. |
|
errr.. damned muscle memory. |
…perties Refactored reference update
This should also fix #222
This PR refactors some of the node reference logic, having no effect on the behavior but making things clearer and also preparing the way for when Doctrine DBAL will support bulk inserts.
Tests at phpcr/phpcr-api-tests#153