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.

Sign up for GitHub

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

Jump to bottom

configureDefaultReporter() options are lost #95

Closed
tomv564 opened this issue Oct 2, 2016 · 2 comments · Fixed by #98
Closed

configureDefaultReporter() options are lost #95

tomv564 opened this issue Oct 2, 2016 · 2 comments · Fixed by #98
Labels
bug ready for work

Comments

@tomv564
Copy link
Contributor

tomv564 commented Oct 2, 2016

In 2.5.x versions of jasmine-npm, calling configureDefaultReporter before calling execute appears to lose the provided options.

For example: configureDefaultReporter({showColors: false}) has no effect, the only way to turn colours off is by setting jasmine.showingColors = false.

I believe this behaviour appeared with the removal of the this.reportersCount === 0 check in 5989948#diff-c413964b3d28796db67dd5fe1f43851bR136.

Perhaps the fix is to check this.defaultReporterAdded before calling configureDefaultReporter in execute?
@jasonwells
Copy link

We are seeing the same behavior in 2.5.2. When {print: function(){}} is passed, the prints of . for success and F for failures is still occurring.

@cnishina cnishina mentioned this issue Oct 28, 2016
Closed
cnishina added a commit to cnishina/jasmine-npm that referenced this issue Oct 28, 2016
@cnishina
fix - prevent execute method from overriding configureDefaultReporter
0ee6e09 
- remove provideFallbackReporter since it is no longer used
- prevent execute method from overriding configureDefaultReporter

closes jasmine#95
@cnishina cnishina mentioned this issue Oct 28, 2016
Merged
@slackersoft slackersoft added the bug label Nov 8, 2016
@slackersoft
Copy link
Member

@tomv564 I think you're correct about the solution. I think we missed this use case in the recent reporter refactoring. We'd be happy to review a pull request that gates the default configuration of the default reporter on whether or not it has already been configured.

Thanks for using Jasmine!

cnishina added a commit to cnishina/jasmine-npm that referenced this issue Nov 8, 2016
@cnishina
Fix setOptions to the ConsoleReporter if it exists on the options param
ac08bc5 
- setOptions to the ConsoleReporter if it exists on the options param
- TODO(cnishina): write tests

closes jasmine#95
cnishina added a commit to cnishina/jasmine-npm that referenced this issue Nov 8, 2016
@cnishina
Fix setOptions to the ConsoleReporter if it exists on the options param
e5cfb1e 
- setOptions to the ConsoleReporter if it exists on the options param
- TODO(cnishina): write tests

closes jasmine#95
cnishina added a commit to cnishina/jasmine-npm that referenced this issue Nov 8, 2016
@cnishina
Fix setOptions to the ConsoleReporter if it exists on the options param
2ae4cb6 
- setOptions to the ConsoleReporter if it exists on the options param
- test setOptions should not override existing options if set multiple times

closes jasmine#95
heathkit added a commit to heathkit/protractor that referenced this issue Nov 8, 2016
@heathkit
chore(deps): Downgrade jasmine to 2.4.1
cd8fa56 
Upgrading to 2.5.2 causes angular#3606. We'll need to wait on a fix for
jasmine/jasmine-npm#95 before upgrading before upgrading
@heathkit heathkit mentioned this issue Nov 8, 2016
Merged
cnishina pushed a commit to angular/protractor that referenced this issue Nov 9, 2016
@heathkit @cnishina
chore(deps): Downgrade jasmine to 2.4.1 (#3715)
a1c8a23 
Upgrading to 2.5.2 causes #3606. We'll need to wait on a fix for
jasmine/jasmine-npm#95 before upgrading before upgrading
@tomv564 tomv564 mentioned this issue Nov 13, 2016
Merged
cnishina added a commit to cnishina/jasmine-npm that referenced this issue Dec 2, 2016
@cnishina
Fix setOptions to the ConsoleReporter if it exists on the options param
1d51773 
- setOptions to the ConsoleReporter if it exists on the options param
- test setOptions should not override existing options if set multiple times

closes jasmine#95
slackersoft pushed a commit that referenced this issue Dec 2, 2016
Merge branch 'fix_defaultreporter' of https://github.com/cnishina/jas…
f5d1d09 
…mine-npm into cnishina-fix_defaultreporter

- Merges #98 from @cnishina
- See also #95
bodyduardU pushed a commit to bodyduardU/protractor that referenced this issue Dec 5, 2022
@heathkit @cnishina
chore(deps): Downgrade jasmine to 2.4.1 (#3715)
e88797d 
Upgrading to 2.5.2 causes #3606. We'll need to wait on a fix for
jasmine/jasmine-npm#95 before upgrading before upgrading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug ready for work
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix setOptions to the ConsoleReporter if it exists on the options param cnishina/jasmine-npm
3 participants
@slackersoft @jasonwells @tomv564