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

Make toml-test the standard test suite for checking compliance of implementations #585

Closed
cktan opened this issue Jan 12, 2019 · 43 comments
Closed

Comments

@cktan
Copy link

cktan commented Jan 12, 2019

No description provided.

@pradyunsg
Copy link
Member

Not yet.

@cktan
Copy link
Author

cktan commented Jan 13, 2019

Is there a doc on the diff between v0.4 and v0.5?

@ehuss
Copy link

ehuss commented Jan 13, 2019

Is there a doc on the diff between v0.4 and v0.5?

There is a changelog which lists what has changed.

@iarna
Copy link

iarna commented Jan 18, 2019

If your goal is to test an existing parser I suggest looking at:

BurntSushi's test suite: https://github.com/BurntSushi/toml-test

And mine: https://github.com/iarna/toml-spec-tests

(Which is in a compatible file format but independently created and differs mostly by having more complete support for TOML 0.5)

I also maintain a supported-feature chart for the various TOML parsers on npm that's produced by running them through these two test suites: TOML-SPEC-SUPPORT

@cktan
Copy link
Author

cktan commented Jan 18, 2019 via email

@pradyunsg pradyunsg changed the title Is there a v0.5 certification test? Standard Test Suite for checking compliance of implementations May 25, 2019
@pradyunsg
Copy link
Member

@iarna 👋🏻

Would you have any pointers on what the current state of affairs is on this front, with 0.5 compliance testing?


I'm trying to figure out what "blessed" compliance tests for TOML 1.0 should look like - they'll live in https://github.com/toml-lang/compliance. I expect/hope we can reuse @BurntSushi and @iarna's work for this.

I am agnostic to the language/tech we use for this, as long as we document how to get the compliance tests running for parser implementers and it's not "too difficult". :)

@iarna
Copy link

iarna commented Jun 15, 2019

Heya. Mine should be complete for 0.5 these days. As an implementer, most of what I want is paired files: "here's some toml" "here's what it should represent". Most of my tests are toml/yaml pairs, where the yaml should produce the same document. There are a couple of mine that follow @BurntSushi's lead and use json, with named data types, and stringified representations, when testing for types that don't exist in yaml.

I chose yaml for two resons: First, it is how the existing tests folder in the toml repo is setup. Second, it was easier to compose while filling out the test suite.

That said, having everything in JSON format as BurntSushi did would be more consistent (only one file type) and save implementers from having to find a yaml implementation. It'd be pretty easy to mechanically convert the yaml files to the JSON format.

@LongTengDao
Copy link
Contributor

LongTengDao commented Aug 8, 2019

What does Standard Test Suite refer to?

  1. A program
  2. Some TOML doc to parse (without below)
  3. Some TOML doc to parse (with expect parsed data in JSON/YAML format)?

@mmakaay
Copy link

mmakaay commented Aug 19, 2019

I am using both @BurntSushi and @iarna tests in my parser implementation. One thing I like about @iarna is that there are some qa-* tests defined, which really pushed me into improving the processing speed of my parser. I am not sure if they would belong in a blessed test set, provided directly from the TOML repo, but it would be nice to have them available as a secondary performance test set.

@iarna
Copy link

iarna commented Aug 19, 2019

Anyone who wants to add to the test suite, or add more qa-type tests, I'm happy to add as a contributor. (Or even move it to a shared org.)

@pradyunsg
Copy link
Member

Making this a critical path item for TOML 1.0, since it makes a lot of sense to do a compliance test suite, in time for TOML 1.0.

@pradyunsg
Copy link
Member

Since @cannikin is handling the website, I'm going be working on this. 🙃

@workingjubilee
Copy link
Contributor

workingjubilee commented Aug 28, 2019

Stating what probably everyone already knows:

The https://github.com/BurntSushi/toml-test DSL built on top of JSON looks good as a baseline, since it relies on returning a JSON object with a name, stringified value, and type annotation, and anything that can do JSON can do that, and since it is abstract (it doesn't really bind itself to fundamental JSON qualities, aside from being able to represent a hashmap, and uh, strings), it doesn't seem to have any obvious drawbacks aside from "also you have to parse JSON", which is a pretty low bar to clear. @iarna's logic for YAML : TOML comparisons being easy is compelling but also hits the wall mentioned.

