-
Notifications
You must be signed in to change notification settings - Fork 71
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
Inconsistent handling of array of tables #52
Comments
awvwgk
pushed a commit
to toml-f/toml-test
that referenced
this issue
Sep 20, 2020
(cherry picked from commit 2f649e0)
arp242
added a commit
that referenced
this issue
Jun 14, 2021
This is a rather large PR that got out of hand 😅 But it fixes and improves quite a few things. There is a lot of parsing going on left and right by various parts: reading the files, the external test tools, and parsing of what they output. Previously it was *really* hard to grok what was parsing what and what exactly failed. It took me ages to grok why these tests were failing for the TOML library; turns out this was a mixture of actual bugs in the TOML library, bugs in the toml-test-* tools, bugs in this test tool, and tests people had added that were just broken. This vastly redoes the test output to look quite nice and understandable (see below), it's also a lot more verbose, but that's a feature IMO. Instead of just "expected type string, got int" actually show what we have. There is some room for improvement here as it shows this with fmt.Printf("%#v"), which gives rather Go-specific output. Maybe show as JSON? Have to see later. Some other changes as well: - All JSON files are formatted consistently according to what json.MarshalIndent(.., "", " ") outputs. Being predictable means it's much easier to compare the desired and actual outputs since you don't have to mentally parse the differences in formatting so much. - Fix a bunch of broken tests, and arrays can now be represented as just "[ ... ]"; no object with type/value needed (or even supported). It was added in case "TOML might support tuples", but we'll cross that bridge if we ever get at it. Fixes #52 Fixes #58 Fixes #67 - Tests are now consistently referenced with the full path relative to the test directory: "valid/name" and "invalid/name". e.g. to run one test use "toml-test [..] valid/name" This means it's clearer what kind of test we're dealing with, means we don't need globally unique tests, and that we can add "js-valid" tests specifically for a language if need be. All filesystem access now also done trough the fs.FS interface; the big upshot of this is that we can produce binaries with the tests embedded in them and that you don't need to pass -testdir (this still works, for development etc.) The downside is that it requires Go 1.16, but I think that's okay for a tool like this. We can just provide binaries once a new release is tagged. Fixes #20 Fixes #23 ---- Example output (test name and some other parts are in bold unless you add -no-bold): Test: invalid/key-multiline (toml-test-decoder < ../toml-test/tests/invalid/key-multiline.toml) Expected an error, but no error was reported. input sent to toml-test-decoder: """long key""" = 1 output from toml-test-decoder (stdout): { "long\nkey": { "type": "integer", "value": "1" } } want: <empty> Test: valid/inline-table-nest (toml-test-encoder < ../toml-test/tests/valid/inline-table-nest.json) Error encoding TOML: toml: TOML array element cannot contain a table input sent to toml-test-encoder: { "arr_arr_tbl_empty": [ [ {} ] ], [.. trim ..] output from toml-test-encoder (stderr): Error encoding TOML: toml: TOML array element cannot contain a table want: <empty>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
None yet
0 participants
I'm testing a TOML encoder on testcase valid/array-table-array-string-backslash.
The encode under test outputs
while the testsuite expects
Clearly these two TOML encodings are equivalent.
However, the toml-test tool reports failure:
I believe this happens because the golden TOML parser uses different types for inline arrays versus arrays of tables. An inline array is parsed into a value of type
[]interface{}
. But a double-bracketed array of tables is parsed into a value of type[]map[string]interface{}
. These are incompatible types according to the Go typesystem. As a result, the toml-test tool reports failure even though the answer is correct.The text was updated successfully, but these errors were encountered: