-
Notifications
You must be signed in to change notification settings - Fork 212
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
test(ava-xs): document bug in deepEqual #5398
base: master
Are you sure you want to change the base?
Conversation
@dckc where should a test like this go? where it is fails because it runs in regular |
The bug should be fixed rather than documented, no? Note also plans to move ava-xs and xsnap to endo: |
Sure, it should. Given priorities, this is what I have time for. And it's work that would have to be done anyway for a fix: a test. So… where should a test like this go? I can't find any testing of the |
I'm not used to testing test harnesses. But the filename you picked seems as good as any. |
What bug in deepEqual? I could not tell from the PR. |
Sure, when consuming a test harness. In this case, the package is making an assertion library. That's what needs tests, just like Ava does https://github.com/avajs/ava/blob/e387cba63ccf1a24a96b2375d61738ca38f48082/test-tap/assert.js#L368 I think the trick will be to define the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to talk this over.
@@ -142,6 +141,8 @@ async function runTestScript( | |||
assertionStatus[msg.status] += 1; | |||
if (msg.status === 'not ok') { | |||
console.warn({ ...msg, filename, label }); | |||
testStatus.total += 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to mix up counting tests with counting assertions. I wonder why the logic below here doesn't suffice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a thought: 3472734
closes: #XXXX
refs: #XXXX
Description
Security Considerations
Documentation Considerations
Testing Considerations