Handle Jasmine 3.3

[Lalem001]

Manually tested, and confirmed to work with [email protected] and [email protected].

Resolves #221

Needs:

• A way to automate testing in the different versions of Jasmine

[nasvillanueva]

Is it an option to use yarn to install multiple versions of jasmine-core? We can have a modified lib/index.js for [email protected] tests that would require.resolve version 3.3.0.

Something like

$ yarn add -D jasmine-core330@npm:[email protected]

// test/main-jasmine-330.js, mainly a copy of lib/index.js
...
var initJasmine = function (files) {
    var jasminePath = path.dirname(require.resolve('jasmine-core330'))
    ...

@johnjbarton Any ideas?

[Havunen]

One way to test it could be multiple sub projects with own package.json files, that way we could install different versions of Jasmine and do some tests with them

[bricss]

Any news on this and when it will get landed?

[Roaders]

I'm waiting for this fix as well

[johnjbarton]

Our objective here should be to support all jasmine 3.x. The jasmine team claims there is no API change, but rather that karma-jasmine never set the config correctly in the first place. So our code is not correct for 3.0 or 3.3.

I think we should make a breaking change: call the jasmine 3.x api as intended and not branch to keep compatibility with old API. But the code has to work for both 3.0 and 3.3.

If you agree remove the branching and mark the PR description with BREAKING CHANGE per
http://karma-runner.github.io/3.0/dev/git-commit-msg.html

[johnjbarton]

Why is this setting (failFast -> failFast) different from the old one (stopOnSpecFailure -> failFast)?

[Lalem001]

The old version of this source code would call jasmineEnv.stopOnSpecFailure. Per [email protected] L1095-L1098

this.stopOnSpecFailure = function(value) {
  this.deprecated('Setting stopOnSpecFailure directly is deprecated, please use the failFast option in `configure`');
  this.configure({failFast: !!value});
};

Note stopOnSpecFailure replaced with failFast option

[johnjbarton]

Why aren't these values consistent with the ones above?

[Lalem001]

These values are just indented to fit within the else block. They are otherwise unchanged from the previous version of karma-jasmine

[johnjbarton]

Is this statement true for both versions of Jasmine?

The purpose of this code is to set jasmine config, so we only need to insure that this works for all 3.x. If stopOnFailure is not a jasmine thing in 3.x we don't need to support passing it.

[Lalem001]

I cannot find stopOnFailure in Jasmine's source code going back all the way through 1.x.x.

Here is the karma-jasmine commit (0b59d69), and diff, that added stopOnFailure. It appears that before this commit, it was not easy to configure jasmine at all.

[johnjbarton]

Branching in tests is a sign of trouble. Please add a comment explaining why this is needed.

[Lalem001]

I chose to branch in the tests to work against the live versions of Jasmine. I do see that this kind of testing is more integration than unit testing.

If you would prefer I can mock out the two versions of Jasmine for testing purposes. This would lose out on potential advance warnings of future braking changes from Jasmine.

[Lalem001]

@johnjbarton

karma-jasmine never set the config correctly in the first place

Karma-jasmine was passing the jasmine configuration methods around and losing the jasmine binding in the process. The simplest fix, that @nasvillanueva actually had a PR (#223) up for, was to re-bind the method to jasmine. This has drawbacks, mentioned in the PR, and below.

call the jasmine 3.x api as intended and not branch to keep compatibility with old API

Jasmine 3.3.0 adds a new jasmine.configure method that accepts a configuration object. This method simply does not exist on older versions.

Jasmine 3.2.1 had helper methods that would set configuration properties that Karma-Jasmine used. Unfortunately use of these helpers methods result in deprecation warnings in [email protected].

I elected to avoid the deprecation warnings altogether when switching to 3.3.x, thus branching to jasmine.configure when available.

[johnjbarton]

Thanks for the thorough explanation.

Given all of this I think we should implement the 3.3 API and add a package.json dependency on 3.3, making this a breaking change. WDYT?

[Lalem001]

@johnjbarton

Given all of this I think we should implement the 3.3 API and add a package.json dependency on 3.3, making this a breaking change.

I agree. I will work on this after work. Would you prefer a new PR? Or a forced push to this branch/PR?

[johnjbarton]

Just force here is fine, thanks!

[Lalem001]

@johnjbarton Updated the PR. Let me know if you see anything you would like changed.

[johnjbarton]

Thanks for the PR and investigation of the issues.

[bricss]

When we can expect new version? 🤔

[bricss]

@johnjbarton Humanity depends on this patch, please release in to production 🙏