Failing tests on 1.3-devel

F21 commented 11 years ago

I have been testing the devel against ArangoDB's devel (usually the latest commit). When running the tests, 7 of them always fails. Since we are looking at moving to 1.3 and finalizing 1.2.1, I think now would be a good time to investigate:

Configuration read from /projects/ArangoDB-PHP/tests/phpunit.xml

...E..EE.......................................................  63 / 114 ( 55%)
E...........E........................EE............

Time: 2 seconds, Memory: 4.75Mb

There were 7 errors:

1) triagens\ArangoDb\AdminTest::testGetServerStatus
triagens\ArangoDb\ServerException: HTTP/1.1 404 Not Found

/projects/ArangoDB-PHP/lib/triagens/ArangoDb/Connection.php:278
/projects/ArangoDB-PHP/lib/triagens/ArangoDb/Connection.php:144
/projects/ArangoDB-PHP/lib/triagens/ArangoDb/AdminHandler.php:124
/projects/ArangoDB-PHP/tests/AdminTest.php:117

2) triagens\ArangoDb\AdminTest::testGetServerConnectionStatistics
triagens\ArangoDb\ServerException: HTTP/1.1 404 Not Found

/projects/ArangoDB-PHP/lib/triagens/ArangoDb/Connection.php:278
/projects/ArangoDB-PHP/lib/triagens/ArangoDb/Connection.php:144
/projects/ArangoDB-PHP/lib/triagens/ArangoDb/AdminHandler.php:192
/projects/ArangoDB-PHP/tests/AdminTest.php:156

3) triagens\ArangoDb\AdminTest::testGetServerRequestStatistics
triagens\ArangoDb\ServerException: HTTP/1.1 404 Not Found

/projects/ArangoDB-PHP/lib/triagens/ArangoDb/Connection.php:278
/projects/ArangoDB-PHP/lib/triagens/ArangoDb/Connection.php:144
/projects/ArangoDB-PHP/lib/triagens/ArangoDb/AdminHandler.php:220
/projects/ArangoDB-PHP/tests/AdminTest.php:205

4) triagens\ArangoDb\ConnectionTest::testGetStatus
triagens\ArangoDb\ServerException: HTTP/1.1 404 Not Found

/projects/ArangoDB-PHP/lib/triagens/ArangoDb/Connection.php:278
/projects/ArangoDB-PHP/lib/triagens/ArangoDb/Connection.php:144
/projects/ArangoDB-PHP/tests/ConnectionTest.php:44

5) triagens\ArangoDb\DocumentExtendedTest::testCreateDocumentWithCreateFromArrayGetbyExampleWithOptionsAndDeleteDocument
Undefined variable: resultingDocument

/projects/ArangoDB-PHP/tests/DocumentExtendedTest.php:168

6) triagens\ArangoDb\GraphExtendedTest::testSaveVertexReplaceUpdateAndRemove
triagens\ArangoDb\ServerException: HTTP/1.1 412 Precondition Failed

/projects/ArangoDB-PHP/lib/triagens/ArangoDb/Connection.php:278
/projects/ArangoDB-PHP/lib/triagens/ArangoDb/Connection.php:195
/projects/ArangoDB-PHP/lib/triagens/ArangoDb/GraphHandler.php:359
/projects/ArangoDB-PHP/tests/GraphExtendedTest.php:511

7) triagens\ArangoDb\GraphExtendedTest::testSaveVerticesAndSaveReplaceUpdateAndRemoveEdge
triagens\ArangoDb\ServerException: HTTP/1.1 412 Precondition Failed

/projects/ArangoDB-PHP/lib/triagens/ArangoDb/Connection.php:278
/projects/ArangoDB-PHP/lib/triagens/ArangoDb/Connection.php:195
/projects/ArangoDB-PHP/lib/triagens/ArangoDb/GraphHandler.php:628
/projects/ArangoDB-PHP/tests/GraphExtendedTest.php:603

FAILURES!
Tests: 114, Assertions: 693, Errors: 7.

The first 4 relates to the statistics not being available in 1.3 yet.

However, 5, 6 and 7 warrants some investigation to see why they are failing, when they worked fine against ArangoDB 1.2.

