Add espec stale feature - Githubissues

antonmi / espec

Elixir Behaviour Driven Development

Other

808 stars 62 forks source link

Add espec stale feature #292

Closed treble37 closed 4 years ago

treble37 commented 5 years ago

Added --stale option for parity with ExUnit test framework

The --stale command line option attempts to run only those test files which reference modules that have changed since the last time you ran this task with --stale.
Addresses https://github.com/antonmi/espec/issues/239
if there are no stale tests, mix espec --stale outputs No stale tests to run...

Notes for reviewers:

1) Right now stale is the last "filter" applied in ESpec.SuiteRunner#filter/2 which means right now if a user does both a focus and stale filter, if the focus test(s) isn't stale, it won't be run. Not sure if that's how this library wants to handle that.

antonmi commented 5 years ago

Thanks, @treble37 ! There are lots of stuff to review! Will do it ASAP.

antonmi commented 5 years ago

Hi, @treble37 ! Sorry for the late response. In general, it is very cool and it works! There is one design approach I don't like - you put "stale" specs filtering into ESpec.Runner module. But it is more logical to have such filtering on a higher level - in Mix.Tasks.Espec.

Please take a look at require_spec_helper\0 function in Mix.Tasks.Espec. It requires all the spec files so corresponding modules are available in ESpec.specs(). I would say that additional filtering for "stale" files should be applied there.

So, let's move the code from Runner to the Espec task.

treble37 commented 5 years ago

ok thank you for the great feedback @antonmi, I believe I made the changes in accordance with your suggestions (please let me know if I misunderstood - I assumed you meant look at require_spec_helper/1 and infer where to best add the stale file filtering) and it helped simplify the port from ExUnit. Also, please feel free to let me know if you want it all squashed into 1 commit or squash the first 2 commits and keep the commit updating the README separate.

antonmi commented 5 years ago

Hi @treble37 ! Look much better! But I suspect that you broke the functionality. Currently, "stale" feature detects changed specs but doesn't detect changes in the related modules. I've tried it with ESpec modules and also inside a separate project. When I change project modules, the corresponding specs won't run. Could you please check?

antonmi commented 5 years ago

@treble37 What is the status of this? Did you have a chance to check?

treble37 commented 5 years ago

Hi @antonmi, thanks for the reminder, I meant to type more than just giving your comment a 👍 , you are correct it needs some changes - it's in progress, just got caught up with some life stuff at the moment, but I'm hoping to finish this in the next couple of weeks.

antonmi commented 5 years ago

Thank you!

treble37 commented 4 years ago

hi @antonmi, took me a while to get back to this - but this feature should now detect changes in related modules as well as related tests. I made some comments about changes regarding the use of Code.require_file/1 since I was a bit unsure if this change might impact anything I didn't intend to. I tested with Elixir 1.6/1.7/1.8/1.9 against the ESpec codebase. One thing I don't have handy at the moment is a production code base that uses ESpec to test against.

antonmi commented 4 years ago

Awesome! Thank you, @treble37 !