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

Any plans to comply with toml v1.0.0? #40

Open
sealmove opened this issue Oct 8, 2020 · 8 comments
Open

Any plans to comply with toml v1.0.0? #40

sealmove opened this issue Oct 8, 2020 · 8 comments

Comments

@sealmove
Copy link

sealmove commented Oct 8, 2020

Hey, toml has reached version 1. Anyone up for adjusting this project to be consistent with it?
@PMunch
Copy link
Member

PMunch commented Oct 8, 2020 •
edited
Loading

Thanks for the notification, we should definitely update this to be compatible with v1.0.0.

EDIT: I can see that it's still a release candidate, but we should start working towards it anyways. Historically the test suite for TOML is always lagging behind though, so it's not the easiest task in the world.

@EchoPouet
Copy link

Version 1.0.0 has been released since the beginning of the year.

@PMunch
Copy link
Member

PMunch commented May 3, 2021 •
edited
Loading

Any test-suite yet that we can work against?

EDIT: For reference this is the one I've used to verify the parser against thus far: https://github.com/BurntSushi/toml-test

@kaushalmodi
Copy link
Member

@PMunch Would this help: https://github.com/status-im/nim-toml-serialization/pull/36/files

@kaushalmodi
Copy link
Member

@PMunch This seems to be the latest test suite: https://github.com/iarna/toml-spec-tests

@PMunch
Copy link
Member

PMunch commented Jun 7, 2021

Hmm, with that one we need to implement a YAML output writer as well

@kaushalmodi
Copy link
Member

kaushalmodi commented Jul 1, 2021 •
edited
Loading

@PMunch

I noticed that the Burntsushi toml-test project has now a new maintainer and it's updated to include TOML v1.0.0 tests.

To install:

  1. Install go 1.16 or newer
  2. go get -u -v github.com/BurntSushi/toml-test/cmd/toml-test@master
  3. toml-test decoder/decoder

With that, I am now getting 49 fails.

toml-test [decoder/decoder]: using embeded tests: 231 passed, 49 failed

kaushalmodi added a commit to kaushalmodi-forks/parsetoml that referenced this issue Jul 1, 2021
@kaushalmodi
GHA: Add skips for failing TOML v1.0.0 tests
ea08401 
Ref: NimParsers#40
kaushalmodi added a commit that referenced this issue Jul 1, 2021
@kaushalmodi
GHA: Enable Go toml-test based test suite (#50)
83de3f3 
* GHA: Set GOPATH

* GHA: Print go version

* GHA: Attempt to fix GOPATH

https://laszlo.cloud/setting-gopath-in-github-actions

* GHA: Attempt 2 to fix GOPATH

https://github.com/golang/go/wiki/SettingGOPATH#go-113

* GHA: Try getting rid of GOPATH

* GHA: Install toml-test

* GHA: Require go version 1.16.5

This was the latest stable go version when this was committed.

* GHA: Fix .yml

* GHA: Specify the version of toml-test to be installed

* GHA: Separate 'uses' and 'run'

Looks like GHA doesn't work if both are in the same yml node.

* GHA: Looks like we do need to set GOPATH

* GHA: Install an older version of toml-test that installs binary

* Update to use tests for TOML 1.0.0

* GHA: Add skips for failing TOML v1.0.0 tests

Ref: #40

* GHA: Disable testing on macos

It fails with these errors:

    go: writing stat cache: mkdir /home/runner: operation not supported
    go: writing stat cache: mkdir /home/runner: operation not supported
    go: downloading github.com/BurntSushi/toml-test v0.1.1-0.20210628215546-8c0aa6d9c74f
    go get github.com/BurntSushi/toml-test/cmd/toml-test@master: mkdir /home/runner: operation not supported
    stack trace: (most recent call last)
    /private/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/nimblecache-946033505/nimscriptapi_610282258.nim(187, 16)
    /Users/runner/work/parsetoml/parsetoml/parsetoml.nimble(36, 7) run_toml_test_with_skipsTask
    /Users/runner/work/parsetoml/parsetoml/nim/lib/system/nimscript.nim(273, 7) exec
    /Users/runner/work/parsetoml/parsetoml/nim/lib/system/nimscript.nim(273, 7) Error: unhandled exception: FAILED: go get -u -v github.com/BurntSushi/toml-test/cmd/toml-test@master [OSError]

* Add few comments about incompatibility with TOML v1.0.0

* GHA: Disable testing on Windows

* GHA: Get rid of GOPATH dependency

Assumption is that $GOPATH/bin is auto-added to $PATH.

* Skip a test for now; it's failing on Windows

Ref:
- #51
- https://github.com/NimParsers/parsetoml/pull/50/checks?check_run_id=2966348113
@kaushalmodi
Copy link
Member

Ref: 83de3f3

Now this passes nimble run_toml_test_with_skips. In this task:

This issue can be closed once nimble run_toml_test passes 100% on Linux.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@PMunch @kaushalmodi @EchoPouet @sealmove