frankmayer commented 11 years ago

Looks like some behavioral change in ArangoDB...

frankmayer commented 11 years ago

I am so happy that we started writing tests for the client :smile:

frankmayer commented 11 years ago

Holding off with the 1.2.1 release because of this. It doesn't affect working with the 1.2 branch, but still, better to resolve anything before release.

F21 commented 11 years ago

Good idea :) I am currently working on other things, so am a bit busy to investigate this :(

F21 commented 11 years ago

After fixing this, we need to test it against 1.2 and 1.3 to make sure the change doesn't break backwards compatibility.

frankmayer commented 11 years ago

Yes, of course :smile: ... Unfortunately I am also still stuck in another project. We'll resume as time allows. :smiley:

F21 commented 11 years ago

I have found the root cause of the problems:

For 5, and issue has been filed with ArangoDB: https://github.com/triAGENS/ArangoDB/issues/491

For 6 and 7, the problem is caused by a combination of things. This snippet in updateVertex and updateEdge:

if (!is_null($revision)) {
        $params[ConnectionOptions::OPTION_REVISION] = $revision;
}

The tests are saving us here because 1.2.x's check against the rev query parameter was probably broken or does not exist, so that was why the tests are passing against it.

Effectively, here's what happens (in 'pseudo code'):

$doc1 = createFromArray(..)
$doc2 = createFromArray(..)
$doc1a = createFromArray(..)

$handler->save($doc1); //doc1 now has revision id
$handler->save($doc2); //doc2 now has revision id

$handler->replace($doc1id, $doc1a) //doc1a now has revision id

$handler->replace($doc1id, $doc1) //doc1 now has latest revision id

$handler->update($doc1id, $doc1a) //revision id mismatch! Exception thrown here.

Now, you might say, why is it that it only happens on update? replaceVertex and replaceEdge also supports a rev parameter to replace conditionally. Well, the answer is that there is no code in replace*() to actually send the $params or options to the server, so this is also another bug.

So, we need to fix this:

[x] in replace*() add the code to merge $params or options into the URL. This can take the relevant lines from update*().
[x] in replace*() and update*(), do not automatically append the revision to the request if one exists in the document. Instead, the revision could be perhaps passed in manually by the user. Not sure if this is the best way, but this is one solution.

F21 commented 11 years ago

Delayed to 1.3

F21 commented 11 years ago

Quick update for 5: It only occurs on 32-bit machines.

F21 commented 11 years ago

@frankmayer Let's close this :smile: 5 is due to 32-bit machines and Jan says he has fixed it. I will test that locally since travis uses 64-bit :smiley:

frankmayer commented 11 years ago

Great! :+1:

F21 commented 11 years ago

Reopening:

This is failing on the latest build of ArangoDB (bb7e3205774d202612248d02d61b4e9a401dcc76) on my 32-bit machine:

Time: 3 seconds, Memory: 4.75Mb

There was 1 error:

1) triagens\ArangoDb\BatchTest::testEmptyBatch
triagens\ArangoDb\ServerException: HTTP/1.1 409 Conflict

/www/ArangoDB-PHP/lib/triagens/ArangoDb/Connection.php:278
/www/ArangoDB-PHP/lib/triagens/ArangoDb/Connection.php:161
/www/ArangoDB-PHP/lib/triagens/ArangoDb/CollectionHandler.php:411
/www/ArangoDB-PHP/lib/triagens/ArangoDb/CollectionHandler.php:350
/www/ArangoDB-PHP/tests/BatchTest.php:24

frankmayer commented 11 years ago

I have seen this error, when I still had testcollections in ArangoDB which were not removed for some reason (for example when debugging and exiting before the phpunit teardown method could drop the collections).

F21 commented 11 years ago

I am investigating this to see how we can prevent it.

F21 commented 11 years ago

This is really strange. Suddenly, I am unable to reproduce it at all!

I will close this for now and hope we have enough logging in the future to fix it if it ever turns up again :smile:

frankmayer commented 11 years ago

Under normal conditions it won't happen. If it happens, just re-run the test. It can happen at so many commands, that it doesn't make sense to dig in that further... :smile:

arangodb / arangodb-php

Failing tests on 1.3-devel #95