Page MenuHomePhabricator

Extension:Wikisource: ResourcesTest::testResourceFiles: HTTP request blocked: https://ocr.wmcloud.org/api/available_langs?engine=google
Closed, ResolvedPublic

Description

05:19:58 There was 1 failure:
05:19:58 
05:19:58 1) ResourcesTest::testResourceFiles
05:19:58 HTTP request blocked: https://ocr.wmcloud.org/api/available_langs?engine=google by MediaWiki\Http\HttpRequestFactory::get. Use MockHttpTrait.
05:19:58 
05:19:58 /workspace/src/tests/phpunit/mocks/NullHttpRequestFactory.php:46
05:19:58 /workspace/src/includes/http/HttpRequestFactory.php:181
05:19:58 /workspace/src/includes/http/HttpRequestFactory.php:204
05:19:58 /workspace/src/extensions/Wikisource/includes/HookHandler/EditPageShowEditFormInitialHandler.php:108
05:19:58 /workspace/src/includes/libs/objectcache/wancache/WANObjectCache.php:1726
05:19:58 /workspace/src/includes/libs/objectcache/wancache/WANObjectCache.php:1556
05:19:58 /workspace/src/extensions/Wikisource/includes/HookHandler/EditPageShowEditFormInitialHandler.php:123
05:19:58 /workspace/src/extensions/Wikisource/includes/HookHandler/EditPageShowEditFormInitialHandler.php:79
05:19:58 /workspace/src/includes/ResourceLoader/FileModule.php:1343
05:19:58 /workspace/src/vendor/wikimedia/testing-access-wrapper/src/TestingAccessWrapper.php:114
05:19:58 /workspace/src/tests/phpunit/structure/ResourcesTest.php:240
05:19:58 phpvfscomposer:///workspace/src/vendor/phpunit/phpunit/phpunit:106

Event Timeline

@Samwilson Could we load the config directly from the OCR tool ?

Samwilson added a subscriber: KLawal-WMF.

Yes, I think so, that sounds a fair bit simpler. The extension is already requesting the actual OCR data, so some config data should be easy to do.

@KLawal-WMF what do you think?

Yes, I think so, that sounds a fair bit simpler. The extension is already requesting the actual OCR data, so some config data should be easy to do.

@KLawal-WMF what do you think?

Sure, we can load the actual data. I am currently refactoring the langs.json, to make it easier to work with when it's fetched the config in one take

@taavi has let me know that I caused this with my change https://gerrit.wikimedia.org/r/c/mediawiki/core/+/1003101. Sorry about that, I didn't expect this check to have false positives.

If you need to unbreak the tests, the most expedient way would be to add if ( !defined( 'MW_PHPUNIT_TEST' ) ) { … } around the HTTP request here: https://gerrit.wikimedia.org/g/mediawiki/extensions/Wikisource/+/e1a7eafd0d5c7cf5728cc8ba6694bbd10d6a097d/includes/HookHandler/EditPageShowEditFormInitialHandler.php#108

But I have to say that making HTTP requests just to compute the module definition is a bad vibe. I guess you've shown that it works fine in practice, but still :) The result is needed to output the startup module, so if it were to fail in some way you haven't predicted, this would break all JavaScript-based functionality on all Wikisources, not just the OCR feature. It would be much more reliable to include a copy of the data you need in a .json file somewhere in the repository.

If you need to unbreak the tests

Thanks for the pointer! I was wondering about that. Sounds like a simple short-term fix.

Probably the best move here is to load the data on the frontend when the extension is loaded (which can then gracefully fail if the tool is not available, which is okay because if it can't provide the config data then it's likely that it also can't provide the actual OCR service).

It would be much more reliable to include a copy of the data you need in a .json file somewhere in the repository.

The reasoning for loading it dynamically was that the data changes semi-regularly (roughly monthly) and also needs to be maintained for the ocr.wmcloud.org tool, so having it stored in only one place seemed best.

Change #1023132 had a related patch set uploaded (by Samwilson; author: Samwilson):

[mediawiki/extensions/Wikisource@master] Skip fetching of the engine lang config when testing

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

Change #1023132 merged by jenkins-bot:

[mediawiki/extensions/Wikisource@master] Skip fetching of the engine lang config when testing

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