Page MenuHomePhabricator

EventLogging ValidateSchemaTest::testValidEvent() fails under HHVM
Closed, ResolvedPublic

Description

Running the PHPUnit tests under HHVM causes a stacktrace:

ValidateSchemaTest::testValidEvent
JsonSchemaException: Invalid node: expecting "string", got "boolean". Path: "Root node -> clientValidated"

extensions/EventLogging/includes/JsonSchema.php:425
extensions/EventLogging/includes/JsonSchema.php:458
extensions/EventLogging/includes/JsonSchema.php:432
extensions/EventLogging/EventLogging.php:146
extensions/EventLogging/tests/ValidateSchemaTest.php:118
tests/phpunit/MediaWikiTestCase.php:141

Dummy change used: https://gerrit.wikimedia.org/r/#/c/70126/

Failure: https://integration.wikimedia.org/ci/job/mwext-EventLogging-testextension-hhvm/1/testReport/

It passes under Zend: https://integration.wikimedia.org/ci/job/mwext-EventLogging-testextension-zend/6/testReport/

Details

Related Gerrit Patches:
mediawiki/extensions/EventLogging : masterMake tests' event capsule flag clientValidated as boolean

Event Timeline

hashar created this task.Dec 16 2014, 5:26 PM
hashar raised the priority of this task from to Needs Triage.
hashar updated the task description. (Show Details)
hashar changed Security from none to None.
hashar added a subscriber: hashar.

Change 180261 had a related patch set uploaded (by QChris):
Make tests' event capsule flag clientValidated as boolean

https://gerrit.wikimedia.org/r/180261

Patch-For-Review

Change 180261 merged by jenkins-bot:
Make tests' event capsule flag clientValidated as boolean

https://gerrit.wikimedia.org/r/180261

Any idea why it would fail under HHVM and not under Zend ?

One sure thing, the test pass now: https://integration.wikimedia.org/ci/job/mwext-EventLogging-testextension-hhvm/2/console

QChris added a subscriber: QChris.Dec 18 2014, 10:01 AM

I saw that the schema verifier took different code paths when figuring out data types.
But HHVM's choice looked saner and right.

The fact that the test passed on Zend is a bug in EventLogging from my point of view.

Do you think it's worth hunting it down?

Ideally we would have a test highlighting the issue. Your call on whether it is important to investigate, my main point was to have the tests to pass in both Zend and HHVM regardless of their usefulness/correctness :)

QChris closed this task as Resolved.Dec 22 2014, 8:54 AM
QChris claimed this task.

Ideally we would have a test highlighting the issue.

If the issue was be the EventLogging code itself, I'd agree.

But here acutally not the EventLogging code, but the EventLogging test was wrong and sent the code into undefined behaviour.

If we'd automatically test against human errors in tests, we'd do tests of tests ... meta-tests :-)