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

Make tests compatible with BusterJS 0.7.4+ #40

Merged
merged 1 commit into from
Feb 11, 2014

Conversation

kamilgrymuza
Copy link

As of BusterJS 0.7.4 assert, refute and expect are no longer global which
breaks the test suite. Using buster.assert, buster.refute and buster.expect
solves the issue.

As of BusterJS 0.7.4 assert, refute and expect are no longer global which
breaks the test suite. Using buster.assert, buster.refute and buster.expect
solves the issue.
@mroderick
Copy link
Owner

@kamilgrymuza thank you for your pull request.

Could you remove the second commit (kamilgrymuza@ff0a84b), as that is also in another pull request?

@kamilgrymuza
Copy link
Author

You're welcome (thanks to you for your effort) and sure - I've removed the commit so that you can merge just the test changes. BTW - I wonder if I didn't introduce a "backwards incompatibility", i.e. whether buster.assert and others were there before 0.7.4 ? Anyway that turns out to be an issue I'm willing to fix this (by checking whether assert is available as a global or scoped under buster. and using what's available rather than making assumptions).

mroderick added a commit that referenced this pull request Feb 11, 2014
Make tests compatible with BusterJS 0.7.4+
@mroderick mroderick merged commit 8623761 into mroderick:master Feb 11, 2014
@mroderick
Copy link
Owner

@kamilgrymuza thank you for your pull request!

@kamilgrymuza
Copy link
Author

@mroderick you're welcome - I'm glad I could contribute a little :)

@mroderick
Copy link
Owner

Before publishing a new release, I'd like to improve things, so that we only require a local buster (which can be defined by package.json), so we won't run into this again.

@kamilgrymuza
Copy link
Author

Sounds like a good idea - I think also PhantomJS could be local ?

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.

2 participants