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

Differences in jruby and mri json output #84

Open
Goltergaul opened this issue Nov 17, 2017 · 8 comments
Open

Differences in jruby and mri json output #84

Goltergaul opened this issue Nov 17, 2017 · 8 comments

Comments

@Goltergaul
Copy link
Contributor

Goltergaul commented Nov 17, 2017

I often have the problem that there are minor differences between the output of an approval test in jruby and mri. We run our tests in both rubies so the approval test always fails for such cases in one of the rubies.

An example of this is that the following generates different approvals in jruby and mri:

verify(format: :json) { { weight: {} } }

In jruby:

weight: {

}

in mri:

weight: {
}

Is there any way to avoid this?

The problem seems to be jsons pretty_generate: https://github.com/kytrinyx/approvals/blob/master/lib/approvals/writers/json_writer.rb#L11

jruby:

JSON.pretty_generate({ weight: {} })
=> "{\n  \"weight\": {\n\n  }\n}"`

mri:

JSON.pretty_generate({ weight: {} })
=> "{\n  \"weight\": {\n  }\n}"
@kytrinyx
Copy link
Contributor

@Goltergaul I didn't realize there were differences between the two rubies.

I can see a few things we could do (there are likely other possible approaches as well).

  • don't use pretty-generate; minimize the whitespace as much as possible, so that we don't get problematic whitespace differences.
  • override pretty_generate in one environment to match the other
  • submit a patch to JRuby to make it behave the same as MRI

Of these I think maybe removing all the whitespace is the most reasonable approach, though that would be backwards incompatible, and would require a major version bump.

Having said that, we could add a flag/option so that the default is to use pretty generate, but if the flag is passed, then generate the JSON with no extra whitespace. That would be backwards compatible, and would still let you generate consistent (one hopes) output in both Rubies.

Thoughts?

@Goltergaul
Copy link
Contributor Author

Goltergaul commented Nov 22, 2017

@kytrinyx leaving out the pretty generate will make it too hard to spot differences in the approvals. Maybe we could use a pure ruby json serializer like https://github.com/flori/json ,this should yield the same json regardless of the platform. Might be a bit slower but this can probably be ignored for tests.

I'm not quite sure if that library is the best option though since its API is the same as the json gems, so if someone would use the normal json gem this might clash :/ Do you know any other pure ruby json library?

what do you think?

@markijbema
Copy link
Contributor

If it's just a few cases you could consider 'fixing' jruby's output. Maybe it's just stripping whitelines between braces?

@kytrinyx
Copy link
Contributor

leaving out the pretty generate will make it too hard to spot differences in the approvals.

You're absolutely right.

Maybe we could use a pure ruby json serializer

That could work, and I'd be fine with something that is a bit slower, if that lets the gem be more reliable.

If it's just a few cases you could consider 'fixing' jruby's output. Maybe it's just stripping whitelines between braces?

This is definitely worth trying—it might be a very small hack.

Another thing I realized could work (but I'm not particularly enamored with the idea) would be to do the serialization twice once with whitespace and once without it—verify on the one without it, but have the unminimized one to compare for humans. Wow, even just saying it out loud makes me like the idea even less :-)

@Goltergaul
Copy link
Contributor Author

Goltergaul commented Nov 22, 2017

If it's just a few cases you could consider 'fixing' jruby's output. Maybe it's just stripping whitelines between braces?

Might be. I'm not entirely sure what else is different or if there are other scenarios where this is different. Having the same serializer would be safest though. Unfortunately I could not really find a good one yet.

Another thing I realized could work (but I'm not particularly enamored with the idea) would be to do the serialization twice once with whitespace and once without it—verify on the one without it, but have the unminimized one to compare for humans. Wow, even just saying it out loud makes me like the idea even less :-)

Might also be an idea. Not sure if the non-pretty one could also have differences though.

@kytrinyx
Copy link
Contributor

Not sure if the non-pretty one could also have differences though.

Yeah, that's true. It could be worth a 30 minute experiment to find out.

@Goltergaul
Copy link
Contributor Author

what if we would not compare the json strings but the parsed version? If there is a difference we could show the diff of a pretty generated version of both the expected and actual hash?

So what i mean: verify(format: :json) { some_hash } would read the expected json string from the approval file, parse it and then compare it to JSON.parse(JSON.generate(some_hash)). If there is a difference then we can generate a pretty version of some_hash and the parsed version of the approval file and diff that one?

@Goltergaul
Copy link
Contributor Author

I'll try this one out and do a PR if that is alright @kytrinyx

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

3 participants