-
Notifications
You must be signed in to change notification settings - Fork 107
feat: allow to manually remove unused fixtures #156
base: master
Are you sure you want to change the base?
Conversation
67ebf65
to
b006082
Compare
This is a very useful feature. I'd suggest to enable it either with something like |
Ping? @assaf |
ping again.. @djanowski this time, maybe? :) |
Hi @mweibel thanks for submitting a PR! I'm not too keen on deleting files automatically, especially after some action did not happen. For example, think of accidentally leaving the flag on and running a subset of your test suite... But we can add reporting for sure. Maybe report automatically when debugging is on? |
Fair point, though hopefully people know what they do when they enable that flag. Also: people usually use version control systems.. If something gets deleted accidentally it can always be retrieved back. |
b006082
to
62fbf8e
Compare
I added now a README entry with the example code for removing fixtures. |
ping @djanowski |
ping @djanowski 😄 |
A function like that is very helpful... I have Node-Replay with a GraphQL API with a lot of mocks and it's a headcache to remove unused mocks ... +1 for you @mweibel ! 👍 |
@assaf @djanowski is there any chance this is going to get merged in? I see there are conflicts now because of a recent PR merge. I don't want to put more work into this if you won't merge it. |
Hello, @assaf @djanowski can you think about this feature please ? I've test the fork and it's ok for me. It's the only missing thing of this package. Thanks ! |
This is a work in progress as to ask for comments and also there's no convenient way to remove the fixtures afterwards, only a way to find which matchers haven't been called.
Maybe that's even enough, writing the removal is simple, e.g.:
This fixes #123
@assaf what's your opinion on this change?
a) would you provide mode or a separate function or nothing at all to actually remove the unused fixtures?
b) what's required to make you merge this PR?