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.

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

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

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:

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.

// 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:

// 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.

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 added a commit to zoe-translates/translate that referenced this issue Mar 26, 2023
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

1 participant