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

Fix for Issue #177 #178

Merged
merged 3 commits into from Jul 27, 2017
Merged

Conversation

reeteshranjan
Copy link
Contributor

Cause

The code that works with Jasmine is completely different from the one working with Mocha and QUnit. The results sent to _report api in case of Jasmine have very limited information resulting in the case as described in this issue.

It was found that the Jasmine reports are as good as Mocha ones and it's our code that does not let it pass through in its entirety causing the limited information.

Modifications

  • jasmine-jsreporter.js - It has the code that prepares per test suite report, and cuts out detailed information. It was modified to retain detailed information. Also some 'summarization' attributes turned out to be duplicate state post these modifications, and they were removed.
  • jasmine-plugin.js - It has code that further summarizes the reports created per suite in jasmine-jsreporter.js before making a call to the _report api. It was modified to retain detailed information.
  • server.js - for Jasmine case, a 'reformatting' adapter function is added. With the changes done in above files, detailed information as good as Mocha is available; however, the structure of the information is different. This function adapts the structure such that the final reports are seen in the format as in README.md. This function is called in the handler for _report api.

Limitations/Differences

  • There is no equivalent of _progress api added. The tests attribute at the topmost level in the final test reports is built in the _progress api handler in case of Mocha. This api gets called as each test completes, in case of Mocha. In Jasmine case, we build the tests attribute completely in the handler for _report api call.
  • In case of Mocha, whether it's an assertion failure or an uncaught exception, stack trace is available for both cases. In Jasmine case, stack trace is available only in case of uncaught exceptions. In case of assertion failures, Jasmine provides only a message, not a trace. This is a limitation that we do not have any control over as it exists in the source test reports data output by Jasmine.

The code that works with Jasmine is completely different from the
one working with Mocha and QUnit. The results sent to '_report' API
in case of Jasmine has very limited information resulting in the
case as described in this issue.

It was found that the Jasmine reports are actually as good as Mocha
and it's our code that does not let it pass through in its entirety
causing the limited information.

- jasmine-jsreporter.js: it has the code that prepares per test suite
  report, and cuts out detailed information. It was modified to retain
  detailed information. Also some 'summarization' attributes are
  just duplicate state, and hence they were removed

- jasmine-plugin.js: it has code that further summarizes the reports
  created per suite in jasmine-jsreporter.js before making a call to
  the '_report' API. It was modified to retain detailed information.

- server.js: for Jasmine case, a 'reformatting' adapter function is
  added. With the changes done in above files, detailed information
  as good as Mocha is available; however, the structure is different.
  This function adapts the structure such that the final reports are
  in the format desired. This function is called in the handler for
  '_report' API.

Limitations/Differences

- There is no equivalent of '_progress' API here. The 'tests' attribute
  at the topmost level is built in the '_progress' API in case of Mocha.
  This API gets called as each test completes with Mocha. In Jasmine case,
  we build the 'tests' attribute in the handler for '_report' call itself.
- In case of Mocha, whether it's an assertion failure or an uncaught
  exception, stack trace is available for both cases. In Jasmine case,
  stack trace is available only in case of uncaught exceptions. In case
  of assertion failures, Jasmine provides only a message, not a trace.
@reeteshranjan
Copy link
Contributor Author

Fixes #177

@reeteshranjan
Copy link
Contributor Author

The build failed. After debugging, it does not seem related to my changes. The external tests are failing. I saw that spine v1.x test failed. Debugging showed, also verified with sessions like this https://www.browserstack.com/automate/builds/7b3bf7b14380d11a15897f9d968c1bb81dde214b/sessions/366add5b8fdc17c56d09c9202b369ffe7153a428, that what is happening that all unit tests are actually executing; but the sessions are timing out for some reason and browserstack-runner does not get to know that all 63 tests actually executed, and eventually leading to failure.

@reeteshranjan
Copy link
Contributor Author

I got further from the timeout issue by changing the browser/platform for spine v1.0 and running only that test. It seems Jasmine version still is coming in the way, as my changes were tested for Jasmine 1.3.1 alone. I would look into this further.

The external test spine-v1 had 2 issues:

- Bad Jasmine output data with NULL suite description which was not
  hanlded by the changed code
- Safari 5.1 is timing out. Changing to Safari 9.0 works.
@reeteshranjan
Copy link
Contributor Author

Bug in terms of not handling bad Jasmine data (comes in to play with spine v1.0 test) was fixed. Also, Safari 5.1 just won't work. Have changed spine v1.0 test browser to Safari 9.0, for which the test went through.

@pulkitsharma07
Copy link
Contributor

Awesome !
Can you make the changes in readme regarding this ?
Mention the jasmine version(s) supported for jasmine in the test_framework section.

@AnkurGel AnkurGel changed the base branch from master to jasmine_reports July 27, 2017 08:50
@AnkurGel AnkurGel merged commit f40c548 into browserstack:jasmine_reports Jul 27, 2017
@reeteshranjan
Copy link
Contributor Author

@pulkitsharma07 @AnkurGel, sorry I was away and checked the messages today. Is there anything you need from me still?

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