search

marick / Midje

Midje provides a migration path from clojure.test to a more flexible, readable, abstract, and gracious style of testing
MIT License
1.69k stars 129 forks source link

Disallow faking of `clojure.core` functions #434

Closed philomates closed 6 years ago

philomates commented 6 years ago

In https://github.com/marick/Midje/pull/433 I added the use of set in some code that is executed while functions being faked are redefined to the fake. This means if you add a fake for clojure.core/set, it will break Midje.

One way around this is to add set to the blacklist here https://github.com/phillipm/Midje/blob/5c80966287707f7d3f92ab417843698cbbca59be/src/midje/parsing/2_to_lexical_maps/fakes.clj#L26 This approach is not good for the maintanability and stability of Midje, because somebody might forget to do so. For instance, I discovered that I should have added set to the blacklist only after encountering a strange test failure in a production service. Imagine how confused one might be if they discovered this issue before I fixed it!

Thus, I propose that we disallow faking of any function defined in clojure.core. This adds limitations to what Midje is capable of, but is much safer, and I personally think that one should really avoid mocking core language constructs.

philomates commented 6 years ago

Turns out midje already has a crazy dynamic check to make sure you don't mock functions that will be used in midje's runtime. Beforehand, such mocking failures would be swallowed, causing non-helpful test failures to be shown to the user. I cleaned up the error messages so that the user is warned in an actionable way in https://github.com/marick/Midje/pull/440, so this can be closed