Page MenuHomePhabricator

mediawiki/core webdriver.io tests fail in EntitySchema extension
Closed, ResolvedPublic

Description

https://integration.wikimedia.org/ci/job/quibble-vendor-mysql-php72-docker/47112/console and https://integration.wikimedia.org/ci/job/quibble-vendor-mysql-php72-docker/47115/console both failed with similar browser test errors:

14:03:02 [0-1] Error in "Page should be creatable"
14:03:02 Timeout of 60000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/workspace/src/tests/selenium/specs/page.js)
14:03:03 [0-1] 
14:03:03 	Video location: /workspace/log/Page-should-be-creatable.mp4 
14:03:03 
14:03:03 	ffmpeg exited with code 255 /workspace/log/Page-should-be-creatable.mp4
14:04:02 [0-1] Error in ""after each" hook for "should be creatable""
14:04:02 Timeout of 60000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves.
14:12:02 [0-1] 2020-02-19T13:12:02.885Z ERROR webdriver: Request failed due to Error: invalid session id
14:12:02   (Driver info: chromedriver=73.0.3683.75,platform=Linux 4.9.0-0.bpo.11-amd64 x86_64)
...
14:12:02 [0-1] 2020-02-19T13:12:02.888Z ERROR webdriver: Request failed due to Error: invalid session id
14:12:02   (Driver info: chromedriver=73.0.3683.75,platform=Linux 4.9.0-0.bpo.11-amd64 x86_64)
...
14:12:02 [0-1] 2020-02-19T13:12:02.889Z ERROR @wdio/sync: Error: invalid session id
14:12:02   (Driver info: chromedriver=73.0.3683.75,platform=Linux 4.9.0-0.bpo.11-amd64 x86_64)
14:12:02     at endReadableNT (_stream_readable.js:1103:12)
14:12:02     at Object.afterTest (/workspace/src/tests/selenium/wdio.conf.js:169:11)
14:12:02 [0-1] FAILED in chrome - /tests/selenium/specs/page.js
14:13:06 [0-2] Error in "Rollback without confirmation "before all" hook for "should perform rollback via POST request without asking the user to confirm""
14:13:06 Timeout of 60000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/workspace/src/tests/selenium/specs/rollback.js)
14:17:07 WARNING:backend.ChromeWebDriver:[1582118227.009][SEVERE]: Timed out receiving message from renderer: 300.000
14:17:07 WARNING:backend.ChromeWebDriver:[1582118227.012][SEVERE]: Timed out receiving message from renderer: -0.003
14:17:07 [0-2] 2020-02-19T13:17:07.089Z ERROR webdriver: Request failed due to Error: invalid session id
14:17:07   (Driver info: chromedriver=73.0.3683.75,platform=Linux 4.9.0-0.bpo.11-amd64 x86_64)
...
14:17:07 [0-2] FAILED in chrome - /tests/selenium/specs/rollback.js
14:18:13 [0-4] Error in "Special:Watchlist should show page with new edit"
14:18:13 Timeout of 60000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/workspace/src/tests/selenium/specs/specialwatchlist.js)
14:18:14 [0-4] 
14:18:14 	Video location: /workspace/log/Special%3AWatchlist-should-show-page-with-new-edit.mp4 
14:18:14 
14:18:14 [0-4] 	ffmpeg exited with code 255 /workspace/log/Special%3AWatchlist-should-show-page-with-new-edit.mp4
14:19:13 [0-4] Error in ""after each" hook for "should show page with new edit""
14:19:13 Timeout of 60000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves.
14:22:14 WARNING:backend.ChromeWebDriver:[1582118534.301][SEVERE]: Timed out receiving message from renderer: 300.000
14:22:14 WARNING:backend.ChromeWebDriver:[1582118534.307][SEVERE]: Timed out receiving message from renderer: -0.006
14:27:14 WARNING:backend.ChromeWebDriver:[1582118834.317][SEVERE]: Timed out receiving message from renderer: 300.000
14:27:14 WARNING:backend.ChromeWebDriver:[1582118834.323][SEVERE]: Timed out receiving message from renderer: -0.006
14:27:14 [0-4] 2020-02-19T13:27:14.399Z ERROR webdriver: Request failed due to Error: invalid session id
14:27:14   (Driver info: chromedriver=73.0.3683.75,platform=Linux 4.9.0-0.bpo.11-amd64 x86_64)
...
14:27:14 [0-4] 2020-02-19T13:27:14.404Z ERROR webdriver: Request failed due to Error: invalid session id
14:27:14   (Driver info: chromedriver=73.0.3683.75,platform=Linux 4.9.0-0.bpo.11-amd64 x86_64)
...
14:27:14 [0-4] 2020-02-19T13:27:14.405Z ERROR @wdio/sync: Error: invalid session id
14:27:14   (Driver info: chromedriver=73.0.3683.75,platform=Linux 4.9.0-0.bpo.11-amd64 x86_64)
14:27:14     at endReadableNT (_stream_readable.js:1103:12)
14:27:14     at Object.afterTest (/workspace/src/tests/selenium/wdio.conf.js:169:11)
14:27:14 [0-4] FAILED in chrome - /tests/selenium/specs/specialwatchlist.js
14:27:19  "dot" Reporter:
14:27:19 ..FF.....FF...
14:27:19 
14:27:19 Spec Files:	 3 passed, 3 failed, 6 total (100% completed) in 00:25:23

Both builds are for this change, which only adjusts a comment, so I would be very surprised if that was the reason.

Event Timeline

Restricted Application added a project: Wikidata. · View Herald TranscriptFeb 19 2020, 2:30 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Doesn’t seem to affect other extensions – the other recent build failures in quibble-vendor-mysql-php72-docker look unrelated, and there are plenty of successful builds too.

Addshore added a subscriber: Addshore.

I also see "PHP Warning: Use of undefined constant NS_ENTITYSCHEMA_JSON - assumed 'NS_ENTITYSCHEMA_JSON' (this will throw an Error in a future version of PHP)
#0 /workspace/src/extensions/EntitySchema/src/MediaWiki/EntitySchemaHooks.php(235): MWExceptionHandler::handleError(integer, string, string, integer, array)"

Strange, that constant is supposed to be defined by MediaWiki via extension.json.

Is there some MediaWiki Core tag / project we can add to this task? Because I don’t think the bug is on our end, to me it looks like a bug in extension registration.

Even if it's true that this started failing when a specific change in core was made. I still believe it is the extension that must be fixed. I might get this wrong, but my idea is: Code should not assume that the "namespaces" section from extension.json was executed, and the PHP constants been made available.

Even if: These PHP constants are not in PHP code. IDEs can't find them. It's incredibly helpful to have them in PHP code, even if this might be redundant with the introduction of "namespaces" in extension.json. All codebases (well, except this one) still have some redundant define() in their startup code, most of the time embedded in the "ExtensionMessagesFiles" that is responsible for specifying the $namespaceNames and aliases.

Code should not assume that the "namespaces" section from extension.json was executed, and the PHP constants been made available.

Then what is the point of that section anyways? What use is it if it defines constants, but only under some obscure circumstances which don’t appear to be documented anywhere?

All codebases (well, except this one) still have some redundant define() in their startup code, most of the time embedded in the "ExtensionMessagesFiles" that is responsible for specifying the $namespaceNames and aliases.

My assumption is that this is only because the extensions still want to support earlier MediaWiki versions, or because nobody has bothered to remove those old define()s yet from back when they were still required.

I would like to ask another question: How is it a good idea do define a PHP constant in a .json file? How is my IDE supposed to find it there?

In other words: As far as I'm concerned the duplication is a must.

I would like to ask another question: How is it a good idea do define a PHP constant in a .json file? How is my IDE supposed to find it there?

I don’t know, ask whoever designed the extension registration system ^^

