feat: add reduceCalc for usage without PostCSS - #304
Conversation
ludofischer
left a comment
ludofischer
left a comment
There was a problem hiding this comment.
Could you add a test that exercises the API you would like to use? So that we don't change it inadvertently in the future.
|
Added. I mostly mirrored cases from plugin unit test. |
ludofischer
left a comment
ludofischer
left a comment
There was a problem hiding this comment.
Could you check you did not add too many redundant tests?
| }); | ||
|
|
||
| // --- Bare math functions (issue #189) ----------------------------------- | ||
| describe('reduceCalc: bare math functions', () => { |
There was a problem hiding this comment.
Aren't tests starting from here a to the end of the file a bit redundant? It understand testing option combinations as they might differ from the postcss plugin and they were touched by the change, but these tests below seem almost duplicates of existing ones.
There was a problem hiding this comment.
Why do you think they are redundant? Those seems to be non-PostCSS edge cases that the new, lower-level function have to cover.
What do you think if we remove those tests in the plugin suite instead? That means the plugin suite is to ensure that the source css is processed correctly given different plugin option combinations. There's no need to cover all edge cases etc.
There was a problem hiding this comment.
Maybe it's better to understand why tests are failing first.
2 participants
Fixes #298