Skip to content

Improve toml-test integration#11

Merged
cyyynthia merged 1 commit intosquirrelchat:mistressfrom
arp242:t
Oct 18, 2023
Merged

Improve toml-test integration#11
cyyynthia merged 1 commit intosquirrelchat:mistressfrom
arp242:t

Conversation

@arp242
Copy link
Copy Markdown
Contributor

@arp242 arp242 commented Oct 18, 2023

Make the scripts executable and add hashbang; this way it can be run a bit easier:

% toml-test -int-as-float ./toml-test-parse.js | tail -n3
toml-test v2023-10-18 [./toml-test-parse.js]: using embedded tests
  valid tests: 163 passed,  3 failed
invalid tests: 342 passed, 13 failed

% toml-test ./toml-test-encode.js -encoder | tail -n2
toml-test v2023-10-18 [./toml-test-encode.js]: using embedded tests
encoder tests: 157 passed,  9 failed

Also add a run-toml-test.bash script to the root, with the correct flags to run the tests and skipping known failures; this way it's easy to test for regressions.

I just added the -int-as-float flag to toml-test, so that you don't get loads of "failed" tests on this. In my reading of the specification ("64-bit signed integers (from −2^63 to 2^63−1) should be accepted") it's not actually required to support the full int64 range. I'll send a PR to clarify the spec on this later.

Make the scripts executable and add hashbang; this way it can be run a
bit easier:

	% toml-test -int-as-float ./toml-test-parse.js | tail -n3
	toml-test v2023-10-18 [./toml-test-parse.js]: using embedded tests
	  valid tests: 163 passed,  3 failed
	invalid tests: 342 passed, 13 failed

	% toml-test ./toml-test-encode.js -encoder | tail -n2
	toml-test v2023-10-18 [./toml-test-encode.js]: using embedded tests
	encoder tests: 157 passed,  9 failed

Also add a run-toml-test.bash script to the root, with the correct flags
to run the tests and skipping known failures; this way it's easy to test
for regressions.

I just added the `-int-as-float` flag to toml-test, so that you don't
get loads of "failed" tests on this. In my reading of the specification
("64-bit signed integers (from −2^63 to 2^63−1) should be accepted")
it's not actually *required* to support the full int64 range. I'll send
a PR to clarify the spec on this later.
@cyyynthia cyyynthia added the enhancement New feature or request label Oct 18, 2023
Copy link
Copy Markdown
Member

@cyyynthia cyyynthia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the contribution!

Yeah, the integer limit is not marked as required by the spec and uses the term "should"; the only enforced requirement is to properly reject integers if they cannot be represented which I do.

I also found a few tests for the encoder fail due to expectations of sub-millisecond precision in the output, and I had to modify my un-tag method to handle +Inf with an uppercase I - IMHO it should be lowercase everywhere for consistency with the other tests and the way it is written in TOML

@cyyynthia cyyynthia merged commit 5ccd1d1 into squirrelchat:mistress Oct 18, 2023
@arp242 arp242 deleted the t branch October 18, 2023 17:55
@arp242
Copy link
Copy Markdown
Contributor Author

arp242 commented Oct 18, 2023

The Inf thing was fixed: toml-lang/toml-test#143 – I didn't really look at the content of the scripts, but you should be able to remove any exceptions you did for that.

I need to look at the other failures; I just added smol-toml to https://arp242.github.io/toml-test-matrix; one of the reason I made that is so I can have some insight what failures happen with different implementations, and fix/improve toml-test as needed. I'll get round to it ... eventually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants