Support test sets for printer options#132
Support test sets for printer options#132gustavoavena wants to merge 1 commit intofourmolu:masterfrom
Conversation
|
|
||
| Run `cabal test` and `./format.sh` before submitting any pull requests. | ||
|
|
||
| If you're adding a new customization, please add a new test set for it in the `PrinterSpec` module. |
There was a problem hiding this comment.
BTW, with #129 in place this would be mandatory as it derives test cases from the option type.
| maintainer: | ||
| Matt Parsons <parsonsmatt@gmail.com> | ||
| George Thomas <georgefsthomas@gmail.com> | ||
|
|
There was a problem hiding this comment.
Per DEVELOPER.md, can we avoid making formatting changes? To reduce merge conflicts with ormolu
|
I really like the overall thrust of this PR! However, I'm personally particularly sensitive to changes that would cause bad merge conflicts when merging Ormolu changes from upstream. @gustavoavena what do you think about keeping |
|
I added an update to #49. To reiterate what I said in that issue, many thanks for the work you've put into this PR. In light of the new direction, though, I'll be closing this PR for now. Please let me know if you have any questions, comments, or concerns. Thanks! |
What
Refactor the
PrinterSpecmodule to support Test Sets.Each set will test an instance of
PrinterOpts, i.e. a combination of fourmolu's customizations.How
Define data types to represent a test case and a test set:
PrinterOpts, which have the combination of options you want to test.This decreases the coupling between the code that defines the configs, examples and file list from the code that runs and checks the examples.
Why
See #49. Fourmolu now has a lot of options, and we need an easier way to test them properly.
With this change, testing a new combination of options should also be much easier.
I don't think this should necessarily be the final state, but IMO it's a step in the right direction.
See an example of adding a test suite for a new option in #102.
RFC and Possible Follow Ups
I added a
canBreakIdempotenceboolean to the test set data type, because some of the customizations currently under review can do this (e.g. Automatic line breaking at specific limit #102, Do hang record constructors when turned on setting #103).checkExample, which shouldn't have any logic specific to an option.We could decouple things even more?