From 2dfe28cc147974749e5ffeafaa3fa43fd5a611fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20T=C3=B3th?= Date: Tue, 13 Feb 2018 16:46:22 +0100 Subject: [PATCH] Pronto integration Integration of pronto required to add few new gems and to update some of the gems. The old method which provided the linter launching was reworked to launch pronto which provide the linter launching. The output of pronto is the result of all linters. The pronto result is transformed into a structure which match the original one. This integration required to change few tests and add new ones. --- Gemfile | 9 +- Gemfile.lock | 60 ++++-- app/workers/concerns/code_analysis_mixin.rb | 75 ++++++- .../rubocop_checker/message_builder_spec.rb | 2 +- .../concerns/code_analysis_mixin_spec.rb | 199 ++++++++++++++++-- 5 files changed, 299 insertions(+), 46 deletions(-) diff --git a/Gemfile b/Gemfile index 5b25e92d..ab128883 100644 --- a/Gemfile +++ b/Gemfile @@ -47,15 +47,20 @@ gem 'travis', '~> 1.7.6' gem 'awesome_spawn', '>= 1.4.1' gem 'default_value_for' -gem 'haml_lint', '~> 0.20.0', :require => false +gem 'haml_lint', '~> 0.27.0', :require => false gem 'more_core_extensions', '~> 2.0.0', :require => 'more_core_extensions/all' gem 'rubocop', '~> 0.52.0', :require => false gem 'rugged', :require => false -gem 'octokit', '~> 4.6.0', :require => false +gem 'octokit', '~> 4.7.0', :require => false gem 'faraday', '~> 0.9.1' gem 'faraday-http-cache', '~> 2.0.0' +gem 'pronto', '~> 0.9.5', :require => false +gem 'pronto-haml', '~> 0.9.0', :require => false +gem 'pronto-rubocop', :require => false +gem 'pronto-yamllint', :require => false + group :development, :test do gem 'rspec' gem 'rspec-rails' diff --git a/Gemfile.lock b/Gemfile.lock index d3d95f76..e73bd5a1 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -41,7 +41,7 @@ GEM tzinfo (~> 1.1) addressable (2.4.0) arel (6.0.4) - ast (2.3.0) + ast (2.4.0) awesome_spawn (1.4.1) axiom-types (0.1.1) descendants_tracker (~> 0.0.4) @@ -125,19 +125,26 @@ GEM multi_json (~> 1.0) net-http-persistent (~> 2.9) net-http-pipeline + gitlab (4.3.0) + httparty + terminal-table globalid (0.4.0) activesupport (>= 4.2.0) - haml (4.0.7) + haml (5.0.4) + temple (>= 0.8.0) tilt - haml_lint (0.20.0) - haml (~> 4.0) + haml_lint (0.27.0) + haml (>= 4.0, < 5.1) + rainbow rake (>= 10, < 13) - rubocop (>= 0.47.0) + rubocop (>= 0.50.0) sysexits (~> 1.1) hashdiff (0.3.6) highline (1.7.8) hike (1.2.3) hitimes (1.2.6) + httparty (0.16.0) + multi_xml (>= 0.5.2) i18n (0.8.6) ice_cube (0.14.0) ice_nine (0.11.2) @@ -171,18 +178,34 @@ GEM more_core_extensions (2.0.0) activesupport (> 3.2) multi_json (1.12.2) + multi_xml (0.6.0) multipart-post (2.0.0) net-http-persistent (2.9.4) net-http-pipeline (1.0.1) nokogiri (1.8.1) mini_portile2 (~> 2.3.0) - octokit (4.6.2) + octokit (4.7.0) sawyer (~> 0.8.0, >= 0.5.3) parallel (1.12.1) - parser (2.4.0.2) - ast (~> 2.3) + parser (2.5.0.0) + ast (~> 2.4.0) pg (0.21.0) powerpack (0.1.1) + pronto (0.9.5) + gitlab (~> 4.0, >= 4.0.0) + httparty (>= 0.13.7) + octokit (~> 4.7, >= 4.7.0) + rainbow (~> 2.1) + rugged (~> 0.24, >= 0.23.0) + thor (~> 0.19.0) + pronto-haml (0.9.0) + haml_lint (~> 0.23) + pronto (~> 0.9.0) + pronto-rubocop (0.9.0) + pronto (~> 0.9.0) + rubocop (~> 0.38, >= 0.35.0) + pronto-yamllint (0.1.0) + pronto (~> 0.9.0) pry (0.9.12.6) coderay (~> 1.0) method_source (~> 0.8) @@ -219,8 +242,9 @@ GEM activesupport (= 4.2.10) rake (>= 0.8.7) thor (>= 0.18.1, < 2.0) - rainbow (3.0.0) - rake (12.1.0) + rainbow (2.2.2) + rake + rake (12.3.0) rb-fsevent (0.10.2) rb-inotify (0.9.10) ffi (>= 0.5.0, < 2) @@ -251,7 +275,7 @@ GEM rspec-mocks (~> 3.6.0) rspec-support (~> 3.6.0) rspec-support (3.6.0) - rubocop (0.52.0) + rubocop (0.52.1) parallel (~> 1.10) parser (>= 2.4.0.2, < 3.0) powerpack (~> 0.1) @@ -299,6 +323,8 @@ GEM sprockets (>= 2.8, < 4.0) sysexits (1.2.0) temple (0.8.0) + terminal-table (1.8.0) + unicode-display_width (~> 1.1, >= 1.1.1) therubyracer (0.12.3) libv8 (~> 3.16.14.15) ref @@ -306,7 +332,7 @@ GEM daemons (~> 1.0, >= 1.0.9) eventmachine (~> 1.0, >= 1.0.4) rack (>= 1, < 3) - thor (0.20.0) + thor (0.19.4) thread_safe (0.3.6) tilt (1.4.1) timecop (0.9.1) @@ -368,14 +394,18 @@ DEPENDENCIES faraday (~> 0.9.1) faraday-http-cache (~> 2.0.0) foreman (~> 0.64.0) - haml_lint (~> 0.20.0) + haml_lint (~> 0.27.0) influxdb (~> 0.3.13) jquery-rails listen minigit (~> 0.0.4) more_core_extensions (~> 2.0.0) - octokit (~> 4.6.0) + octokit (~> 4.7.0) pg + pronto (~> 0.9.5) + pronto-haml (~> 0.9.0) + pronto-rubocop + pronto-yamllint rails (~> 4.2.4) rspec rspec-rails @@ -396,4 +426,4 @@ DEPENDENCIES webmock BUNDLED WITH - 1.15.4 + 1.16.1 diff --git a/app/workers/concerns/code_analysis_mixin.rb b/app/workers/concerns/code_analysis_mixin.rb index 9ccd9ef2..dbb6f7a9 100644 --- a/app/workers/concerns/code_analysis_mixin.rb +++ b/app/workers/concerns/code_analysis_mixin.rb @@ -1,3 +1,15 @@ +require 'pronto/runners' +require 'pronto/rubocop' +require 'pronto/yamllint' +require 'pronto/haml' +require 'pronto/git/repository' +require 'pronto/git/patches' +require 'pronto/git/patch' +require 'pronto/git/line' + +require 'fileutils' +require 'tmpdir' + module CodeAnalysisMixin def merged_linter_results results = { @@ -19,11 +31,64 @@ def merged_linter_results results end + # run linters via pronto and return the pronto result + def pronto_result + p_result = nil + + # temporary solution for: download repo, obtain changes, get pronto result about changes + Dir.mktmpdir do |dir| + FileUtils.copy_entry(@branch.repo.path.to_s, dir) + repo = Pronto::Git::Repository.new(dir) + rg = repo.instance_variable_get(:@repo) + rg.fetch('origin', @branch.name.sub(/^prs/, 'pull')) + rg.checkout('FETCH_HEAD') + rg.reset('HEAD', :hard) + patches = repo.diff(@branch.merge_target) + p_result = Pronto::Runners.new.run(patches) + end + + p_result + end + def run_all_linters - unmerged_results = [] - unmerged_results << Linter::Rubocop.new(branch).run - unmerged_results << Linter::Haml.new(branch).run - unmerged_results << Linter::Yaml.new(branch).run - unmerged_results.tap(&:compact!) + pronto_result.group_by(&:runner).values.map do |linted| # group by linter + output = {} + if linted.first.runner.name == "Pronto::Rubocop" + output["metadata"] = { + "rubocop_version" => ::RuboCop::Version.version, + "ruby_engine" => RUBY_ENGINE, + "ruby_version" => RUBY_VERSION, + "ruby_patchlevel" => RUBY_PATCHLEVEL.to_s, + "ruby_platform" => RUBY_PLATFORM + } + end + + output["files"] = linted.group_by(&:path).map do |path, value| # group by file in linter + { + "path" => path, + "offenses" => value.map do |msg| # put offenses of file in linter into an array + { + "severity" => msg.level.to_s, + "message" => msg.msg, + "cop_name" => msg.runner, + "corrected" => false, + "location" => { + "line" => msg.line.position, + "column" => 0, # TODO value cannot be obtained from Pronto::Message + "length" => msg.line.length + } + } + end + } + end + + output["summary"] = { + "offense_count" => output["files"].reduce(0) { |sum, item| sum + item['offenses'].length }, + "target_file_count" => output["files"].length, + "inspected_file_count" => 0 # TODO value cannot be obtained from the result of `pronto_result` method + } + + output + end end end diff --git a/spec/workers/commit_monitor_handlers/commit_range/rubocop_checker/message_builder_spec.rb b/spec/workers/commit_monitor_handlers/commit_range/rubocop_checker/message_builder_spec.rb index 59eb3574..1d0b2a07 100644 --- a/spec/workers/commit_monitor_handlers/commit_range/rubocop_checker/message_builder_spec.rb +++ b/spec/workers/commit_monitor_handlers/commit_range/rubocop_checker/message_builder_spec.rb @@ -28,7 +28,7 @@ - [ ] :exclamation: - [Line 4](https://github.com/some_user/some_repo/blob/8942a195a0bfa69ceb82c020c60565408cb46d3e/spec/workers/commit_monitor_handlers/commit_range/rubocop_checker/data/#{rubocop_check_directory}/coding_convention.rb#L4), Col 5 - [Layout/AlignHash](http://rubydoc.info/gems/rubocop/#{rubocop_version}/RuboCop/Cop/Layout/AlignHash) - Align the elements of a hash literal if they span more than one line. **spec/workers/commit_monitor_handlers/commit_range/rubocop_checker/data/#{rubocop_check_directory}/ruby_syntax_error.rb** -- [ ] :bomb: :boom: :fire: :fire_engine: - [Line 3](https://github.com/some_user/some_repo/blob/8942a195a0bfa69ceb82c020c60565408cb46d3e/spec/workers/commit_monitor_handlers/commit_range/rubocop_checker/data/#{rubocop_check_directory}/ruby_syntax_error.rb#L3), Col 1 - [Lint/Syntax](http://rubydoc.info/gems/rubocop/0.52.0/RuboCop/Cop/Lint/Syntax) - unexpected token kEND +- [ ] :bomb: :boom: :fire: :fire_engine: - [Line 3](https://github.com/some_user/some_repo/blob/8942a195a0bfa69ceb82c020c60565408cb46d3e/spec/workers/commit_monitor_handlers/commit_range/rubocop_checker/data/#{rubocop_check_directory}/ruby_syntax_error.rb#L3), Col 1 - [Lint/Syntax](http://rubydoc.info/gems/rubocop/#{rubocop_version}/RuboCop/Cop/Lint/Syntax) - unexpected token kEND (Using Ruby 2.3 parser; configure using `TargetRubyVersion` parameter, under `AllCops`) **spec/workers/commit_monitor_handlers/commit_range/rubocop_checker/data/#{rubocop_check_directory}/ruby_warning.rb** diff --git a/spec/workers/concerns/code_analysis_mixin_spec.rb b/spec/workers/concerns/code_analysis_mixin_spec.rb index 1637f8eb..547484e2 100644 --- a/spec/workers/concerns/code_analysis_mixin_spec.rb +++ b/spec/workers/concerns/code_analysis_mixin_spec.rb @@ -1,5 +1,5 @@ describe CodeAnalysisMixin do - let(:example_rubocop_result) { {"metadata"=>{"rubocop_version"=>"0.47.1", "ruby_engine"=>"ruby", "ruby_version"=>"2.3.3", "ruby_patchlevel"=>"222", "ruby_platform"=>"x86_64-linux"}, "files"=>[{"path"=>"app/helpers/application_helper/button/mixins/discovery_mixin.rb", "offenses"=>[]}, {"path"=>"app/helpers/application_helper/button/button_new_discover.rb", "offenses"=>[{"severity"=>"warning", "message"=>"Method `ApplicationHelper::Button::ButtonNewDiscover#visible?` is defined at both /tmp/d20171201-9050-1m4n90/app/helpers/application_helper/button/button_new_discover.rb:5 and /tmp/d20171201-9050-1m4n90/app/helpers/application_helper/button/button_new_discover.rb:9.", "cop_name"=>"Lint/DuplicateMethods", "corrected"=>nil, "location"=>{"line"=>9, "column"=>3, "length"=>3}}]}], "summary"=>{"offense_count"=>1, "target_file_count"=>2, "inspected_file_count"=>2}} } + let(:example_rubocop_result) { {"metadata" => {"rubocop_version" => "0.47.1", "ruby_engine" => "ruby", "ruby_version" => "2.3.3", "ruby_patchlevel" => "222", "ruby_platform" => "x86_64-linux"}, "files" => [{"path" => "app/helpers/application_helper/button/mixins/discovery_mixin.rb", "offenses" => []}, {"path" => "app/helpers/application_helper/button/button_new_discover.rb", "offenses" => [{"severity" => "warning", "message" => "Method `ApplicationHelper::Button::ButtonNewDiscover#visible?` is defined at both /tmp/d20171201-9050-1m4n90/app/helpers/application_helper/button/button_new_discover.rb:5 and /tmp/d20171201-9050-1m4n90/app/helpers/application_helper/button/button_new_discover.rb:9.", "cop_name" => "Lint/DuplicateMethods", "corrected" => nil, "location" => {"line" => 9, "column" => 3, "length" => 3}}]}], "summary" => {"offense_count" => 1, "target_file_count" => 2, "inspected_file_count" => 2}} } let(:test_class) do Class.new do include CodeAnalysisMixin @@ -8,34 +8,187 @@ end subject { test_class.new } - context "#merged_linter_results" do + describe "#merged_linter_results" do it "should always return a hash with a 'files' and 'summary' key, even with no cops running" do - expect(Linter::Rubocop).to receive(:new).and_return(double(:run => nil)) - expect(Linter::Haml).to receive(:new).and_return(double(:run => nil)) - expect(Linter::Yaml).to receive(:new).and_return(double(:run => nil)) - - expect(subject.merged_linter_results).to eq("files"=>[], "summary"=>{"inspected_file_count"=>0, "offense_count"=>0, "target_file_count"=>0}) + allow(subject).to receive(:pronto_result).and_return([]) + expect(subject.merged_linter_results).to eq("files" => [], "summary" => {"inspected_file_count" => 0, "offense_count" => 0, "target_file_count" => 0}) end + end - it "merges together with one result" do - expect(Linter::Rubocop).to receive(:new).and_return(double(:run => example_rubocop_result)) - expect(Linter::Haml).to receive(:new).and_return(double(:run => nil)) - expect(Linter::Yaml).to receive(:new).and_return(double(:run => nil)) + describe "#run_all_linters" do + let(:item_rubocop) { dup } + let(:item_haml) { dup } + let(:item_yaml) { dup } + let(:item_msg) { dup } + let(:item_line) { dup } + before do + allow(subject).to receive(:pronto_result).and_return(input) - expect(subject.merged_linter_results).to eq( - "files" => example_rubocop_result["files"], - "summary" => {"inspected_file_count"=>2, "offense_count"=>1, "target_file_count"=>2} - ) - end + allow(item_rubocop).to receive(:runner).and_return("Rubocop copname-text") + allow(item_haml).to receive(:runner).and_return("Haml copname-text") + allow(item_yaml).to receive(:runner).and_return("Yaml copname-text") + + allow(item_rubocop.runner).to receive(:name).and_return("Pronto::Rubocop") + allow(item_haml.runner).to receive(:name).and_return("Pronto::Haml") + allow(item_yaml.runner).to receive(:name).and_return("Pronto::Yaml") - it "merges together with one result" do - expect(Linter::Rubocop).to receive(:new).and_return(double(:run => example_rubocop_result)) - expect(Linter::Haml).to receive(:new).and_return(double(:run => example_rubocop_result)) - expect(Linter::Yaml).to receive(:new).and_return(double(:run => nil)) + allow(item_rubocop).to receive(:path).and_return("Rubocop filepath.rb") + allow(item_haml).to receive(:path).and_return("Haml filepath.rb") + allow(item_yaml).to receive(:path).and_return("Yaml filepath.rb") + + allow(item_rubocop).to receive(:level).and_return("Rubocop warning-text") + allow(item_haml).to receive(:level).and_return("Haml warning-text") + allow(item_yaml).to receive(:level).and_return("Yaml warning-text") + + allow(item_rubocop).to receive(:msg).and_return("Rubocop message-text") + allow(item_haml).to receive(:msg).and_return("Haml message-text") + allow(item_yaml).to receive(:msg).and_return("Yaml message-text") + + allow(item_msg).to receive(:line).and_return(line) + allow(item_line).to receive(:position).and_return(4) + end - results = subject.merged_linter_results - expect(results["files"]).to include(*example_rubocop_result["files"]) - expect(results["summary"]).to eq("inspected_file_count"=>4, "offense_count"=>2, "target_file_count"=>4) + context "input is array of pronto messages" do + let(:input) { [item_rubocop, item_rubocop, item_haml, item_haml, item_yaml, item_yaml] } + let(:message) { item_msg } + let(:line) { item_line } + it "returns empty array" do + expect(subject.run_all_linters).to eq( + [ + { + "metadata" => + { + "rubocop_version" => ::RuboCop::Version.version, + "ruby_engine" => RUBY_ENGINE, + "ruby_version" => RUBY_VERSION, + "ruby_patchlevel" => RUBY_PATCHLEVEL.to_s, + "ruby_platform" => RUBY_PLATFORM + }, + "files" => + [ + { + "path" => item_rubocop.path, + "offenses" => + [ + { + "severity" => item_rubocop.level, + "message" => item_rubocop.msg, + "cop_name" => item_rubocop.runner, + "corrected" => false, + "location" => + { + "line" => item_msg.line.position, + "column" => 0, + "length" => 0 + } + }, + { + "severity" => item_rubocop.level, + "message" => item_rubocop.msg, + "cop_name" => item_rubocop.runner, + "corrected" => false, + "location" => + { + "line" => item_msg.line.position, + "column" => 0, + "length" => 0 + } + } + ] + } + ], + "summary" => + { + "offense_count" => input.group_by(&:runner).values[0].count, + "target_file_count" => input.group_by(&:runner).values[0].group_by(&:path).count, + "inspected_file_count" => 0 + } + }, + { + "files" => + [ + { + "path" => item_haml.path, + "offenses" => + [ + { + "severity" => item_haml.level, + "message" => item_haml.msg, + "cop_name" => item_haml.runner, + "corrected" => false, + "location" => + { + "line" => item_msg.line.position, + "column" => 0, + "length" => 0 + } + }, + { + "severity" => item_haml.level, + "message" => item_haml.msg, + "cop_name" => item_haml.runner, + "corrected" => false, + "location" => + { + "line" => item_msg.line.position, + "column" => 0, + "length" => 0 + } + } + ] + } + ], + "summary" => + { + "offense_count" => input.group_by(&:runner).values[1].count, + "target_file_count" => input.group_by(&:runner).values[1].group_by(&:path).count, + "inspected_file_count" => 0 + } + }, + { + "files" => + [ + { + "path" => item_yaml.path, + "offenses" => + [ + { + "severity" => item_yaml.level, + "message" => item_yaml.msg, + "cop_name" => item_yaml.runner, + "corrected" => false, + "location" => + { + "line" => item_msg.line.position, + "column" => 0, + "length" => 0 + } + }, + { + "severity" => item_yaml.level, + "message" => item_yaml.msg, + "cop_name" => item_yaml.runner, + "corrected" => false, + "location" => + { + "line" => item_msg.line.position, + "column" => 0, + "length" => 0 + } + } + ] + } + ], + "summary" => + { + "offense_count" => input.group_by(&:runner).values[2].count, + "target_file_count" => input.group_by(&:runner).values[2].group_by(&:path).count, + "inspected_file_count" => 0 + } + } + ] + ) # expect END + end end end end