mpizenberg / elm-test-rs

Fast and portable executable to run your Elm tests
BSD 3-Clause "New" or "Revised" License
81 stars 13 forks source link

Make Deno available as a runtime instead of Node #92

Closed mpizenberg closed 3 years ago

mpizenberg commented 3 years ago

Alright we are almost there but I've encountered one issue that I don't know yet how to fix. I'm unable to capture Debug.log calls happening at the file evaluation. I believe the issue comes from the fact that ES module are evaluated from child to parent. So even though we have defined the following in deno_runner.mjs:

// Capture Debug.log from elm code
// which has been kernel-switched to "console.elmlog"
let logs = [];
console.elmlog = (str) => logs.push(str);

// Compiled by elm-test-rs from templates/Runner.elm
import { Elm } from "./Runner.elm.js";

Since the imported elm module is evaluated first, its top-level calls to Debug.log are not captured.

This also causes a runtime exception since console.elmlog is not defined yet at the evaluation time.

mpizenberg commented 3 years ago

Alright, I think I got it working this time by extracting the console logging override into its own module that I import before the elm module so that it is evaluated first.

// deno_logger.mjs

export const logs = [];
console.elmlog = (str) => logs.push(str);
// deno_runner.mjs

// Capture Debug.log from elm code
// which has been kernel-switched to "console.elmlog"
import { logs } from "./deno_logger.mjs";

// Compiled by elm-test-rs from templates/Runner.elm
import { Elm } from "./Runner.elm.js";
mpizenberg commented 3 years ago

In terms of performances, this supervisor / runner scheme seems to be much slower with Deno than with Node. For example, on elm-color with 8963 tests, and with only one worker (--workers=1) we have:

And yet both are using the same JS engine, V8. So I'd tend to believe that it's the way they implement communication between main and workers that make this slower.

mpizenberg commented 3 years ago

I opened an issue regarding performances on Deno's GitHub: https://github.com/denoland/deno/issues/11561

Viir commented 3 years ago

The post on the Elm forum asking for test runs with this version brought me here.

I am testing on Windows 10 and am curious about the switch to Deno.

I went via the commits list to https://github.com/mpizenberg/elm-test-rs/runs/3206310279 to find the artifact and found this link https://github.com/mpizenberg/elm-test-rs/suites/3375374767/artifacts/79521712

I checked if the version from the UI helps us make sure we are running the version from this branch. Not sure if there is something in the output that allows this distinction. Here is what I got from the 'help' subcommand:

PS C:\Users\John> ."C:\Users\John\Downloads\elm-test-rs_x86_64-pc-windows-msvc\elm-test-rs.exe"  help
elm-test-rs 1.1.0

USAGE:
    elm-test-rs.exe [FLAGS] [OPTIONS] [PATH or GLOB]... [SUBCOMMAND]

FLAGS:
        --deno       Rerun tests with Deno instead of Node
    -h, --help       Prints help information
        --offline    No network call made by elm-test-rs
    -V, --version    Prints version information
    -v               Increase verbosity. Can be used multiple times -vvv
        --watch      Rerun tests on file changes

OPTIONS:
        --compiler <compiler>        Use a custom path to an Elm executable [default: elm]
        --dependencies <strategy>    Choose the newest or oldest compatible dependencies (mostly useful for package
                                     authors) [possible values: newest, oldest]
        --elm-home <path>            Use a custom directory for elm home [env: ELM_HOME=]
        --filter <string>            Keep only tests whose description contains the given string
        --fuzz <N>                   Number of iterations in fuzz tests [default: 100]
        --project <path>             Path to the root directory of the project [default: .]
        --report <report>            Print results to stdout in the given format [default: console]  [possible values:
                                     console, consoleDebug, json, junit, exercism]
        --seed <seed>                Initial random seed for fuzz tests [default: <random>]
        --workers <N>                Number of worker threads [default: <number of logic cores>]

ARGS:
    <PATH or GLOB>...    Path to a test module, or glob pattern such as tests/*.elm

SUBCOMMANDS:
    help       Prints this message or the help of the given subcommand(s)
    init       Initialize tests dependencies and directory
    install    Install packages to "test-dependencies" in your elm.json
    make       Compile tests modules

Now for the actual test. I found three Elm projects in open source repositories to use this on:

When using the --deno option on those projects, I get following error:

Error: Deno supervisor failed to start

Caused by:
    The system cannot find the file specified. (os error 2)
mpizenberg commented 3 years ago

Thanks @Viir for testing this out! I think you got the right version, but maybe not in the easiest way. If you click at the bottom of this PR thread on "Show all checks", then "details" of one of the checks, you'll get to a page where you can click on "Artifacts" and download the one you need.

image

image

image

I've just tested with one of the folders you are linking and they seemed to run smoothly. I think the error means that you might be missing the deno executable. The --deno option of elm-test-rs allows you to switch from running the tests in Node to running them in Deno. It does not however embed the full Deno executable within elm-test-rs, so you still have to install it on your machine.

Is it your issue? (missing deno executable on your machine). In that case you can simply install it with choco or scoop as suggested in deno installation instructions: https://deno.land/#installation

Viir commented 3 years ago

I was surprised it does not bundle the required parts of Deno but depends on a separate installation. Knowing this now, I am following the instructions from the Deno documentation for installation.

Following the links there, I landed at https://github.com/denoland/deno_install and found this command for installing:

iwr https://deno.land/x/install/install.ps1 -useb | iex

The behavior of elm-test-rs --deno changed: I do not anymore see the error mentioned before. Now I got this error:

PS K:\Source\Repos\elm-fullstack\implement\elm-fullstack\ElmFullstack\compile-elm-program> ."C:\Users\John\Downloads\elm-test-rs_x86_64-pc-windows-msvc\elm-test-rs.exe"  --deno
error: Uncaught (in worker "") Unsupported scheme "k" for module "k:\Source\Repos\elm-fullstack\implement\elm-fullstack\ElmFullstack\compile-elm-program\elm-stuff\tests-0.19.1\js\deno_runner.mjs". Supported schemes: [
    "data",
    "blob",
    "file",
    "http",
    "https",
]
error: Uncaught (in promise) Error: Unhandled error event reached main worker.
    at Worker.#pollControl (deno:runtime/js/11_workers.js:247:23)

My guess is it's related to the representation of file paths in Windows.

mpizenberg commented 3 years ago

I was surprised it does not bundle the required parts of Deno but depends on a separate installation

Well it's the same as Node. We do not embed the full Node binary, Node needs to be installed to run elm-test-rs (without --deno). The thing is Node can be a pain to install on some systems if you don't have admin rights, while Deno is much easier to install just for the current user. In theory, if you trust elm-test-rs, running your tests in Deno also makes sure that you can run any elm code without security risk, even if it contains kernel code. This is because Deno is invoked with deno run --allow-read --allow-hrtime but does not have write or network access. So there are still advantages in running tests with Deno instead of Node, even if it requires you to install Deno.

The alternative would be to use deno_core which is on another level of complexity.

mpizenberg commented 3 years ago

I do not anymore see the error mentioned before. Now I got this error: ... My guess is it's related to the representation of file paths in Windows.

Seems like it indeed. Workers are invoked like below:

    let runner = new Worker(new URL(runnerFile, import.meta.url).href, { type: "module" });

I suppose I have to change runnerFile on Windows. I'll let you know when I have something that you can test. Thanks again!

Viir commented 3 years ago

I was surprised it does not bundle the required parts of Deno but depends on a separate installation

Well it's the same as Node. We do not embed the full Node binary, Node needs to be installed to run elm-test-rs (without --deno). The thing is Node can be a pain to install on some systems if you don't have admin rights, while Deno is much easier to install just for the current user. In theory, if you trust elm-test-rs, running your tests in Deno also makes sure that you can run any elm code without security risk, even if it contains kernel code. This is because Deno is invoked with deno run --allow-read --allow-hrtime but does not have write or network access. So there are still advantages in running tests with Deno instead of Node, even if it requires you to install Deno.

The alternative would be to use deno_core which is on another level of complexity.

Thank you for the explanation! I remember installing Node some time ago. As far as I remember, it was much more complicated than what I did to install Deno today. The simpler installation alone is a good reason for me to look into the Deno variant.

mpizenberg commented 3 years ago

@Viir I think the latest commit should fix the issue you had. Let me know if that's the case or not.

Viir commented 3 years ago

@Viir I think the latest commit should fix the issue you had. Let me know if that's the case or not.

Yes, works on my machine with the artifact from https://github.com/mpizenberg/elm-test-rs/commit/3b88e58a5a8d5e4d990e7736d83671dea8aca95d

Viir commented 3 years ago

I just tested with a Node-free Windows 10 system. On that system, trying to run the tests without the --deno option failed with the following error message:

Error: "node --version" failed to start

Caused by:
    The system cannot find the file specified. (os error 2)

Adding the --deno option made it work as expected.

Removing the dependency on Node made it much simpler to set up a development environment. This simplification also makes it easier to integrate into downstream projects like the web-based integrated Elm Editor.