Testing: Fix failure to run non-web translators' tests in debug build. #429
Conversation
- In the testTranslators tool included in the debug build, there is an incorrect script path preventing the type schema data from loading. - In cachedTypes.js, the callback passed to `getSchema()` is not called. (The call site is in `translatorTester_viewer.js`). The overall effect is that non-web translators' test code did not run at all. This is fixed by including the correct script path, and call the callback in `getSchema()`.
src/common/cachedTypes.js
Outdated
| @@ -178,7 +178,7 @@ Zotero.Connector_Types = new function() { | |||
| /** | |||
| * Passes schema to a callback | |||
| */ | |||
| this.getSchema = async function() { | |||
| return TypeSchema; | |||
| this.getSchema = async function(callback) { | |||
There was a problem hiding this comment.
This is supposed to make this work:
However I doubt whether this is the original intention. Why is it named "get"?
There was a problem hiding this comment.
getSchema() was a callback-based function to get the schema, and it was changed to return a promise. So you just have this backwards — it's translatorTester_viewer.js that needs to be updated, not this.
There was a problem hiding this comment.
Thanks a lot for solving the mystery for me! That was what I suspected.
…sted. This partially reverts 734faa3 and restores the call signature of `getSchema()` (with the stale comment about it removed). Instead, fix the caller by moving the callback to a `then()` handler after it.
| @@ -40,7 +40,7 @@ Zotero_TranslatorTester.prototype.runTests = function(callback) { | |||
| } else { | |||
| // other translators get passed right through, after we get schema and preferences | |||
| var me = this; | |||
| Zotero.Connector_Types.getSchema(function(schema) { | |||
| Zotero.Connector_Types.getSchema().then(function(schema) { | |||
There was a problem hiding this comment.
This is the "minimally invasive" change that I can think of. It did make the tests work again, however, I can definitely use some illumination about Zotero.Connector_Types and what it actually does.
There was a problem hiding this comment.
I even tried hooking the access to the "schema" property on the Zotero.Connector_Types object so as to catch any code that tries to get the value of "schema", but I didn't find anything while running the tests. A search into the code seems to confirm that nothing else makes use of Zotero.Connector_Types.schema. So I don't know why this "schema" property is created and passed to the context of the test runs. What am I missing? Could @dstillman @AbeJellinek help? Thanks!
The file node_modules.js was deleted in 69627e7.
|
Thanks for working on this! dstillman -- If this will take a while to fix, could we disable the test runs for non-web translators? This is both confusing for new contributors and a hassle for everyone |
getSchema()is not called. (The call site is intranslatorTester_viewer.js).The overall effect is that non-web translators' test code did not run at all.
This is fixed by including the correct script path, and call the callback in
getSchema().