The harness itself using Go is secondary, and he does link to a few other projects which replicate the harness.

I'm a little uncertain how one would go about extracting the test suite from https://github.com/iarna/iarna-toml as a freestanding test suite that could be called by e.g. a Bash script, but it also is quite comprehensive, and probably is the most up to date and thorough set of tests, barring the recent edits. I am guessing you would probably prefer to wrangle a test suite in Python, however, on the code side, and I mean, it's kind of hard to go wrong with "most popularly used language", if we have to offer a piece of software to use, as long as it has no dependencies. Whatever it is, the format probably should include code-less data files and instructions on how to (re)build the harness.

@iarna
Copy link

iarna commented Aug 28, 2019

I'm up for converting my own test suite to the same format as the BurntSushi one, at which point there's no integration issues, we can use the go implementation or any other. (This can be done mechnically: They're currently just ymal files representing the object we expect, they can be easily converted to the json format.)

Edited to add:
There aren't a lot of tests in https://github.com/iarna/iarna-toml that I'd expect anyone else to care about? They're just some extra converage tests, and a very early pass at some more QA style blackbox tests. Those I intend to port over to https://github.com/iarna/toml-spec-tests, which is my complete suite of 0.5 tests, in an already stand alone language agnostic format.

@workingjubilee
Copy link
Contributor

workingjubilee commented Aug 29, 2019

Ah that would explain it, I somehow missed an entire repo! Good job, me.

@ChristianSi
Copy link
Contributor

ChristianSi commented Jan 7, 2020

As far as I can see, this seems to be the only still open issue that blocks the release of TOML 1.0? Hope it won't block too long... (The world has waited long enough, me thinks.)

@sgarciac
Copy link
Contributor

sgarciac commented Jan 9, 2020

Besides the tests themselves, there needs to be an specification of the toml -> json encoding rules. If I'm not wrong, @iarna 's tests follow BurntSushi's guidelines, which would need to be updated since they are missing local datetime, date and time types. (I addressed this in this P.R. toml-lang/toml-test#51 , using the same conventions as Iarna's, if I remember well.)

@goodmami
Copy link
Contributor

One more reason to get an official location for test cases, however imperfect it may be at first, is for those that document updates to the spec.

Case in point: I created #728 to clarify something in the spec and wanted to include a test case with the associated PR. I'd like to add it to https://github.com/toml-lang/compliance, but that repo is completely empty. I also don't see a test for this case in either https://github.com/BurntSushi/toml-test/ or https://github.com/iarna/toml-spec-tests/, but neither has been updated in over a year (but maybe that'll change soon with the v1.0.0-rc.1 being out), so I worry that my test case would be forgotten if I submitted a PR to those. There's also the tests/ directory of this repo, but there's just a handful of files there, so it doesn't look used.

I guess the best I can do for now is add the test to this issue thread? If so, here it is:

When the last non-whitespace character in a multi-line string is a backslash it ignores any following whitespace, but not if the backslash is escaped. A naive parser might just check if the last non-whitespace character is \, or maybe also if the last two are \\, but these checks are inadequate. For example, the following should ignore whitespace following the \ after foo and baz \\, but not after bar \.

ml-escaped-nl = """
   foo \ 
   bar \\ 
   baz \\\ 
   quux"""

@iarna
Copy link

iarna commented Apr 17, 2020

Your PR wouldn't be forgotten, but I too would prefer that compliance tests consolidate in one (official) place.

@iarna
Copy link

iarna commented Apr 22, 2020

I've added a branch to my toml-spec-tests repo for 1.0.0-rc.1:

https://github.com/iarna/toml-spec-tests/tree/1.0.0-rc.1

This is updated from the diff to the English docs between 0.5.0 and 1.0.0-rc.1. Likely it could use a second pass with an eye toward bringing in details from the ABNF.

@filmor
Copy link

filmor commented Jul 4, 2020

Now that's just splitting hairs. The date format is strictly ISO8601, the canonical format for this kind of data. All I'm asking for is that floats get a similarly well-defined format (like exponential notation with 6 digits or something like that) so that I don't have to special case them.

@iarna
Copy link

iarna commented Jul 5, 2020

