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

[chore] Port missing Constructor tests #394

Open
Tracked by #399
lloydk opened this issue Feb 3, 2024 · 4 comments
Open
Tracked by #399

[chore] Port missing Constructor tests #394

lloydk opened this issue Feb 3, 2024 · 4 comments
Labels
help wanted Extra attention is needed Topic: testing

Comments

@lloydk
Copy link
Collaborator

lloydk commented Feb 3, 2024

There are 5 tests in the old Constructor test suite and 3 in the new suite.

@lloydk lloydk changed the title Port missing Constructor tests to the new format Port missing Constructor tests Feb 3, 2024
@LeaVerou LeaVerou changed the title Port missing Constructor tests [chore] Port missing Constructor tests Feb 4, 2024
@LeaVerou LeaVerou added the help wanted Extra attention is needed label Feb 4, 2024
@lloydk
Copy link
Collaborator Author

lloydk commented Feb 4, 2024

These tests have been ported. I was just looking at the count of tests in the browser and didn't realize there was an issue with htest and the deepest nested tests.

Uncaught TypeError: Cannot read properties of undefined (reading 'cells')
    at TestResult.<anonymous> (render.js:70:7)
    at TestResult.dispatchEvent (BubblingEventTarget.js:21:16)
    at TestResult.dispatchEvent (BubblingEventTarget.js:17:12)
    at TestResult.evaluate (TestResult.js:148:8)
    at TestResult.run (TestResult.js:84:8)
    at TestResult.js:126:10

Tests that are causing the exception

name: "new Color(spaceId, coords)",
tests: [
{
args: ["P3", [0, 1, 0]],
expect: { "spaceId": "p3", "coords": [ 0, 1, 0 ], "alpha": 1 }
},
{
args: ["rec2100pq", [0.34, 0.34, 0.34]],
expect: { "spaceId": "rec2100pq", "coords": [ 0.34, 0.34, 0.34 ], "alpha": 1 }
}
]
},

@LeaVerou
Copy link
Member

LeaVerou commented Feb 4, 2024

Ah yes, the HTML view of these tests is an MVP and only deals with up to a certain level of nesting. The Node runner should work fine though.
If you can file a bug on this so I can track it, that would be very helpful!
Should I close this?

@lloydk
Copy link
Collaborator Author

lloydk commented Feb 4, 2024

There are some failing tests

This test fails because of a strict equality check:

name: "new Color(hsl string)",

The test passes if I change === to == here:

https://github.com/LeaVerou/htest/blob/70958deaf9bfd58ecb1f4ce247ffb7a9a3af2608/src/check.js#L43

Two other tests are failing because alpha is undefined. I'll submit a PR that fixes those two tests sometime today.

@LeaVerou
Copy link
Member

LeaVerou commented Feb 4, 2024

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed Topic: testing
Projects
None yet
Development

No branches or pull requests

2 participants