mpizenberg / elm-test-rs

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

v0.4.1 - Windows 10 - Crash while parsing string literal in Elm Syntax - 'byte index 1 is not a char boundary' #42

Closed Viir closed 3 years ago

Viir commented 3 years ago

I found the current version fails when running in this directory:

https://github.com/elm-fullstack/elm-fullstack/tree/250de2e830e28b9263f0262b2dee4ed1cd651920/implement/elm-fullstack/ElmInteractive/interpret-elm-program

Here is the output I got, tested on Windows 10:

elm-test-rs 0.4.1
-----------------

thread 'main' panicked at 'byte index 1 is not a char boundary; it is inside '✔' (bytes 0..3) of `✔️"  """
                    , expectedValueElmExpression = "\"just a literal ✔️\""
                    }
        , Test.test "Just a literal List String" <|
            \_ ->
                expectationForElmInteractiveScenario
                   `[...]', src\parser.rs:263:17
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

The Elm test code appearing in the error message comes from here: https://github.com/elm-fullstack/elm-fullstack/blob/250de2e830e28b9263f0262b2dee4ed1cd651920/implement/elm-fullstack/ElmInteractive/interpret-elm-program/tests/ElmInteractiveTests.elm#L12-L19

I guess it has problems with some characters in string literals in the Elm syntax.

harrysarson commented 3 years ago

Unicode str indexing issue! The most annoying kind

Viir commented 3 years ago

Unicode str indexing issue

Maybe that would make a better title? 🤔 I am not familiar with the language in this area.

mpizenberg commented 3 years ago

Thanks for this! the issue is indeed indexing of string slices in parsing of block comments and multiline strings. I've added failing unit tests for these in #43 . It should not be too difficult to fix I think. I'll have a look at it as soon as I can (probably not before next week).

mpizenberg commented 3 years ago

I got a little time to fix it. @Viir would you mind checking the CI build artifacts of the PR (top right corner of this page: https://github.com/mpizenberg/elm-test-rs/pull/43/checks?check_run_id=1630064400)

Viir commented 3 years ago

Thank you for the fast implementation! I took the artifact named x86_64-pc-windows-msvc_elm-test-rs to test. This one works. It does not crash anymore for the linked project. Here is the output I got:

elm-test-rs 0.4.1
-----------------

Total time spent in the elm compiler: 2.1169314s

Running 34 tests. To reproduce these results later, run:
elm-test-rs --seed 2679562804 --fuzz 100 (TODO: pass files to reporter)

TEST RUN PASSED

Duration: 38138 ms
Passed:   34
Failed:   0
Running duration (since Node.js start): 5822 ms

For comparison, this is the output from elm-test:

elm-test 0.19.1-revision4
-------------------------

Running 34 tests. To reproduce these results, run: elm-test --fuzz 100 --seed 179260980726499

TEST RUN PASSED

Duration: 6388 ms
Passed:   34
Failed:   0

With these observations, I consider this issue solved.

mpizenberg commented 3 years ago

Awesome!