search

jasmine / jasmine-browser-runner

Serve and run your Jasmine specs in a browser
49 stars 23 forks source link

Import Map support #30

Closed wraiford closed 1 year ago

wraiford commented 1 year ago

NOTE: This is still a rough draft and is not ready to be merged atow (April 18, 2023)

Goal

This PR is to add functionality to enable import maps when testing ES Module libraries in the browser context. The consumer should be able to add config in jasmine-browser.json that results in an importmap <script> tag in the html template of the browser harness, such that ESM imports resolve correctly and bare imports errors are avoided.

This references issue #28

Broad overview of changes

links

sgravrock commented 1 year ago

Thanks for this. Free time has been scarce this week but I should be able to dig into this and respond to your earlier questions this weekend.

wraiford commented 1 year ago

I should be able to give this a more thorough review and tire kicking in a day or three, but I wanted to mention some low hanging fruit that I noticed in my initial pass.

In addition to my inline comments: CI builds are failing on Windows and on Node 12.

The tests are failing on Node 12 because the ?? operator doesn't exist yet, and on Windows because the import map in the output HTML contains backslashes instead of slashes. You should be able to fix Windows by avoiding path.sep and by replacing backslashes with slashes in the result of path.join.

Ah, I see! The HTML should always contain forward slashes eesh 🙄 I can try to run it on my Windows machine but that will add a bit of a delay for getting the environment up there.

As for Node 12, it's up to you whether you want to support it. I'm planning to release 2.0, which will drop everything below Node 18 and probably require jasmine-core 5.x, in a month or two. So you've got two choices:

* Convert to Node 12 compatible syntax. If this PR merges before 2.0, I'll release it in 1.4.0.

* Change the PR's destination branch to 2.0. It'll be released in 2.x.

I'm fine with either approach.

If the only problem is the null coalescing I have no problem changing those over for backwards compatibility. Spiritually (or whatever) import maps do seem more like a more modern approach though. Personally, I am on v19 (need to switch to v20) in order to get better isomorphic WebCrypto API support and globalThis.

Regardless of the backwards compatibility though, if you're releasing a new major version then I probably should test it against that branch also anyway.

wraiford commented 1 year ago

The tests are failing on Node 12 because the ?? operator doesn't exist yet...

I've changed to a logical OR with this commit.

wraiford commented 1 year ago

Ah well, I tried a blind stab at the windows fix. I'll have to get it setup on my windows machine, which I've only just gotten going recently. I don't even have vscode on it yet, and I don't have any windows VMs going atm either (I do very little in VMs these days...getting old!).

sgravrock commented 1 year ago

Ah well, I tried a blind stab at the windows fix. I'll have to get it setup on my windows machine, which I've only just gotten going recently. I don't even have vscode on it yet, and I don't have any windows VMs going atm either (I do very little in VMs these days...getting old!

Don’t worry about it. I thought the blind stab would work, but since it didn’t, I can take care of Windows. I already have a test machine handy.

wraiford commented 1 year ago

Don’t worry about it. I thought the blind stab would work, but since it didn’t, I can take care of Windows. I already have a test machine handy.

Well I didn't worry about it but I did go ahead and get a VM up and going! (I already had a partially initialized Windows VM after all) For me personally, it was throwing an error about not finding ChromeDriver:

Failed: Child suite failed with error: Error: Command failed: node ../../../bin/jasmine-browser-runner runSpecs Error: The ChromeDriver could not be found on the current PATH. Please download the latest version of the ChromeDriver from http://chromedriver.storage.googleapis.com/index.html and ensure it can be found on your PATH. at new ServiceBuilder (C:\Users\billm\ibgib\forks\jasmine-browser-runner\node_modules\selenium-webdriver\chrome.js:175:13) at thenableWebDriverProxy.getDefaultService (C:\Users\billm\ibgib\forks\jasmine-browser-runner\node_modules\selenium-webdriver\chrome.js:264:12) at thenableWebDriverProxy.createSession (C:\Users\billm\ibgib\forks\jasmine-browser-runner\node_modules\selenium-webdriver\chromium.js:669:49) at thenableWebDriverProxy.createSession (C:\Users\billm\ibgib\forks\jasmine-browser-runner\node_modules\selenium-webdriver\chrome.js:255:13) at createDriver (C:\Users\billm\ibgib\forks\jasmine-browser-runner\node_modules\selenium-webdriver\index.js:147:33) at Builder.build (C:\Users\billm\ibgib\forks\jasmine-browser-runner\node_modules\selenium-webdriver\index.js:675:16) at buildWebdriver (C:\Users\billm\ibgib\forks\jasmine-browser-runner\lib\webdriver.js:31:10) at Object.runSpecs (C:\Users\billm\ibgib\forks\jasmine-browser-runner\index.js:101:25) at process.processTicksAndRejections (node:internal/process/task_queues:95:5) at async Command.runSpecs (C:\Users\billm\ibgib\forks\jasmine-browser-runner\lib\command.js:187:5)

stdout:
Jasmine server is running here: http://localhost:50624
Jasmine tests are here:         C:\Users\billm\ibgib\forks\jasmine-browser-runner\spec\fixtures\esmIntegration\spec
Source files are here:          C:\Users\billm\ibgib\forks\jasmine-browser-runner\spec\fixtures\esmIntegration\src
 The ChromeDriver could not be found on the current PATH, trying Selenium Manager
Unable to obtain driver using Selenium Manager: Error: Error executing command with C:\Users\billm\ibgib\forks\jasmine-browser-runner\node_modules\selenium-webdriver\bin\windows\selenium-manager.exe,--browser,chrome

stderr:
Error: The ChromeDriver could not be found on the current PATH.
      Please download the latest version of the ChromeDriver
      from http://chromedriver.storage.googleapis.com/index.html
      and ensure it can be found on your PATH.
    at new ServiceBuilder (C:\Users\billm\ibgib\forks\jasmine-browser-runner\node_modules\selenium-webdriver\chrome.js:175:13)
    at thenableWebDriverProxy.getDefaultService (C:\Users\billm\ibgib\forks\jasmine-browser-runner\node_modules\selenium-webdriver\chrome.js:264:12)
    at thenableWebDriverProxy.createSession (C:\Users\billm\ibgib\forks\jasmine-browser-runner\node_modules\selenium-webdriver\chromium.js:669:49)
    at thenableWebDriverProxy.createSession (C:\Users\billm\ibgib\forks\jasmine-browser-runner\node_modules\selenium-webdriver\chrome.js:255:13)
    at createDriver (C:\Users\billm\ibgib\forks\jasmine-browser-runner\node_modules\selenium-webdriver\index.js:147:33)
    at Builder.build (C:\Users\billm\ibgib\forks\jasmine-browser-runner\node_modules\selenium-webdriver\index.js:675:16)
    at buildWebdriver (C:\Users\billm\ibgib\forks\jasmine-browser-runner\lib\webdriver.js:31:10)
    at Object.runSpecs (C:\Users\billm\ibgib\forks\jasmine-browser-runner\index.js:101:25)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async Command.runSpecs (C:\Users\billm\ibgib\forks\jasmine-browser-runner\lib\command.js:187:5)

I tried installing chromedriver globally and still no dice, even though my nodejs folder (which was indeed on the PATH) contained files chromedriver and chromedriver.cmd. I downloaded the latest chromedriver win32 zip here and extracted the chromedriver.exe to the nodejs folder, updated my chrome browser proper (chromedriver version has to match), and the specs passed except the pending sauce labs spec.

Though there were quite a few logs that I don't see in mac in the form of:

[0503/130519.036:INFO:CONSOLE(3)] "spec 4", source: http://localhost:51412/__spec__/aSpec.js (3) [0503/130519.039:INFO:CONSOLE(159)] "Spec '4' has no expectations.", source: http://localhost:51412/__jasmine__/jasmine-html.js (159)

notes

summary

So for me it seems the specs themselves are passing, but not on the build server. It looks like it's a Windows Server image, whereas I'm on Windows Home. I'm not sure how that might make either the html output be materially different or if the node version's regular expression engine somehow differ. 🤷

I can dumb down the regular expression checking and just check for individual lines, or if you have any suggestions on writing a better regular expression, I'm all ears (I'm not a pro at them but usually I can get them working).

EDIT: I also did a sanity check on the specs that were failing on the server build using a fail('testing'); line and indeed it did fail. Probably not important but hey it's a thing.

wraiford commented 1 year ago

I thought I saw a difference in the length of texts. Now with them as RegExp objects, the output error shows them as regular expressions whose source's are the same length. However, it still failed with regular expressions.

However!

In the very same file, there are multiple comments about problems with line endings of \r?\n, which was in the back of my mind...that and the fact that line endings are a pita often. Now it looks to be passing, having removed the \n which was redundant anyway since that matches against the \s* part. 😅

wraiford commented 1 year ago

So I await your kicking the hell out of those tires.

sgravrock commented 1 year ago

This is merged in aabd966, but GitHub doesn't show it as merged because I squashed the commits. Thanks for all your hard work on this.

wraiford commented 1 year ago

Hopefully it will benefit many users of a great library! Thanks for working with me 👍 😃