As it stands, Manual:Using custom namespaces § In extensions doesn’t mention a requirement or even recommendation to redundantly define() the namespace, and neither do Manual:Extension registration or Manual:Extension.json/Schema § namespaces; and the “examples” extension registers namespaces in extension.json and nowhere else. So my assumption remains that defining a namespaces outside of extension.json is not supposed to be required, and the change that broke EntitySchema CI must therefore be a bug.

And since I think that bug could break third-party extensions as well, I’m taking the liberty of adding this task to MW-1.35-release – maybe that will get some more eyes on it. (Anyone, feel free to remove it if you disagree.)

I dug into the code and indeed found the place that creates constants based on what's said in extension.json: https://phabricator.wikimedia.org/source/mediawiki/browse/master/includes/registration/ExtensionRegistry.php$512. But this appears to be more like a fallback, not a replacement. Again, how does it make sense to kill constants from PHP code when they are still used in PHP code? All code using these constants would appear broken to all tools and IDEs. I'm still like 99% sure the "constant" piece in extension.json is meant to reference a PHP constant, not necessarily replace it.

(EntitySchema is also WMF-deployed.)

There are no (obvious) team tags associated with this task. Who is waiting for what/whom?

For my part, I’m waiting for “someone from MediaWiki core” (Core Platform Team? not sure tbh) to clarify how the constant field of a namespace in extension.json is supposed to work, and whether this bug is due to EntitySchema having relied on something that wasn’t supposed to be a feature, or whether MediaWiki core broke something that causes the constant to no longer be defined in some cases.

Nice, ok lets see re-check these tests once that gets a +2 ?

Well, the point of this task, as far as I understand, is that such changes to extensions shouldn’t be necessary…

When someone is able and willing to identify the original issue, we can undo code changes that are not needed any more then. As long as this is not the case let us please, please unblock people from working on these codebases.

Peter.ovchyn added a subscriber: nnikkhoui.

@nnikkhoui Both Art and me are blocked by this issue, so I think we could jump on it to fix it properly.

@Peter.ovchyn ok :) I spent half of yesterday trying to re-produce locally so i could test some fixes and was unable to, even on the correct branches everything was passing. The only thing I did notice was that was another extension is reporting the same error (https://integration.wikimedia.org/ci/job/quibble-vendor-mysql-php72-docker/50811/console)

Change 580076 had a related patch set uploaded (by Nikki Nikkhoui; owner: Nikki Nikkhoui):
[mediawiki/core@master] Experimental fix for T245629. Do not merge.

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

Change 580076 abandoned by Nikki Nikkhoui:
Experimental fix for T245629. Do not merge.

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

Change 580076 restored by Nikki Nikkhoui:
Experimental fix for T245629. Do not merge.

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

Change 580155 had a related patch set uploaded (by Art-Baltai; owner: BAGArt):
[mediawiki/core@master] Bug with CACHE: undefined constant

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

Change 580076 had a related patch set uploaded (by Nikki Nikkhoui; owner: Nikki Nikkhoui):
[mediawiki/core@master] Fix for T245629.

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

Art-Baltai added a comment.EditedMar 18 2020, 1:19 AM

Case with a problem:

  • determine constants from a valid cache or actual data
  • try to load from cache non-cached attributes
  • making data
  • define of constants is skipped: is already defined. $data['defines'] = array();
  • override cached data with an empty constant list

next run:

  • load data from cache with empty constants but the valid response

PS: other cases with problems are possible:

  • Depends on $this context with mutable methods
  • other data may be skipped/incorrect

Change 580076 merged by jenkins-bot:
[mediawiki/core@master] ExtensionRegistry: Avoid losing 'defines' when loading lazy-loaded attributes

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

Change 580155 abandoned by Ppchelko:
Fix error: undefined constant

Reason:
I4f151f88ece56cf718749b9de11fc8e204ccf29d

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

Lucas_Werkmeister_WMDE closed this task as Resolved.Mar 19 2020, 5:26 PM

Seems to be resolved, thanks a lot!

@AMooney, I understand that this ticket is resolved. Should it be in the Done column instead of the Waiting for Review one?