Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suggestion: Trap Interrupt signal (Ctrl+C) and exit gracefully #124

Closed
DannyBen opened this issue Jul 22, 2022 · 5 comments · Fixed by #128
Closed

Suggestion: Trap Interrupt signal (Ctrl+C) and exit gracefully #124

DannyBen opened this issue Jul 22, 2022 · 5 comments · Fixed by #128

Comments

@DannyBen
Copy link

DannyBen commented Jul 22, 2022

Right now, when pressing Ctrl+C after running a simple retest, exits with a ruby trace - sometimes a long one.

It would be nice to just see a friendly Goodbye or at least not see the trace.

$ retest
Setup identified: [RSPEC]. Using command: 'bundle exec rspec <test>'
Launching Retest...
^C
/home/vagrant/.rbenv/versions/3.1.0/lib/ruby/gems/3.1.0/gems/rb-inotify-0.10.1
/lib/rb-inotify/notifier.rb:208:in `directory?': Interrupt
        from /home/vagrant/.rbenv/versions/3.1.0/lib/ruby/gems/3.1.0/gems/rb-inotify-0.10.1/lib/rb-inotify/notifier.rb:208:in `block in watch'
        from /home/vagrant/.rbenv/versions/3.1.0/lib/ruby/gems/3.1.0/gems/rb-inotify-0.10.1/lib/rb-inotify/notifier.rb:202:in `each'
        from /home/vagrant/.rbenv/versions/3.1.0/lib/ruby/gems/3.1.0/gems/rb-inotify-0.10.1/lib/rb-inotify/notifier.rb:202:in `watch'

... snipped ...

(example rough fix in this fork)

@AlexB52
Copy link
Owner

AlexB52 commented Jul 23, 2022

Hi @DannyBen
Thanks for the issue. This sounds like a really good idea.
The change in your fork seems straight forward and I could add this feature to the next release 👍

Do you want to create a pull request or are you ok with me pushing that forward?

If you decide to create a pull request, please add a feature test in the ruby-bare app that simulates ctrl+c
Maybe using Process.kill("INT", pid) like in Signal documentation

@DannyBen
Copy link
Author

DannyBen commented Jul 23, 2022

Do you want to create a pull request or are you ok with me pushing that forward?

It will probably take you a fraction of the time it will take me to "round up" the implementation and tests, so feel free to go ahead and implement as and where you see fit.

And BTW - you may have noticed I also removed and .gitignored the .ruby-version file. I do not believe it should be a part of the repo, since each developer uses their own version, and all versions should work (starting from the minimum version in the gemspec of course).

@AlexB52
Copy link
Owner

AlexB52 commented Jul 23, 2022

And BTW - you may have noticed I also removed and .gitignored the .ruby-version file.

Yeah I saw that too, thanks for pointing it out, it makes sense. I'll double check this separately at a later stage 👍

@AlexB52
Copy link
Owner

AlexB52 commented Jul 23, 2022

Fixed in release 1.9.0
https://github.com/AlexB52/retest/releases

Thanks for these great suggestions @DannyBen

@DannyBen
Copy link
Author

Thanks for implementing :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants