Catch FileAlreadyExistsError and exit cleanly #319
Conversation
timriley
left a comment
timriley
left a comment
There was a problem hiding this comment.
This is looking great, thanks @stephannv! :)
I've got one comment/suggestion regarding the testing. Let me know what you think and then we can merge this either now or after that change.
| it "exits with error message" do | ||
| expect do | ||
| within_application_directory { generate_action } | ||
| end.to raise_error SystemExit do |exception| | ||
| expect(exception.status).to eq 1 | ||
| expect(error_output).to eq Hanami::CLI::FileAlreadyExistsError::ERROR_MESSAGE % {file_path:} | ||
| end |
There was a problem hiding this comment.
All these tests for file conflict handling, spread across each generator, get pretty repetitious. With you having the experience of authoring these changes, do you have thoughts about keeping the tests more maintainable?
I think my preference would be not to worry about testing for conflicts in every generator spec, instead add e.g. a spec/unit/hanami/cli/commands/app/generate/file_conflict_handling_spec.rb that demonstrates the conflict handling for just one of the generators, which would be sufficient for us to know the code is doing what it needs.
This feels appropriate given that code is centralised into a single place, rather than something that actually requires special handling inside every generator class.
It would also make each generator spec simpler and more focused on its happy path.
Is this something you'd be happy to do, either here or in a follow-up PR?
There was a problem hiding this comment.
I agree, @timriley. I will do it in a next PR since these tests are already in the codebase currently, I just adapted them to work with the new exception. I will explore some alternatives and propose some changes.
There was a problem hiding this comment.
This sounds good to me, thanks!
There was a problem hiding this comment.
Regarding the testing approach I suggested above, we do have an existing example in this repo already: spec/unit/hanami/cli/commands/app/generate/slice_detection_spec.rb. This is a feature that applies to all generate commands, but it would have been wasteful to test the identical thing for every single one of them, so we extracted it to a dedicated file and tested it just once.
2 participants
Exits with status 1 when a command tries to generate an already existing file.
Fixes #181
This is a rebased version of #279