I'm really not splitting hairs: ISO8601 doesn't have a singular string representation for a given date. There are many valid representations of the same date. For ex 2020-12-12T00:00Z is the same as 2020-12-12T00:00:00.00-00:00. If you parse the field, as I've suggested, this is not a problem. If you're trying to stringify your dates and hoping the string representation is identical though...

@arp242
Copy link
Contributor

arp242 commented Jun 14, 2021

I updated https://github.com/BurntSushi/toml-test with a bunch of additional tests; I think it should support most of 1.0 now, but I need to go over some details and there's probably a few things I missed. I also added a bunch of unit tests in https://github.com/BurntSushi/toml that I need to port over, and there are a few further improvements to the test runner tool that can be made.

Either way, after some period of inactivity it's maintained now (by me). Just FYI.

@arp242
Copy link
Contributor

arp242 commented Jun 25, 2021

What's the path forward on this?

There are now essentially two "competing" solutions: BurntSushi/toml-test and toml/compliance

When I started working on this a few weeks ago toml/compliance seemed like an empty placeholder repo as it didn't contain any actual tests, but since then someone opened a PR to actually add some tests, and looking at run.py a bit closer it does actually implement a test runner.

Practically speaking:

  • toml-test/compliance seems less mature and polished; it took me some time to get it running (it assumes the working directory, and throws exception on startup: I had to add a if json_file is None: return as a quick fix), and the errors aren't all that helpful/useful IMHO; compliance just says Got a non-zero exit code: 1 or Should have rejected input – that's not very helpful as such. Errors on various other conditions (like wrong directory) aren't super helpful either right now.

    Solvable problems for sure, but also already solved in toml-test.

  • There are more tests: 126 TOML files in the PR that was opened vs. 279 in toml-test now.

  • toml-test is written in Go and toml/compliance in Python. I have nothing against Python and generally don't really care what language is used, but Go has the practical upshot that it can be distributed as a single static binary which doesn't need any external dependencies at all (the actual tests are compiled in the binary, although they can also be read from the filesystem). This makes it a lot easier to run on both people's personal computers and CI systems.

Personally, I think it makes sense to replace toml/compliance with BurntSushi/toml-test. BurntSushi already said he's fine with that, and I'm fine with it too. I mean, people could spend a lot of effort on fixing all of this in toml/compliance, but it seems a bit pointless if there's already a good solution? Especially since this is a fairly niche tool where you don't really need 2 or 3 different solutions that people can choose from according to preferences/use case.

@pradyunsg
Copy link
Member

Personally, I think it makes sense to replace toml/compliance with BurntSushi/toml-test.

And I agree, especially given that toml-test is actively maintained now. :)

The only concern I have is the mechanics of making this happened -- I don't have the admin bit on the toml organisation (only @mojombo does, I believe) and @BurntSushi will likely need to do things in the GitHub admin UI as well, to make this change happen. Let's see if the relevant folks respond here, otherwise I'll go around chasing them over email/etc. :)

@BurntSushi
Copy link
Member

@pradyunsg What do you need from me? Happy to go along with whatever y'all decide.

@pradyunsg
Copy link
Member

With toml-lang/compliance#11, I've pointed toml/compliance to BurntSushi/toml-test.

@BurntSushi @mojombo I think we should transfer the toml-test repository to the toml org. I don't have the relevant permissions on GitHub (on either the repo nor this org) to do the relevant things.

@pradyunsg pradyunsg changed the title Standard Test Suite for checking compliance of implementations Make toml-test the standard test suite for checking compliance of implementations Oct 3, 2022
@pradyunsg
Copy link
Member

Filed an issue over at toml-test, for the maintainers to chime in. We'll discuss the logistics there, and I'll close this issue out once the transfer is done and the spec is updated to reflect the changes to be made.

@arp242
Copy link
Contributor

arp242 commented Aug 29, 2023

It's now moved to https://github.com/toml-lang/toml-test

I got the same kind of collaborator/commit access as before, so no further action is needed for that.

We will probably want to delete https://github.com/toml-lang/compliance? Could also archive it, but not too much value in keeping it, I think?

@arp242
Copy link
Contributor

arp242 commented Sep 23, 2023

@pradyunsg I think we can close this? Or do we need any further action?

@pradyunsg
Copy link
Member

We're done here. Thanks for the patience and the continued maintenance of toml-test!

@pradyunsg pradyunsg unpinned this issue Apr 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests