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

feat(config): Add a clearContext config to prevent clearing of context #1356

Merged
merged 1 commit into from
Jan 4, 2016
Merged

feat(config): Add a clearContext config to prevent clearing of context #1356

merged 1 commit into from
Jan 4, 2016

Conversation

cbayram
Copy link
Contributor

@cbayram cbayram commented Mar 18, 2015

Enable a clearContext config which when set to false:

  • prevents clearing of context window upon completion of running of the tests.
  • always (re)sets up context regardless of errors
    This configuration is useful when embedding the Jasmine html reporter within the context window.

@cbayram
Copy link
Contributor Author

cbayram commented Mar 19, 2015

If and when merged, karma-jasmine (v0.1.5, jasmine-1_0 branch) adapter needs to support clearContext config. It should conditionally "memory clean up" only when clearContext is true. Memory clean up wipes out test (suite & spec) results and prevents Jasmine HTML reporter or any other reporter in a multi reporter set-up from receiving the result data.

  this.reportSuiteResults = function(suite) {
    if(tc.config.clearContext) {
      // memory clean up
      suite.after_ = null;
      suite.before_ = null;
      suite.queue = null;
    }
  };

  this.reportSpecResults = function(spec) {
    ...
    if(tc.config.clearContext) {
      // memory clean up
      spec.results_ = null;
      spec.spies_ = null;
      spec.queue = null;
    }
    ...
  };

@maksimr
Copy link
Contributor

maksimr commented Mar 19, 2015

This configuration is useful when embedding the Jasmine html reporter within the context window.

I don't think this is good idea, but I don't fully understand the problem now.

Thanks

@cbayram
Copy link
Contributor Author

cbayram commented Mar 19, 2015

Jasmine Spec runner gives a user an interactive page of the test results and ability to single out a spec or suite. Karma executes tests and report the results always through the console or write to a static file through plugins.

The motivation is ability to embed the Jasmine version 1.3 HTML Reporter into Karma to see the tests visually on the page with every run.

Karma on Chrome

VS

Karma on Chrome with Jasmine HTML reporter embedded

To embed the Jasmine HTML reporter into Karma I added the jasmine css and jasmine-html reporter files to the files in the karma configuration.

frameworks: ['jasmine', 'requirejs'],

// list of files / patterns to load in the browser
files: [
  'path/to/jasmine.css',
  'path/to/jasmine-html.js',

Then I added the reporter to the jasmine environment so that jasmine reports to both html reporter and to Karma reporter in a multi-reporter setup.

var jasmineEnv = jasmine.getEnv();

jasmineEnv.updateInterval = 1000;
var htmlReporter = new jasmine.HtmlReporter();
jasmineEnv.addReporter(htmlReporter);

jasmineEnv.specFilter = function(spec) {
  return htmlReporter.specFilter(spec);
};

When all this is setup and Karma is running and re-running the tests, I noticed that it clears the context iframe/window src to 'about:blank' wiping out the Jasmine HTML reporter results from the page. Additionally, karma-jasmine nullifies jasmine suite and spec data, preventing Jasmine reporter from ever receiving the results.

The changes proposed take care of the above issues. Hopefully this makes more sense now and you are able to give better direction as how to proceed.

Thanks Maksim!

As a side note and out of the scope of this discussion, one minor issue remaining is that trying to single out a single suit and/or spec test to run with Jasmine HTML Reporter embedded in Karma. This works fine when on the 'DEBUG' window. In the main Karma window, the link points to /context.html?spec=The%20___%20module since it's in the iframe.

@dignifiedquire
Copy link
Member

@maksimr have you thought about this some more? I'm not a 100% sure either, but then again I never use jasmine so I'm probably not the best judge.

@cbayram As Jasmine is now at 2.0 is this still needed there?

@maksimr
Copy link
Contributor

maksimr commented May 15, 2015

@dignifiedquire no, but we rarely look at contex.html page in our projects when run test.
Usually we look at report in terminal or IDE.
For running single test case/suite we use fit/fdescribe. On TV we display result of tests on CI. CI use xml reporter for this purpose.
For me this feature is not so useful :(

Maybe it's useful/familiar for people who move from html reporter to karma.

@acnovais
Copy link

+1

I would like this option too as I use jasmine html reporter too.

@dignifiedquire
Copy link
Member

I think I'm getting around to merging this, if you could rebase onto the latest master please @cbayram

@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.

@GitCop
Copy link

GitCop commented Jan 4, 2016

Thank you for your contribution. We detected an issue with your commit message though and would ask you kindly to fix it.

  • Commit: 0e11830
    • Your commit message body contains a line that is longer than 80 characters
    • Your subject line is longer than 70 characters

Guidelines are available at http://karma-runner.github.io/0.13/dev/git-commit-msg.html and please do not hesitate to ask if you have any questions.


This message was auto-generated by https://gitcop.com

Enable a clearContext config which when set to false:
- prevents clearing of context window upon completion of running of the tests.
- always (re)sets up context regardless of errors
This configuration is useful when embedding the Jasmine html reporter
within the context window.
@dignifiedquire
Copy link
Member

Ignore @GitCop, commit message is fine

@GitCop
Copy link

GitCop commented Jan 4, 2016

Thank you for your contribution. We detected an issue with your commit message though and would ask you kindly to fix it.

  • Commit: 5fc8ee7
    • Your subject line is longer than 70 characters

Guidelines are available at http://karma-runner.github.io/0.13/dev/git-commit-msg.html and please do not hesitate to ask if you have any questions.


This message was auto-generated by https://gitcop.com

@dignifiedquire
Copy link
Member

Looks like there are problems with the tests though

@cbayram cbayram changed the title feat(config): Add a clearContext config to prevent clearing of context. feat(config): Add a clearContext config to prevent clearing of context Jan 4, 2016
@cbayram
Copy link
Contributor Author

cbayram commented Jan 4, 2016

I'm unable to run grunt test:client locally on OSX due to error noted in #1508

failed to add custom browserify preprocessor
{ [Error: Cannot find module 'karma/lib/preprocessor'] code: 'MODULE_NOT_FOUND' }
22 07 2015 00:05:14.797:INFO [framework.browserify]: 472361 bytes written (0.87 seconds)
22 07 2015 00:05:14.808:INFO [karma]: Karma v0.13.2 server started at http://localhost:9876/
22 07 2015 00:05:14.814:INFO [launcher]: Starting browser Chrome
22 07 2015 00:05:16.157:INFO [Chrome 43.0.2357 (Mac OS X 10.10.4)]: Connected on socket rTnfwhUtnLPRh4G1AAAA with id 19490884
Chrome 43.0.2357 (Mac OS X 10.10.4) ERROR
  Uncaught ReferenceError: require is not defined
  at /Users/kruk/dev/karma/test/client/karma.spec.js:1
Chrome 43.0.2357 (Mac OS X 10.10.4): Executed 0 of 0 ERROR (0.162 secs / 0 secs)
[Error: failed to add custom browserify preprocessor
{ [Error: Cannot find module 'karma/lib/preprocessor'] code: 'MODULE_NOT_FOUND' }]
Fatal error: Client unit tests failed.

@dignifiedquire
Copy link
Member

Are you sure you referenced the correct issue? I'm doing all development on OS X and have all tests passing

@dignifiedquire
Copy link
Member

You need to run the whole grunt task before though to generate all needed files

@dignifiedquire
Copy link
Member

Also travis is happy Oo, so just need you to sign the CLA I'm afraid

@googlebot
Copy link

CLAs look good, thanks!

@dignifiedquire
Copy link
Member

Thanks :octocat:

dignifiedquire added a commit that referenced this pull request Jan 4, 2016
feat(config): Add a clearContext config to prevent clearing of context
@dignifiedquire dignifiedquire merged commit e509f84 into karma-runner:master Jan 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants