WikiTextEditor appears trivial, but interacts with the super-complex EditPage. Some kind of integration test should make sure this interaction can't break, probably with a browser test
The WikiTextEditor handles the editing of the file info, building the text boxes
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Resolved | thiemowmde | T190290 Reach a stable test coverage for the FileImporter PHP code | |||
| Resolved | awight | T190829 Test WikiTextEditor class with browser tests. |
Oooh--my current guess is now simply that we can't connect to commonswiki from the test.
Thoughts: we can upload a file to the local test wiki, implement a test-only parameter "ignoreDuplicateHash", and import from self.
I'm able to connect to commonswiki from an integration worker, so this theory might be wrong.
@zeljkofilipin @hashar @Krinkle I would love some help either enabling screenshots for the test failure in https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/FileImporter/+/509768/ , or generally your thoughts about why this might be happening. AIUI, the possible causes of failure I can think of all seem unlikely:
[chrome #0-9] ChangeFileInfo page
[chrome #0-9]
[chrome #0-9] ChangeFileInfo page
[chrome #0-9] 1) "before all" hook
[chrome #0-9]
[chrome #0-9]
[chrome #0-9] 1 failing (4s)
[chrome #0-9]
[chrome #0-9] 1) ChangeFileInfo page "before all" hook:
[chrome #0-9] An element could not be located on the page using the given search parameters (".mw-importfile-edit-button button").
[chrome #0-9] Error: An element could not be located on the page using the given search parameters (".mw-importfile-edit-button button").
[chrome #0-9] at Promise.F (node_modules/core-js/library/modules/_export.js:36:28)
[chrome #0-9] at click() - at Context.before (extensions/FileImporter/tests/selenium/specs/wikitext_editor.js:10:42)
[chrome #0-9]In short: do not do anything in a before hook which could eventually fail. The video capturing is not set up yet.
Because of T223431, we might have to introduce some nasty code changes and a test-only parameter to allow us to navigate to Special:ImportFile pages even when $wgEnableUploads is false. Of course, the final check must be respected to prevent actually uploading on submit.
This patch plus https://gerrit.wikimedia.org/r/#/c/integration/quibble/+/510709/ should do the job.
Change 511402 had a related patch set uploaded (by Awight; owner: Awight):
[mediawiki/core@master] Exclude FileImporter browser tests
Change 511402 merged by jenkins-bot:
[mediawiki/core@master] Exclude FileImporter browser tests
The dependencies could be merged as soon as next week, so I'm moving this task to remind myself to stay engaged. See https://gerrit.wikimedia.org/r/#/c/integration/quibble/+/510709/
@hashar Thanks for all your effort to make the per-repo tests actually work!
So, now we're at a point where we can experiment with our repo-specific browser tests. The biggest challenge remaining is how to safely set LocalSettings.php configuration which will only apply to our repo. I think it would be slightly irresponsible to merge what I've done so far, since we know it will have side effects during gate-and-submit. Ideally, there is a config snippet which is loaded for our tests but unloaded before testing other repos. My only thought is that we need an isolated devserver.
We need browser tests for a stable deployment. Let's put this on the roadmap again, now that the preparatory work has been done.
If it doens't break any other extensions, I think a quick-and-dirty cat >> LocalSettings.php is on the table, so we don't get too blocked on configuration.
Change 540118 had a related patch set uploaded (by Awight; owner: Awight):
[integration/quibble@master] Allow uploads to support FileImporter tests
This will take some tweaks to https://gerrit.wikimedia.org/r/#/c/integration/quibble/+/540118/ , and then in theory should work, but realistically will need another round of debugging CI.
If I'm not mistaken, two patches are still open here. They depend on each other, so it's conceptually only one patch.
@hashar left some super helpful comments. However, to me it does not sound like we have an obvious way forward. @awight, what do you suggest?
Thanks for nudging us towards a more verbose update :-) That's exactly right, the main patch is in good shape but cannot be merged until CI passes. The current blocker to CI is that $wgEnableUploads defaults to false, so I've tried to address that in https://gerrit.wikimedia.org/r/#/c/integration/quibble/+/540118/ . I'll work on this today and will move back to the Review column after that's ready. I can run Quibble locally to verify my patch.
Change 540118 merged by jenkins-bot:
[integration/quibble@master] Allow uploads to support FileImporter tests
Watching until quibble 0.0.36 is deployed, then we can try to run and debug the FileImporter tests.
Unblocked! Browser tests are passing...
This is ready for final review: https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/FileImporter/+/509768/
Change 509768 merged by jenkins-bot:
[mediawiki/extensions/FileImporter@master] Browser tests for external dependencies
Change 545850 had a related patch set uploaded (by Awight; owner: Awight):
[mediawiki/core@master] FileImporter browser tests require uploads enabled
Change 545850 merged by jenkins-bot:
[mediawiki/core@master] FileImporter browser tests require uploads enabled