Skip to content

Conversation

@ariporad
Copy link
Contributor

This isn't as good as it could be, as it doesn't have a nice assertion diff, because that's generated by power assert, which does all kinds of other things too that we don't want

Fixes #102.

@sindresorhus, @jamestalmage, @vdemedes: thoughs?

@sindresorhus
Copy link
Member

Could be more succinct. Something like, but better:

Assertion count of 4 does not match planned of 5

@ariporad
Copy link
Contributor Author

@sindresorhus: It's not related to the stack. Check my comment in #102. It's a power-assert thing.

@ariporad
Copy link
Contributor Author

@sindresorhus: Message fixed.

@jamestalmage
Copy link
Contributor

Just wondering here. Now that #371 is merged, should we just use that for this? Is a stack trace really going to be that helpful? Especially since test.exit() is now always deferred via a promise?

@sindresorhus
Copy link
Member

@jamestalmage I would still want to know which assertion and test that triggers the "incorrect plan count error".

@jamestalmage
Copy link
Contributor

We would be setting it via test._setAssertError, so we will still know which test triggered that.
We currently only check the assert count when the test exits, not for every assert, so you currently can't know which assertion put you over the limit now.

(test._setAssertError is a bit of a misnomer now).

@ariporad
Copy link
Contributor Author

@sindresourhus, @jamestalmage: yeah, I think we probably should. I'll work on that in the morning.

@jamestalmage
Copy link
Contributor

@ariporad - How close are you to finishing this?

@ariporad
Copy link
Contributor Author

@jamestalmage, @sindresorhus: OK, rebased onto master, and now uses AvaError.

@ariporad ariporad self-assigned this Dec 30, 2015
…ch the plan.

This isn't as good as it could be, as it doesn't have a nice assertion diff, because that's generated by power assert, which does all kinds of other things too that we don't want
@sindresorhus
Copy link
Member

Tests are failing.

@ariporad
Copy link
Contributor Author

@sindresorhus: Of course they are. Give me a second.

@ariporad
Copy link
Contributor Author

Naturally, They're all false negatives.

@sindresorhus: Do AvaError's need .operators, .actuals or .expecteds?

@ariporad
Copy link
Contributor Author

@jamestalmage: ^^^^^^^^^^

@sindresorhus
Copy link
Member

@ariporad Sorry, missed this one. Can you rebase on master and fix the merge conflict?

Do AvaError's need .operators, .actuals or .expecteds?

No

@sindresorhus
Copy link
Member

So, what should we do about this PR?

@sindresorhus
Copy link
Member

Would like to see this land, but closing for now to clean up inactive PRs. @ariporad Feel free to do a new one if you ever have time ;)

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 this pull request may close these issues.

3 participants