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
Fix flaky tests, add a template per case on asset debugging tests #43188
Fix flaky tests, add a template per case on asset debugging tests #43188
Conversation
class ::PostsController < ActionController::Base ; end | ||
cases.each do |(view_method, tag_match)| | ||
app_file "app/views/posts/#{view_method}.html.erb", "<%= #{view_method} '#{contents}', skip_pipeline: true %>" | ||
::PostsController.define_method(:index) { render view_method } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would not this cause a warning since we are defining the same method more than once?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right, I didn't notice it on dev, but it appears in the CI. You can see it with ruby -w bin/test test/application/asset_debugging_test.rb
The query implementation approach is easier for avoid that warning, I am going to push another commit without warnings.
73fcd46
to
5dc8f97
Compare
The red tests seem unrelated. Could you squash commits? |
It avoids the problem when the file checker is not able to detect changes on the files during testing.
5dc8f97
to
0063fb3
Compare
It avoids the problem when the file checker is not able to detect changes
on the files during testing.
Summary
railties/test/application/asset_debugging_test.rb
are failing from time to time on CI. After some investigations, I discovered that it is caused due howFileUpdateChecker
works.FileUpdateChecker
requires 1 second to detect file changes in a file.This PR avoids the problem of creating a new template per case.
Other Information
It is a PR for debugging the issue #43174
Alternative approaches: