Remove unneeded subdirectories in generator tests #45653
Conversation
|
|
||
| FileUtils.cd(app_root) do |
There was a problem hiding this comment.
This test and the others like it are passing because the generator chdirs to its destination_root, rather than because setup does1. It would probably be better not to rely on that though.
If we want to clean up the cd blocks a bit, we could introduce an in_root helper method, like generators themselves have.
Alternatively, if we want to eliminate the cd blocks, we could introduce run_app_update and read_file helper methods (and maybe write_file for test_app_update_does_not_change_config_target_version). run_app_update would chdir as necessary, and read_file would simply File.expand_path with destination_root. That could be a follow-up PR, though.
Footnotes
-
setupactually callsensure_current_path, which chdirs toFile.expand_path(Dir.pwd). ↩
This pattern is useful in some cases, like moving the project folder from myapp to myapp_moved, or testing generated behavior based on the app name being hyphenated-app. However, for the majority of cases this is not needed and adds additional complexity to some assertion helpers. For example, `assert_file` always expands paths relative to `destination_root` instead of whatever the working directory is.
skipkayhil
force-pushed
the
rm-myapp-generator-tests
branch
from
0d8bbff
to
337fa75
Compare
|
Thank you, @skipkayhil! 😄 |
Summary
This pattern is useful in some cases, like moving the project folder
from myapp to myapp_moved, or testing generated behavior based on the
app name being hyphenated-app.
However, for the majority of cases this is not needed and adds
additional complexity to some assertion helpers. For example,
assert_filealways expands paths relative todestination_rootinstead of whatever the working directory is.
Other Information
Ref #45652