Skip to content

Adapt jasmine tests for jasmine-core@3 #3368

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

Closed
etpinard opened this issue Dec 21, 2018 · 7 comments · Fixed by #3969
Closed

Adapt jasmine tests for jasmine-core@3 #3368

etpinard opened this issue Dec 21, 2018 · 7 comments · Fixed by #3969
Assignees

Comments

@etpinard
Copy link
Contributor

etpinard commented Dec 21, 2018

npm i [email protected] [email protected]
npm run test-jasmine -- --nowatch

make numerous tests fail, spits out deprecation warnings:

image

and doesn't get along with our negateIf custom matcher

jasmine.Expectation.prototype.negateIf = function(negate) {
if(negate) return this.not;
return this;
};

image


More info about jasmine@3:

https://github.com/jasmine/jasmine/blob/master/release_notes/3.0.md

More info about karma-jasmine@2:

https://github.com/karma-runner/karma-jasmine/blob/master/CHANGELOG.md#200-2018-11-15

@etpinard etpinard changed the title Adapt jasmine test for jasmine-core@3 Adapt jasmine tests for jasmine-core@3 Dec 21, 2018
@etpinard
Copy link
Contributor Author

... and note that [email protected] is only compatible with karma-jasmine@2x

https://github.com/mnasyrov/karma-jasmine-spec-tags

@etpinard
Copy link
Contributor Author

See https://github.com/plotly/plotly.js/tree/bump-jasmine for the current state of affairs.

@etpinard
Copy link
Contributor Author

etpinard commented Jun 11, 2019

It seems that the new jasmine-core version shuffle the tests by default. We'll need to rewrite the tests have a @noCIdep tag.

EDIT: yep, it stated:

image

here under the breaking changes.

@etpinard
Copy link
Contributor Author

their docs look more thorough than before too:

https://jasmine.github.io/api/3.4/global

@etpinard etpinard self-assigned this Jun 12, 2019
@etpinard
Copy link
Contributor Author

Things are looking up!

By setting random: false thus making our test running in order (like they are currently) make npm run test-jasmine run smooth ⛵

https://github.com/plotly/plotly.js/compare/bump-jasmine

Now, the main "blocker" is negateIf. We either:

  • convert its declaration to something that jasmine-core@v3 is ok with (see src for inspiration), or
  • 🔪 it and adapt the tests currently using negateIf .

As adding stuff to the jasmine.Expectation prototype isn't recommended in jasmine docs, I'll vote for the 🔪 . Does anyone oppose?

@etpinard
Copy link
Contributor Author

etpinard commented Jun 12, 2019

[email protected] added expect(...).withContext('extra message').toEqual(...) to (finally) make toEqual(...) log extra info in failure messages.

https://github.com/jasmine/jasmine/blob/master/release_notes/3.3.0.md

🎉

@alexcjohnson
Copy link
Collaborator

🔪 negateIf is fine - it's just a convenience. If it becomes cumbersome without it, could always turn it into a plain function... just doesn't read quite as well as the method.

function negateIf(expectation, condition) {
   return condition ? expectation.not : expectation
}

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 a pull request may close this issue.

2 participants