Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

sync: crashes on TOML files but lint shows no problems #613

Closed
verdammelt opened this issue Jun 7, 2022 · 1 comment · Fixed by #614
Closed

sync: crashes on TOML files but lint shows no problems #613

verdammelt opened this issue Jun 7, 2022 · 1 comment · Fixed by #614
Assignees
@ee7
Labels
cmd: sync kind: bug User-facing incorrect behavior

Comments

@verdammelt
Copy link
Member

Encountered this using configlet version 4.0.0-beta.4
I run the command configlet sync --tests --docs --metadata --filepaths and it crashes with the following sort of error:

Checking exercises...
parsetoml.nim(908)       parseValue
Error: unhandled exception: /Users/mark/SRC/exercism/common-lisp/exercises/practice/affine-cipher/.meta/tests.toml(6:16) unexpected character "e" [TomlError]

This is because my TOML file has lines like: description = blah blah rather than description = "blah blah".

But configlet lint reports no problems.

  1. Perhaps sync should not crash in this case?
  2. Perhaps lint could signal that the TOML file is invalid?
@ee7 ee7 added kind: bug User-facing incorrect behavior cmd: sync labels Jun 7, 2022
@verdammelt verdammelt mentioned this issue Jun 7, 2022
Closed
@ee7
Copy link
Member

ee7 commented Jun 7, 2022 •
edited
Loading

To others: see an example problematic tests.toml file.

The problem is indeed that the file contains an unquoted string, which is invalid TOML (see the TOML spec).

However, I agree that configlet sync could produce a better error message, and configlet lint could report that the toml file is invalid (tracked in #289).

@ee7 ee7 self-assigned this Jun 7, 2022
@ee7 ee7 mentioned this issue Jun 7, 2022
Merged
@ee7 ee7 closed this as completed in #614 Jun 17, 2022
ee7 added a commit that referenced this issue Jun 17, 2022
@ee7
sync(tests): improve error for invalid TOML (#614)
3f4193f 
A manually edited `tests.toml` file might contain invalid TOML. For
example:

    [2ee1d9af-1c43-416c-b41b-cefd7d4d2b2a]
    description = encode yes

where `encode yes` is invalid [1] because it is unquoted.

Before this commit, `configlet sync` would not handle the TOML parsing
exception:

    $ cd /tmp
    $ git clone --quiet https://github.com/exercism/common-lisp
    $ cd common-lisp
    $ git checkout f521b1fb0f04ffc2d5baa6cf0bba37c231cc1bd7
    $ bin/fetch-configlet
    $ bin/configlet sync --tests
    Updating cached 'problem-specifications' data...
    Checking exercises...
    parsetoml.nim(908)       parseValue
    Error: unhandled exception: /tmp/common-lisp/exercises/practice/affine-cipher/.meta/tests.toml(6:16) unexpected character "e" [TomlError]

With this commit, configlet still exits immediately, but handles the
exception:

    $ configlet sync --tests
    Updating cached 'problem-specifications' data...
    Checking exercises...

    Error: A 'tests.toml' file contains invalid TOML:
    /tmp/common-lisp/exercises/practice/affine-cipher/.meta/tests.toml(6:16) unexpected character "e"

    The expected 'tests.toml' format is documented in
    https://exercism.org/docs/building/configlet/sync#h-tests

Note that `configlet lint` does not yet lint `tests.toml` files, and so
invalid TOML does not yet cause `configlet lint` to indicate an error.

[1] https://toml.io/en/v1.0.0#string

Fixes: #613
@ee7 ee7 moved this to Done in Configlet roadmap Jul 1, 2022
@ee7 ee7 changed the title Sync crashing on TOML files but lint shows no problems. sync: crashes on TOML files but lint shows no problems Jul 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ee7 ee7

Labels
cmd: sync kind: bug User-facing incorrect behavior
Projects
Configlet roadmap
Status: Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

sync(tests): improve error for invalid TOML ee7/exercism-configlet
2 participants
@verdammelt @ee7