Passing an empty Array as the ignore_files value generates a RegexpError
etagwerker opened this issue · 5 comments
Hi @danmayer! 👋
Thanks for maintaining churn!
It seems that I've found an edge case while trying to use churn from metric_fu. I recently noticed that the Appveyor build was failing: https://ci.appveyor.com/project/bf4/metric-fu/builds/32531867/job/ja1pbtisllw801sv -- Failure is happening within the ChurnCalculator#reject_ignored_files method.
Here is the backtrace:
******* STARTING METRIC churn
Traceback (most recent call last):
22: from bin/metric_fu:9:in `<main>'
21: from /Users/etagwerker/Projects/fastruby/metric_fu/lib/metric_fu/cli/client.rb:19:in `run'
20: from /Users/etagwerker/Projects/fastruby/metric_fu/lib/metric_fu/cli/helper.rb:19:in `run'
19: from /Users/etagwerker/Projects/fastruby/metric_fu/lib/metric_fu/run.rb:9:in `run'
18: from /Users/etagwerker/Projects/fastruby/metric_fu/lib/metric_fu/run.rb:19:in `measure'
17: from /Users/etagwerker/Projects/fastruby/metric_fu/lib/metric_fu/run.rb:19:in `each'
16: from /Users/etagwerker/Projects/fastruby/metric_fu/lib/metric_fu/run.rb:21:in `block in measure'
15: from /Users/etagwerker/Projects/fastruby/metric_fu/lib/metric_fu/reporting/result.rb:48:in `add'
14: from /Users/etagwerker/Projects/fastruby/metric_fu/lib/metric_fu/generator.rb:104:in `generate_result'
13: from /Users/etagwerker/Projects/fastruby/metric_fu/lib/metric_fu/metrics/churn/generator.rb:12:in `emit'
12: from /Users/etagwerker/Projects/fastruby/metric_fu/lib/metric_fu/metrics/churn/generator.rb:35:in `run'
11: from /Users/etagwerker/Projects/fastruby/metric_fu/vendor/bundle/ruby/2.5.0/gems/churn-1.0.5/lib/churn/calculator.rb:56:in `report'
10: from /Users/etagwerker/Projects/fastruby/metric_fu/vendor/bundle/ruby/2.5.0/gems/churn-1.0.5/lib/churn/calculator.rb:95:in `analyze'
9: from /Users/etagwerker/Projects/fastruby/metric_fu/vendor/bundle/ruby/2.5.0/gems/churn-1.0.5/lib/churn/calculator.rb:191:in `calculate_revision_changes'
8: from /Users/etagwerker/Projects/fastruby/metric_fu/vendor/bundle/ruby/2.5.0/gems/churn-1.0.5/lib/churn/calculator.rb:191:in `each'
7: from /Users/etagwerker/Projects/fastruby/metric_fu/vendor/bundle/ruby/2.5.0/gems/churn-1.0.5/lib/churn/calculator.rb:196:in `block in calculate_revision_changes'
6: from /Users/etagwerker/Projects/fastruby/metric_fu/vendor/bundle/ruby/2.5.0/gems/churn-1.0.5/lib/churn/calculator.rb:208:in `calculate_revision_data'
5: from /Users/etagwerker/Projects/fastruby/metric_fu/vendor/bundle/ruby/2.5.0/gems/churn-1.0.5/lib/churn/calculator.rb:279:in `parse_logs_for_updated_files'
4: from /Users/etagwerker/Projects/fastruby/metric_fu/vendor/bundle/ruby/2.5.0/gems/churn-1.0.5/lib/churn/calculator.rb:287:in `reject_ignored_files'
3: from /Users/etagwerker/Projects/fastruby/metric_fu/vendor/bundle/ruby/2.5.0/gems/churn-1.0.5/lib/churn/calculator.rb:287:in `reject'
2: from /Users/etagwerker/Projects/fastruby/metric_fu/vendor/bundle/ruby/2.5.0/gems/churn-1.0.5/lib/churn/calculator.rb:287:in `block in reject_ignored_files'
1: from /Users/etagwerker/Projects/fastruby/metric_fu/vendor/bundle/ruby/2.5.0/gems/churn-1.0.5/lib/churn/calculator.rb:287:in `any?'
/Users/etagwerker/Projects/fastruby/metric_fu/vendor/bundle/ruby/2.5.0/gems/churn-1.0.5/lib/churn/calculator.rb:287:in `block (2 levels) in reject_ignored_files': empty char-class: /[]/ (RegexpError)
Here are the arguments that we are using to create an instance:
ARGS: {:start_date=>"\"1 year ago\"", :minimum_churn_count=>10, :ignore_files=>[], :data_directory=>"tmp/metric_fu/scratch/churn"}
Here is a potential solution that is working locally for me:
def reject_ignored_files(files)
return files if @ignores.compact.any?
files.reject{ |file, _| @ignores.any?{ |ignore| /#{ignore}/ =~ file } }
end
If you're interested in a solution like that, I can submit a PR.
Please let me know.
Thanks!
So churn seems to expect a string for ignores...
@ignores = (options.fetch(:ignore_files){ options[:ignores] || @ignores }).to_s.split(',').map(&:strip)
This will fix your issue, #80
Although I wanted to see if you could dig a bit closer, it seems like this should be passing :ignore_files as a comma-delimited string...
ARGS: {:start_date=>"\"1 year ago\"", :minimum_churn_count=>10, :ignore_files=>[], :data_directory=>"tmp/metric_fu/scratch/churn"}
see the code above that splits and the CLI expects a string arg, https://github.com/danmayer/churn/blob/master/bin/churn#L26
@danmayer Good point. I don't understand why the default values we are setting in metric_fu don't match the documentation: https://github.com/metricfu/metric_fu#example-configuration -- I'm new to the project and trying to help with maintenance.
I've submitted a PR to fix metric_fu's churn integration: https://github.com/metricfu/metric_fu/pull/313/files#diff-80da03c2cd45ea7450503d07f9c505ffL11
So I'm going to try setting that value to an empty string by default, which should work... :)
ok release 1.0.6 should resolve this issue
@danmayer Awesome, thanks for the quick response! I'll check this later today and close the issue. 👍
I can confirm this is fixed in v1.0.6 🎉