Add Jest tests for api #87
Conversation
| nextPromise, | ||
| ]).then(([originalMap, newMap]) => | ||
| imo.mergeImportMap(originalMap, newMap) | ||
| return Promise.all([promise, nextPromise]).then( |
There was a problem hiding this comment.
Are these prettier changes because of your installation of VS Code? Since the version of prettier is pinned, I don't know why these changes are happening. Please add a check-format script to the package.json that runs prettier --check . What I want to avoid is a situation where a global installation of prettier that's specific to one contributor's computer is changing the format of the code
There was a problem hiding this comment.
|
Addressed the initial comments. I will continue working on adding tests. |
| @@ -0,0 +1 @@ | |||
| require("jest-fetch-mock").enableMocks(); | |||
There was a problem hiding this comment.
This is needed in order to safely mock fetch (for testing external maps)
| parsedContent = JSON.parse(scriptEl.textContent); | ||
| } catch (e) { | ||
| parsedContent = createEmptyImportMap(); | ||
| } | ||
| nextPromise = Promise.resolve(parsedContent); |
There was a problem hiding this comment.
While testing I realized this function bombs when the import map is empty or is malformed. To prevent a malformed import map from breaking the plugin, I modified this code to instead return an empty map.
robmosca
changed the title
robmosca
changed the title
| (externalOverrideMapPromises[importMapUrl] = | ||
| fetchExternalMap(importMapUrl)); | ||
| return promise.then( | ||
| () => !includes(imo.invalidExternalMaps, importMapUrl) |
There was a problem hiding this comment.
I believe this was a bug. I found it out when writing a test for this function. The map is valid if it IS NOT included in the invalidExternalMaps array.
|
Closed in favour of #96 |
While working on the import-map-overrides project, I realized the code is not covered by unit tests. I would like to contribute Jest tests, to make sure that further modifications to the package do not introduce regressions.
Current coverage: