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

Consistent floating point representation between tests #67

Closed
yyny opened this issue Feb 27, 2021 · 2 comments · Fixed by #69
Closed

Consistent floating point representation between tests #67

yyny opened this issue Feb 27, 2021 · 2 comments · Fixed by #69

Comments

@yyny
Copy link

yyny commented Feb 27, 2021

For example, tests/valid/exponent-part-float.json represents floats without a trailing .0, but tests/valid/float-exponent.json represents floats with a trailing .0.

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>
@filmor
Copy link
Contributor

filmor commented Jun 15, 2021

@arp242 Could it be that this one got closed accidentally? The formatting is still not completely consistent. Additionally, as I already mentioned in toml-lang/toml#585 (comment), it would be good if the exact representation of the floats in JSON (e.g. number of digits) could be specified.

@arp242
Copy link
Collaborator

arp242 commented Jun 15, 2021

All natural numbers are without a .0, so I figured it was fixed, but I didn't really understand why it mattered either. Anyway, we can talk about that in the PR.

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

Successfully merging a pull request may close this issue.

3 participants