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
Handle empty backtrace from external filter #696
base: master
Are you sure you want to change the base?
Conversation
In the wild the backtrace_filter can be set by external filter (Rails do it for their silencer). This can lead to the case there the external backtrace_filer returns empty array. Other parts of minitest count on the fact that there will still be something in the backtrace to use, and can lead to issues such as "undefined method `split' for nil:NilClass" when producing deprecation warnings. Since we already had the logic to use all the backtrace when filtering too much, I've moved this logic to be used also for external filters.
Due to a bug on minitest that is being currently fixed, our tests are broken when they run as the "test_develop_pr_katello" job in Jenkins. The fix is published as minitest/minitest#696 however there is no version of minitest with it yet. A possible workaround is to prevent these assertions like this PR does.
Due to a bug on minitest that is being currently fixed, our tests are broken when they run as the "test_develop_pr_katello" job in Jenkins. The fix is published as minitest/minitest#696 however there is no version of minitest with it yet. A possible workaround is to prevent these assertions like this PR does.
By rails/rails#29572 and upgrading Rails from 5.1.2 -> 5.1.3, I hit an error ( |
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.
how about:
new_bt = backtrace_filter.filter bt
new_bt.empty? ? bt.dup : new_bt
Also, this suggestion is not meant as any rejection or "no" vote. If it is understood to be that, then I wish to retract it.
seeing this as seemingly just a side effect of Rails (5.2.3) being loaded. No application code setting a custom backtrace filter or anything like that. Checking the value of |
In the wild the backtrace_filter can be set by external filter (Rails do
it for their silencer). This can lead to the case there the external
backtrace_filer returns empty array. Other parts of minitest count on
the fact that there will still be something in the backtrace to use, and
can lead to issues such as "undefined method `split' for nil:NilClass"
when producing deprecation warnings.
Since we already had the logic to use all the backtrace when filtering
too much, I've moved this logic to be used also for external filters.