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

Added support for jasmine 2.0.0 while maintaining similar interface #7

Closed
wants to merge 1 commit into from

Conversation

sclevine
Copy link
Contributor

@sclevine sclevine commented Mar 7, 2014

As far as I can tell, Jasmine 2 doesn't expose the number of passed expectations per spec, so the passedCount/failedCount/totalCount are all 1 or 0. It also doesn't expose the nesting in the callback functions, so we parse jasmine.getEnv().topSuite() instead.

But, this will allow most things expecting JSReporter output to work with jasmine 2. It may make more sense to leave this on a branch.

@alextreppass
Copy link
Contributor

I've had a play with this version, and due to jasmine not exposing enough information, it reports "durationSec": null for each suite/spec, and information on the number of specs total / passed / failed are no-longer available so we get binary passedCount/failedCount/totalCount which is less than ideal.

I wonder if we need to ask the jasmine guys to make their specs easier to instrument / report on. It seems that sort of information is no-longer exposed.

@sclevine
Copy link
Contributor Author

Hi @alextreppass,

I don't seem to be getting the same errors. I'm a maintainer of dc.js and wrote this to get OpenSauce working with our Jasmine 2 specs. Here's what some of the jsreporter output for our suite looks like: https://saucelabs.com/tests/ffb768eb7ba74d14a6f9d0b40985a549 and https://saucelabs.com/tests/8ad4480dfb434d90ac10d4b31abf84d8 (under metadata).

(Unfortunately, Sauce Labs seems to ignore this input when it gets too large. If I combine these suites together (with no extra tests) the reporter outputs correct looking JSON locally, but Sauce seems to parse it as null: https://saucelabs.com/tests/48c9d3f6f7414f12b415ab1477cb6cd9)

This could still be some issue with the above pull request. Let me know if you figure anything out.

@sclevine
Copy link
Contributor Author

Sauce Labs just got back to me, and they do believe it's a length issue with that field. So I don't believe there are any other issues with this pull request.

(@alextreppass: It should be compatible with Sauce Labs, for sufficiently small suites. They suggested I strip out all of the passing specs until they have a fix.)

@detro
Copy link
Owner

detro commented Mar 12, 2014

Let's do like this.
I'll create a Jasmine-2.0 branch and this can go against that instead of master, so we don't break compatibility with the previous Jasmine.

@detro
Copy link
Owner

detro commented Mar 12, 2014

OK.
I have created branch jasmine-2.0. Feel free to change this to be a PR against that.
I'm not sure you can re-target it, so maybe a new PR is required.

Close this one once we have that.

@sclevine
Copy link
Contributor Author

Re-targeting a branch is currently not possible. Check out #8. Closing this PR.

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.

None yet

3 participants