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

New test suite has problem with null #362

Closed
svgeesus opened this issue Nov 24, 2023 · 3 comments
Closed

New test suite has problem with null #362

svgeesus opened this issue Nov 24, 2023 · 3 comments

Comments

@svgeesus
Copy link
Member

In the new color conversions tests:

white | 100, 0, null | 100, 0.0146,

notice the missing third parameter in the expected result.

The actual js lists the third parameter:

tests: [
	{
		args: "slategray",
		expect: [52.697472, 11.242899, 253.010088]
	},
	{
		args: "white",
		expect: [100, 0.0146, null]
	},
	{
		args: "black",
		expect: [0, 0, null]
	}
]

I suspect that this work-around might be the culprit:

check: check.deep(function (actual, expect) {
	if (expect === null || Number.isNaN(expect)) {
		// Treat NaN and null as equivalent for now
		return actual === null || Number.isNaN(actual);
	}

weirdly the third sub-test still passes.

Note to self: (unrelated to this bug) the white test should be corrected, we now calculate a chroma of 0 rather than the slightly-off chroma of 0.0146 due to #354 which also means we can improve the rather slack epsilon in this test)

@svgeesus
Copy link
Member Author

Seems to be restricted to the html output, running the same test in node gives passes.

├── sRGB to LCH ✅ 3/3 PASS (1.4 ms)

@svgeesus
Copy link
Member Author

Note to self: (unrelated to this bug) the white test should be corrected

Test failure now corrected with better expected values; unexpected removal of null in HTML output remains, however.

@svgeesus
Copy link
Member Author

Fixed with 2428ea2 - use NaN in the actual expected results, it formats as null

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

No branches or pull requests

2 participants