Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Translator tester: running URL-specified translator(s) does not do what it is supposed to do (not awaiting translators to fully load) #20

Open
zoe-translates opened this issue Mar 26, 2023 · 0 comments · May be fixed by #21
Open

Translator tester: running URL-specified translator(s) does not do what it is supposed to do (not awaiting translators to fully load) #20

zoe-translates opened this issue Mar 26, 2023 · 0 comments · May be fixed by #21

Comments

@zoe-translates
Copy link

Context

The translator tester (accessible in the debug build of the connector extension) is supposed to run the tests for the translators specified on the URL.

This is used by the Selenium-driven test for pull requests in zotero/translators, but this test (most of the time) does not actually run:

https://github.com/zotero/translators/blob/3a9544d7b0b6fdcb6cdcbc8c08392f91d20d99b4/.ci/pull-request-check/selenium-test.js#L128-L130

The problem

I think the problem is that, when runURLSpecifiedTranslators() is called at the end of init(), most likely the translators have not been loaded into the global object translatorTestViewsToRun which is used by the function to identify the translator(s) to run:

translate/testTranslators/testTranslators.js

Lines 580 to 590 in fcb7b8a

async function runURLSpecifiedTranslators() {
const href = document.location.href;
let hashParams = href.split('#')[1];
if (!hashParams) return;
let translatorIDs = new Set(hashParams.split('translators=')[1].split(',').map(decodeURI));
let translatorTestViews = [];
for (let type in translatorTestViewsToRun) {
for (const translatorTestView of translatorTestViewsToRun[type]) {
if (translatorIDs.has(translatorTestView._translatorTester.translator.translatorID)) {
translatorTestViews.push(translatorTestView);

But the sequence of calls is like the following:

init() calls haveTranslators() which returns a promise that is not awaited/then-handled. (The following snippet is the call site, and itself is inside one big initialization loop waited for by await Promise.all(...) on Line 428.

translate/testTranslators/testTranslators.js

Lines 474 to 478 in fcb7b8a

// get translators, with code for unsupported translators
if(!viewerMode) {
let translators = await Zotero.Translators.getAllForType(translatorType, true);
haveTranslators(translators, translatorType);
}

Then the init() function runs its course, and makes the call to runURLSpecifiedTranslators() at the very end:

translate/testTranslators/testTranslators.js

Lines 532 to 536 in fcb7b8a

// Run translators specified in the hash params if any
runURLSpecifiedTranslators();
}
}

And by this time when runURLSpecifiedTranslators() accesses translatorTestViewsToRun, the haveTranslators() function likely has not been called (enough times) yet, so translatorTestViewsToRun is empty or incomplete.

Impact

Because of this, the following lines in runURLSpecifiedTranslators() are executed without the tests being actually run.

translate/testTranslators/testTranslators.js

Lines 599 to 601 in fcb7b8a

var elem = document.createElement('p');
elem.setAttribute('id', 'translator-tests-complete');
document.body.appendChild(elem);

That is to say, the page falsely reports that the tests passed by the URL are complete. This can even happen if the URL-passed translator ID is invalid.
zoe-translates added a commit to zoe-translates/translate that referenced this issue Mar 26, 2023
@zoe-translates
Testing: Only test URL-specified translators after page is ready.
c4ca691 
Fixes zotero#20.
zoe-translates added a commit to zoe-translates/translate that referenced this issue Mar 26, 2023
@zoe-translates
Translator tester: Only test URL-specified translators when ready.
e10c3e0 
Wait for `translatorTestViewsToRun` to be populated, and then call the
handler for the URL-specified translators' tests. If the
`translatorTestViewsToRun` object is not ready, the handler
(`runURLSpecifiedTranslators`) will silently skip the tests it is
supposed to run.

Fixes zotero#20.
This was referenced Mar 26, 2023
Open
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Translator tester: Only test URL-specified translators when ready. zoe-translates/translate
1 participant
@zoe-translates