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.

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

Travis failure after test for "array of nested inline table" was added to toml_test #30

Closed
kaushalmodi opened this issue Dec 14, 2018 · 8 comments

Comments

@kaushalmodi
Copy link
Member

kaushalmodi commented Dec 14, 2018

toml_test was updated with commit toml-lang/toml-test@fb8ca06 on Nov 27.

The Travis tests for parsetoml run every week, and the next test day after Nov 27 happened to be Nov 20 when it failed: https://travis-ci.org/NimParsers/parsetoml/builds/461974298.

Test: nested-inline-table-array (valid)
../src/parsetoml.nim(883) parseValue
Error: unhandled exception: (1:14) unexpected character "{" [TomlError]
126 passed, 1 failed

/cc @PMunch

@PMunch
Copy link
Member

PMunch commented Dec 16, 2018

The run before says "126 passed, 0 failed" while this one says "126 passed, 1 failed" so it appears a test was added for something we don't support. I'll look into it but the logic for inline arrays is pretty hairy and might need a complete rewrite to support this

@kaushalmodi
Copy link
Member Author

@PMunch I meant exactly that, that the toml-test has been updated with new tests supporting a new TOML syntax.

I'll look into it but the logic for inline arrays is pretty hairy and might need a complete rewrite to support this

Thank you!

I am watching out for when toml-lang/toml-test#51 and toml-lang/toml-test#53 get merged in toml-test .. Those also seem to add tests for a lot of new syntax. But our CI uses the latest toml-test. So we will be auto-notified if any update in toml-test causes tests with parsetoml to fail.

@kaushalmodi
Copy link
Member Author

Reminder: parsetoml to be reenabled in Nim ci once all tests pass; ref: nim-lang/Nim@af6539b

@PMunch
Copy link
Member

PMunch commented May 28, 2019

Isn't this fixed?

@kaushalmodi
Copy link
Member Author

kaushalmodi commented May 28, 2019

I need to check why parsetoml was removed from Nim CI yesterday. I was lazy to just put a reminder to check that here.

@kaushalmodi
Copy link
Member Author

@PMunch I see the same error today too.

Running nimble run_toml_test gives:

github.com/BurntSushi/toml-test
Test: nested-inline-table-array (valid)

/home/kmodi/sandbox/nim/parsetoml/src/parsetoml.nim(890) parseValue
Error: unhandled exception: (1:14) unexpected character "{" [TomlError]


127 passed, 1 failed

@PMunch
Copy link
Member

PMunch commented Jun 10, 2019

Fixed by 988f6f5

@PMunch PMunch closed this as completed Jun 10, 2019
@kaushalmodi
Copy link
Member Author

@PMunch: wow! you are on a roll! Time to cut a new release of parsetoml.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants