:encoder_opts formatter option

[anthonator]

Wanted to get a sanity check before moving forward with other formatters to make sure I'm on the right track.

I'm not 100% sure what to do for testing. I basically need to test the behavior of Jason when passing an encoder option which seems clunky and brittle no matter how it gets implemented. At the moment I'm doing a partial regex match on a pretty printed string. Open to other ideas!

[bvobart]

Nice, looks like a pretty sane approach to me (but I'm not @AndrewDryga).

For testing, I think you have two options:

1. Mock the Jason.encode_to_iodata! function using Mimic during the encoder_opts tests, then simply verify whether the call to Jason includes the encoder options that were set on the formatter earlier. This makes it more of a unit test and you won't need to test Jason's behaviour.
2. Do it like you're doing now, pick a couple (or a bunch) of Jason encoder options of which you know exactly how they transform the output, then log something and check the output with regex matches. This would make it more of an integration test and therefore also tests whether specifying encoder_opts actually makes a functional difference in log output, but would fail when Jason makes a breaking change to their encoder options, or how they are passed in. That should constitute a major version upgrade on their side, so I doubt its much of a risk (especially looking at Jason's release history, I don't think there's a v2 coming soon).

Maybe both is better for completeness: the integration test for testing that specifying the encoder_opts actually makes a difference in log output, the unit test to ensure that all encoder_opts are being passed through.

[anthonator]

@bvobart thanks for reviewing and providing your thoughts!

Since I'm not the maintainer of this library I'd prefer not to introduce something like mocking. Also, since the integration with Jason is minimal for this feature I think the current approach should be fine for now from a maintainability perspective. Which is my main concern.

I'll keep moving forward with my current approach.

Thanks again for the review! ❤️

[anthonator]

@AndrewDryga this pull request is ready for review.

closes #130

[AndrewDryga]

Great work, thank you!

[anthonator]

@AndrewDryga thank you for the review! I updated this pull request with your suggestions.

[coveralls]

coverage: 100.0%. remained the same
when pulling c00876a on anthonator:encoder-opts
into 2e72e28 on Nebo15:master.

[AndrewDryga]

Thank you!

[anthonator]

No, thank you! 😄