Page MenuHomePhabricator

Parsoid should run tests against the Cite parser tests, not just MediaWiki core's
Closed, ResolvedPublic

Description

Per discussion with C. Scott.

Event Timeline

Jdforrester-WMF raised the priority of this task from to Normal.
Jdforrester-WMF updated the task description. (Show Details)
Jdforrester-WMF added projects: Parsoid, Cite.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 30 2015, 3:19 PM
hashar added a subscriber: hashar.Sep 30 2015, 7:47 PM

TL;DR: we can merge the tests with a proper Jenkins job that is shared by both Parsoid and Cite.

For the mediawiki/services/parsoid we currently trigger:

  • parsoidsvc-php-parsertests : runs MediaWiki parserTests.php against the parserTests.txt copy in Parsoid repo
  • parsoidsvc-source-parse-tool-check, parsoidsvc-source-roundtrip-test-check : smoke tests
  • parsoidsvc-source-npm-0.8 / 0.10 : npm test

I guess the topic of this bug is the last job. It just clones mediawiki/services/parsoid. But we can make it to clone Cite and any other extensions having Parser tests.

The easiest way to find the .txt file is to rely on the extension.json file that keep track of the files:

{
    "ParserTestFiles": [
        "citeParserTests.txt"
    ]
}

So we can end up with a directory structure such as:

/src/parsoidsvc
/src/extensions/Cite
/src/extensions/Math

The parsoid test suite would need some logic to glob the extension.json files, grab the ParserTestFiles list then inject the tests in the test suite.

Might need an env variable to setup the looking path, or we can use whatever convention may fit.

Or the job can handle the extension.json and move the files under the parsoidSvc working copy whereever it will be recognized (e.g: tests/parserTests/thirdparties/).

Once adjusted, the job can be triggered on all the repositories participating, Making sure a Cite parser test is recognized by Parsoid before the Cite change is merged.

Well, it's always nicer if we can run npm test locally without relying on Jenkins magic. Which would mean adding the cite extension as a git submodule of Parsoid, but i'm not really sure we want to do that.

And note that Parsoid tests always merge cleanly to core because we run php parsertests before commit. But the converse isn't true: core frequently changes the PHP parser in ways that need to be synchronized with Parsoid, and core doesn't run Parsoid on its parserTests file to detect these cases. (And that's actually a feature, since otherwise parser changes would somehow have to land exactly simultaneously in both core and Parsoid before either of their jenkins tests would pass.)

A similar thing is likely to be true with the Cite/Math/etc extension.

Arlolra claimed this task.Feb 7 2017, 3:27 AM

Change 336568 had a related patch set uploaded (by Arlolra):
[WIP] T114256: Run tests against the Cite parserTests

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

Change 336873 had a related patch set uploaded (by Arlolra):
Run parsoid's cite parser tests as well

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

Change 336873 merged by jenkins-bot:
Run parsoid's cite parser tests as well

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

Change 336568 merged by jenkins-bot:
T114256: Run tests against the Cite parserTests

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

Arlolra closed this task as Resolved.Feb 14 2017, 7:26 AM