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

add change assertion - enforce a change to a value #218

Closed
wants to merge 1 commit into from

Conversation

timruffles
Copy link
Member

Hi - initial pull request to see what people think of a change assertion. It's useful to avoid false positives where a test that previously caused an effect doesn't any longer, but this failure is masked by something else having caused it.

addUser();
assert.equal(users.length,1);

This is prone to false positives: if we break addUser() and merge it into a branch where users started with a default user, we'd not get a failing test. With a change assertion we can be more specific about the change we want to see:

assert.change(function() { return users.length},addUser,{by: 1});

Would avoid this issue.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 7a062a6 on timruffles:add-change-assertion into 0e560c6 on chaijs:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 7a062a6 on timruffles:add-change-assertion into 0e560c6 on chaijs:master.

@logicalparadox
Copy link
Member

This is a great idea but would be better implemented as a plugin.

  • If you have not implemented a plugin before, chai-spies is a good place to see a project template.
  • Once completed and all tests pass open a PR adding your plugin to the official list here.

@timruffles
Copy link
Member Author

Okay - is this the default answer to new assertions? Would be good to note on the docs so people know not to open PRs etc.

@logicalparadox
Copy link
Member

Ahh, good point. I updated our wiki with more details and will put together a contributing document for next release.

@timruffles
Copy link
Member Author

Thanks :) Will take a look at adding a plugin.

On Saturday, 30 November 2013, Jake Luer wrote:

Ahh, good point. I updated our wikihttps://github.com/chaijs/chai/wiki#contributingwith more details and will put together a contributing document for next
release.


Reply to this email directly or view it on GitHubhttps://github.com//pull/218#issuecomment-29556753
.

@vesln
Copy link
Member

vesln commented Nov 30, 2013

Cool idea, though, I wanted to implement this back then. I would be a happy user of this plugin.

@timruffles
Copy link
Member Author

Ok, plugin up! Let me know what you think :)

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.

4 participants