Page MenuHomePhabricator
Create Task
Maniphest T78680

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

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 Changes in Gerrit:
SubjectRepoBranchLines +/-
Make tests' event capsule flag clientValidated as booleanmediawiki/extensions/EventLoggingmaster+1 -1
Customize query in gerrit

Related Objects
Search...

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 added projects: MediaWiki-extensions-EventLogging, HHVM.
hashar changed Security from none to None.
hashar subscribed.
gerritbot added a project: Patch-For-Review.Dec 16 2014, 8:43 PM

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

gerritbot added a comment.Dec 16 2014, 8:56 PM

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

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

QChris mentioned this in rEEVL65143ca758a4: Make tests' event capsule flag clientValidated as boolean.Dec 16 2014, 8:57 PM
hashar added a comment.Dec 17 2014, 9:17 AM

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 subscribed.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?

hashar added a comment.Dec 18 2014, 11:05 AM

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.
In T78680#932191, @hashar wrote:

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 :-)

QChris mentioned this in rMEXT7ca8ed304cc7: Updated mediawiki/extensions Project: mediawiki/extensions/EventLogging….Mar 11 2015, 6:22 AM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits