Use Test.RunnerV2

[lydell]

Use Test.RunnerV2 from elm-explorations/test#260, and implement everything that unlocks.

More info coming soon.

[jfmengels]

Great work I'm not sure I reviewed everything, it was a lot of changes, but here are a few suggestions I have.

I think at this point it would be better for me to review by trying it out. If you have some instructions on how to use the 2 branches together, I would love to see them.

[jfmengels]

Maaaybe worth using a record to make function calls easier to read?

[lydell]

The call sites look like this:

Ports.sendResult testId False jsDefinitionName labels expectationElmCode debugLogs report

Ports.sendResult testId True jsDefinitionName labels expectationElmCode debugLogs report

So a record would mostly be 🦜 (but give a name to the boolean).

I wonder what’s faster, passing a big record or many parameters. My guess would be many parameters (up to a point).

[jfmengels]

Alright that's fair.

I wonder what’s faster, passing a big record or many parameters.

Haven't checked it, but I'm pretty sure many parameters is faster. (will try to add a benchmark for that in my benchmark repo 👀 )

[jfmengels]

You could also do

let
  ( expectation, duration ) =
    runWithDuration (\() -> Runner.runUnitTest unitTest)
  -- ...
in
sendUnitTestResult testId unitTest cachedExpectation duration (getAndClearDebugLogs False) model.testReporter

or

let
  ( expectation, duration ) =
    runWithDuration (\() -> Runner.runUnitTest unitTest)
in
let
  debugLogs =
    getAndClearDebugLogs False
  -- ...
in
-- ...

In both cases, this is a clearer sign that getAndClearDebugLogs will be called after runWithDuration. The order of computation of a tuple seems too incidental to rely on comfortably I'd say. (Also, one less tuple allocation)

I think a comment indicating that getAndClearDebugLogs has to be computed afterwards because it relies on side-effects from runWithDuration would be welcome still.

[jfmengels]

I feel like the conversion a custom type is unnecessary, especially as it gets reconverted to something else right away in sendUnitTestResult

[lydell]

It is this way because sendUnitTestResult is also called in a different place – where we read test expectations from cache instead of running tests.

[jfmengels]

What is the output of this? Would it start with CachedUnitTestFail ...? If so, that feels a bit odd.

[lydell]

Yes, for example:

CachedUnitTestFail { description = "Expect.equal", reason = Equality "\" blah\"" "\"blah\"" }

It can also be CachedUnitTestPass if there are debug logs.

Why does it feel a bit odd?

[lydell]

Maybe I should mention that it is used for codegen:ing the cache file.

[jfmengels]

Use function(paused) instead of => to keep compiling to ES5?

[jfmengels]

Seems like there's a number of places where we ignre the returned logs. Should there be a variant of getAndClearDebugLogs that simply clears Debug.logs?

[jfmengels]

Do you know if these chains are reduced to their smallest possible cycles? (Similar to the Elm import cycle sometimes including unrelated modules)

[lydell]

I don’t. All I know is that I naively “solved” recursive function chains myself, and kept finding cases where it broke. Then I read up on it and learned that it is a well-known computer science graph problem, and that Tarjan’s algorithm can solve it. Then I found an implementation of it on npm with lots of dependents that looked good. Since I started using it I haven’t run into issues.

[lydell]

If you have some instructions on how to use the 2 branches together, I would love to see them.

In elm-explorations/test, run node --run install-local && rm -f ~/.elm/0.19.2/packages/registry.dat to copy the local version of the package into ~/.elm/.

In node-test-runner:

1. cd example-application
2. rm -rf elm-stuff
3. ../bin/elm-test

To test it on some other project’s test suite:

1. Make sure that the project uses Elm 0.19.2.
2. Make sure that the project uses "elm-explorations/test": "2.2.1".
3. rm -rf elm-stuff
4. ~/path/to/node-test-runner/bin/elm-test