Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Keeping track of passed expectations, and the total number of expectations #575

Closed
wants to merge 6 commits into
from

Conversation

Projects
None yet
5 participants
Contributor

alextreppass commented Apr 14, 2014

As per pivotal#547

Contributor

alextreppass commented Apr 28, 2014

Hi @infews / @ragaskar / @sheelc , is there a timeline for getting this into Jasmine? Anything I can do to help move this forward?

@alextreppass alextreppass referenced this pull request in axemclion/grunt-saucelabs May 6, 2014

Open

Add support for jasmine 2 #109

Contributor

infews commented May 6, 2014

We can't take this pull as-is.

First, there are merge conflicts. Please re-base.

Second, you need more test work. You're not verifying that the new arrays are correctly utilized by the Spec. Also, I'd expect to verify that this data gets down to the reporters correctly.

Work on these and then we can have a discussion about if this makes sense to take.

Thanks for taking the time to start this work. It helps a lot to frame the discussion.

Contributor

alextreppass commented May 7, 2014

Great thanks, I'll rebase and add tests

Contributor

alextreppass commented May 13, 2014

Thanks @infews - I've added a test to verify the spec results keep track of totalExpectations, passedExpectations and failedExpectations.

I've had a dig around the other specs looking for a way to test the spec.result object is being sent down to the reporters and that it contains the expected result attributes. I'm not seeing an appropriate place at the moment -- can you advise?

Thanks

stdavis commented May 27, 2014

Would love to see this get accepted. It would help with a false positive problem that we have running jasmine tests in travis.

Contributor

sheelc commented Jun 3, 2014

The specs added seem like the appropriate place to me. There isn't currently a place that we make sure all the parts of the result object make it to the reporters. For example, commenting out the 'description' (an existing property) from the result object only causes the same spec @alextreppass has updated to fail.

@infews were you thinking that there are some new specs that need to be written along the lines of reporters receiving the expected results? We can do these as a new integration spec either in integration/EnvSpec.js (which has a test that a spec result has a 'status') or a new file.

Contributor

infews commented Jun 23, 2014

How related is this to #547?

Contributor

alextreppass commented Jun 23, 2014

Directly related

Contributor

alextreppass commented Jun 23, 2014

Hopefully the tests and code included above are enough to merge and close this out

Contributor

sheelc commented Jun 28, 2014

Merged in the commit manually here: 5f34be4

Made a slight change to remove the 'totalExpectations' counter since it's essentially denormalized data over the failedExpectations and passedExpectations arrays. Hopefully this isn't too big of a deal. If need be, I figure we can always add in more keys later (since they are much harder to remove without breaking changes).

Thanks for the PR! Hopefully this closes out this issue and #547?

steveoh commented Jul 17, 2014

So what is the status of this now? I'm trying to make jasmine tests work in sauce with the jsreporter. Following the rabbit hole leads me here. Has this been merged so #547 is then complete? Is there a release containing this code?

Contributor

sheelc commented Jul 17, 2014

The commit mentioned above (5f34be4) should have the relevant code changes to make this all work with slight change I mentioned above (no totalExpectations counter but arrays of all the passedExpectations and failedExpectations which makes it trivial to get the total expectation count.)

This code is just on master at the moment. There is no release containing it at this time.

steveoh commented Jul 17, 2014

Do you have any idea @sheelc when a release may be cut? We use grunt plugins and would love to be able to have all of our dependencies be able to update.

Contributor

sheelc commented Jul 18, 2014

Sorry @steveoh, I'm not sure. Based on the Tracker backlog, it looks like a 2.0.1 release is scheduled/coming up soon, but I don't think there's a concrete date that is promised.

@infews might be able to add more context here.

Contributor

infews commented Jul 23, 2014

Closing as this feels done to me.

@infews infews closed this Jul 23, 2014

alextreppass referenced this pull request in detro/jasmine-jsreporter Jul 26, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment