add accept_confirm if turbo is available so system tests don't fail. fixes #519 #520
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
flavorjones
requested changes
Mar 19, 2025
| class ScaffoldGenerator < Base # :nodoc: | ||
| def fix_system_test | ||
| if turbo_defined? | ||
| gsub_file File.join("test/system", class_path, "#{file_name.pluralize}_test.rb"), "click_on \"Destroy this #{human_name.downcase}\", match: :first", "accept_confirm { click_on \"Destroy this #{human_name.downcase}\", match: :first }" |
There was a problem hiding this comment.
Can we make this a bit less brittle (in the case where the exact text of the system test changes) by using a more permissive regular expression? Maybe something like
Suggested change
| gsub_file File.join("test/system", class_path, "#{file_name.pluralize}_test.rb"), "click_on \"Destroy this #{human_name.downcase}\", match: :first", "accept_confirm { click_on \"Destroy this #{human_name.downcase}\", match: :first }" | |
| gsub_file File.join("test/system", class_path, "#{file_name.pluralize}_test.rb"), /(click_on.*Destroy this.*)$/", "accept_confirm { \\1 }" |
There was a problem hiding this comment.
Actually I tested this change and it works, so I added a commit doing it!
There was a problem hiding this comment.
Much nicer! thanks!
|
Yes, I feel the same. I didn't want to maintain a copy of the system test just for this. And looks much nicer with your version of the gsub! |
|
I also just added a very simple integration test to make sure the gsub applies cleanly. I don't think I'm ready to add a full Rails system test to CI. 😅 |
flavorjones
force-pushed
the
fix-system-tests
branch
from
ed7600b to
37c98c3
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Generated tests starting failing after we added the data turbo confirm attribute to delete links.
This PR fixes that by monkey patching the test_unit/scaffold generator so after everything is generated it will wrap the
click_oncall with aaccept_confirmcall, only if turbo is defined.will try to add test for this but wanted to confirm if the approach is something everyone would agree on.
Alternatives:
source_paths << File.expand_path("templates", __dir__)and copy thesystem_test.rb.ttfrom rails and add a guard there to check if turbo is available instead of gsubing the file in the generatordata-turbo-confirmfrom the template.