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

Instructions for installing & using seem to be incomplete, throwing ERROR #29

Closed
benjaminapetersen opened this issue Sep 14, 2017 · 13 comments

Comments

@benjaminapetersen
Copy link

Following the example in the README.md:

$ npm install karma-jasmine-diff-reporter --save-dev
my-project@1.3.0-alpha ~/github/my-project
└── karma-jasmine-diff-reporter@1.1.0

and

reporters: ['jasmine-diff', 'progress', 'coverage'],

results in:

Running "karma:unit" (karma) task
14 09 2017 14:07:29.870:ERROR [reporter]: Can not load reporter "jasmine-diff", it is not registered!
  Perhaps you are missing some plugin?
14 09 2017 14:07:30.133:ERROR [karma]: Found 1 load error
@mradionov
Copy link
Owner

mradionov commented Sep 14, 2017

Hey, do you have "plugins" option in your Karma configuration file? If you do, then you also have to add the reporter in there, that's how karma works (see "Loading Plugins" section)

@benjaminapetersen
Copy link
Author

benjaminapetersen commented Sep 15, 2017

Ah, my bad. Thanks. Might be worth updating the README.md for others who oops like I did.

So added that, but I still get the same output:

	Expected $[0] to have properties
	    candy: 'bars'
	@test/spec/services/membership/membershipSpec.js:284:7

Essentially I'm running:

expect(Module.generateConfig(foo, bar)).toEqual({ candy: 'bars' }); // except this is a 65 line object...

Guessing this hits the pitfalls section where you mention MAX_PRETTY_PRINT_DEPTH MAX_PRETTY_PRINT_ARRAY_LENGTH?

@benjaminapetersen
Copy link
Author

Ran a few follow-up tests looking similar to:

      expect({
        name: 'foo',
        value: [1,2,3,4,5,6],
        things: {stuff: [{andThings: 'shizzle'}]}
      }).toEqual({
        name: 'foo',
        value: [1,2,4,4,5,7],
        things: {stuff: [{andThings: 'shizzlesss'}]}
      });

Can't be the pitfalls issue, still getting standard output:

	Expected $.value[2] = 3 to equal 4.
	Expected $.value[5] = 6 to equal 7.
	Expected $.things.stuff[0].andThings = 'shizzle' to equal 'shizzlesss'.

@benjaminapetersen
Copy link
Author

My setup is:

"grunt-karma": "^2.0.0",
"jasmine-core": "^2.8.0",
"karma": "^1.7.1",
"karma-jasmine": "^1.1.0",

@mradionov
Copy link
Owner

You are using a jasmne version, which has it's own diff reporting now. My reporter only works with the old jasmine output format. In order to use legacy jasmine output format and my reporter as well, use a 'legacy' option in configuration file. (https://github.com/mradionov/karma-jasmine-diff-reporter#legacy)

I will update documentation to adress both of these issues.

@benjaminapetersen
Copy link
Author

Thanks, thats helpful. Is there a recommended path? If what I am seeing is Jasmine's diff, its not awesome :)

@benjaminapetersen
Copy link
Author

Have to say, your output:

screen shot 2017-09-15 at 10 16 22 am

Is infinitely better than:

	Expected $[0] to have properties
	    candy: 'bars'
	@test/spec/services/membership/membershipSpec.js:284:7

Once the diff is beyond a single simple discrepancy. Unless I'm using Jasmine's default incorrectly, this hardly seems "legacy"!

@mradionov
Copy link
Owner

I guess you choose what works for you. My position is to respect whatever the creators of the Jasmine lib do, not to change it's default behavior for the end users out of the box. Maybe I should consider releasing a new major version, which will have a legacy output by default, if Jasmine's diff is really that bad. It will be either one way or another, because these two ways of displaying diffs are completely different.

@benjaminapetersen
Copy link
Author

I took legacy = deprecated. Meaning it might be supported soon. Is that incorrect? If not, great. I'm trying to sell my team on using your lib as we upgrade our toolchain, but since it sounded already deprecated there were reservations.

Thanks for the feedback!

@mradionov
Copy link
Owner

Turning the option on (legacy: true) means that you don't want to use new Jasmine built-in diffs and rather switch back to old Jasmine "legacy" output, so the reporter, which was designed to work with the old output, could be used to highlight diffs. It's more about Jasmine "legacy", not reporter "legacy".

@benjaminapetersen
Copy link
Author

Gotcha. Because with the new output there is no good way to actually do a diff.

@mradionov
Copy link
Owner

I've updated the docs to address both of your issues and released a new patch version in case anyone will have the same questions. Thank you for bringing this up.

Version 1.1.1 is available on npm.

@benjaminapetersen
Copy link
Author

Great, thx!

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

No branches or pull requests

2 participants