From db40a7c4e359052313b9a7bf104aa4e9586deada Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Fri, 1 Jun 2018 02:04:55 +0800 Subject: [PATCH 1/4] Preserve warnings even if it passed --- lib/tasks/lint.rake | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/tasks/lint.rake b/lib/tasks/lint.rake index fe5032cae18..8b86a5c72a5 100644 --- a/lib/tasks/lint.rake +++ b/lib/tasks/lint.rake @@ -30,11 +30,12 @@ unless Rails.env.production? lint:static_verification ].each do |task| pid = Process.fork do - rd, wr = IO.pipe + rd_out, wr_out = IO.pipe + rd_err, wr_err = IO.pipe stdout = $stdout.dup stderr = $stderr.dup - $stdout.reopen(wr) - $stderr.reopen(wr) + $stdout.reopen(wr_out) + $stderr.reopen(wr_err) begin begin @@ -48,14 +49,13 @@ unless Rails.env.production? ensure $stdout.reopen(stdout) $stderr.reopen(stderr) - wr.close + wr_out.close + wr_err.close - if msg - warn "\n#{msg}\n\n" - IO.copy_stream(rd, $stderr) - else - IO.copy_stream(rd, $stdout) - end + warn "\n#{msg}\n\n" if msg + + IO.copy_stream(rd_out, $stdout) + IO.copy_stream(rd_err, $stderr) end end From 39b6f31c66ff51451033ff84a2832731065cd28d Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Fri, 1 Jun 2018 02:43:47 +0800 Subject: [PATCH 2/4] Eliminate constants warnings by: * Replace `require` or `require_relative` with `require_dependency` * Remove unneeded `autoload` --- app/helpers/webpack_helper.rb | 2 -- config/initializers/2_gitlab.rb | 2 +- config/initializers/omniauth.rb | 9 ++++----- lib/gitlab/auth.rb | 4 ++++ lib/omni_auth/strategies/jwt.rb | 4 +--- lib/rspec_flaky/listener.rb | 10 +++++----- lib/rspec_flaky/report.rb | 4 ++-- scripts/prune-old-flaky-specs | 5 ++++- spec/lib/omni_auth/strategies/jwt_spec.rb | 6 +++--- 9 files changed, 24 insertions(+), 22 deletions(-) diff --git a/app/helpers/webpack_helper.rb b/app/helpers/webpack_helper.rb index e12e4ba70e9..72f6b397046 100644 --- a/app/helpers/webpack_helper.rb +++ b/app/helpers/webpack_helper.rb @@ -1,5 +1,3 @@ -require 'gitlab/webpack/manifest' - module WebpackHelper def webpack_bundle_tag(bundle) javascript_include_tag(*webpack_entrypoint_paths(bundle)) diff --git a/config/initializers/2_gitlab.rb b/config/initializers/2_gitlab.rb index 1d2ab606a63..8b7f245b7b0 100644 --- a/config/initializers/2_gitlab.rb +++ b/config/initializers/2_gitlab.rb @@ -1 +1 @@ -require_relative '../../lib/gitlab' +require_dependency 'gitlab' diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb index e33ebb25c4c..a93a43d88ee 100644 --- a/config/initializers/omniauth.rb +++ b/config/initializers/omniauth.rb @@ -20,11 +20,10 @@ end if Gitlab.config.omniauth.enabled provider_names = Gitlab.config.omniauth.providers.map(&:name) require 'omniauth-kerberos' if provider_names.include?('kerberos') -end -module OmniAuth - module Strategies - autoload :Bitbucket, Rails.root.join('lib', 'omni_auth', 'strategies', 'bitbucket') - autoload :Jwt, Rails.root.join('lib', 'omni_auth', 'strategies', 'jwt') + Gitlab::Auth.omniauth_providers.each do |provider| + if provider_names.include?(provider) + require_dependency "omni_auth/strategies/#{provider}" + end end end diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb index 8e5a985edd7..7047724cfe1 100644 --- a/lib/gitlab/auth.rb +++ b/lib/gitlab/auth.rb @@ -14,6 +14,10 @@ module Gitlab DEFAULT_SCOPES = [:api].freeze class << self + def omniauth_providers + %w[bitbucket jwt] + end + def find_for_git_client(login, password, project:, ip:) raise "Must provide an IP for rate limiting" if ip.nil? diff --git a/lib/omni_auth/strategies/jwt.rb b/lib/omni_auth/strategies/jwt.rb index 2349b2a28aa..ebdb5c7faf0 100644 --- a/lib/omni_auth/strategies/jwt.rb +++ b/lib/omni_auth/strategies/jwt.rb @@ -3,7 +3,7 @@ require 'jwt' module OmniAuth module Strategies - class JWT + class Jwt ClaimInvalid = Class.new(StandardError) include OmniAuth::Strategy @@ -56,7 +56,5 @@ module OmniAuth fail! :claim_invalid, e end end - - class Jwt < JWT; end end end diff --git a/lib/rspec_flaky/listener.rb b/lib/rspec_flaky/listener.rb index 5b5e4f7c7de..9cd0c38cb55 100644 --- a/lib/rspec_flaky/listener.rb +++ b/lib/rspec_flaky/listener.rb @@ -1,10 +1,10 @@ require 'json' -require_relative 'config' -require_relative 'example' -require_relative 'flaky_example' -require_relative 'flaky_examples_collection' -require_relative 'report' +require_dependency 'rspec_flaky/config' +require_dependency 'rspec_flaky/example' +require_dependency 'rspec_flaky/flaky_example' +require_dependency 'rspec_flaky/flaky_examples_collection' +require_dependency 'rspec_flaky/report' module RspecFlaky class Listener diff --git a/lib/rspec_flaky/report.rb b/lib/rspec_flaky/report.rb index a8730d3b7c7..1c362fdd20d 100644 --- a/lib/rspec_flaky/report.rb +++ b/lib/rspec_flaky/report.rb @@ -1,8 +1,8 @@ require 'json' require 'time' -require_relative 'config' -require_relative 'flaky_examples_collection' +require_dependency 'rspec_flaky/config' +require_dependency 'rspec_flaky/flaky_examples_collection' module RspecFlaky # This class is responsible for loading/saving JSON reports, and pruning diff --git a/scripts/prune-old-flaky-specs b/scripts/prune-old-flaky-specs index f7451fbd428..59f97e833b5 100755 --- a/scripts/prune-old-flaky-specs +++ b/scripts/prune-old-flaky-specs @@ -5,7 +5,10 @@ # gem manually on the CI require 'rubygems' -require_relative '../lib/rspec_flaky/report' +singleton_class.__send__(:alias_method, :require_dependency, :require) +$LOAD_PATH.unshift(File.expand_path('../lib', __dir__)) + +require 'rspec_flaky/report' report_file = ARGV.shift unless report_file diff --git a/spec/lib/omni_auth/strategies/jwt_spec.rb b/spec/lib/omni_auth/strategies/jwt_spec.rb index 23485fbcb18..88d6d0b559a 100644 --- a/spec/lib/omni_auth/strategies/jwt_spec.rb +++ b/spec/lib/omni_auth/strategies/jwt_spec.rb @@ -43,7 +43,7 @@ describe OmniAuth::Strategies::Jwt do end it 'raises error' do - expect { strategy.decoded }.to raise_error(OmniAuth::Strategies::JWT::ClaimInvalid) + expect { strategy.decoded }.to raise_error(OmniAuth::Strategies::Jwt::ClaimInvalid) end end @@ -61,7 +61,7 @@ describe OmniAuth::Strategies::Jwt do end it 'raises error' do - expect { strategy.decoded }.to raise_error(OmniAuth::Strategies::JWT::ClaimInvalid) + expect { strategy.decoded }.to raise_error(OmniAuth::Strategies::Jwt::ClaimInvalid) end end @@ -80,7 +80,7 @@ describe OmniAuth::Strategies::Jwt do end it 'raises error' do - expect { strategy.decoded }.to raise_error(OmniAuth::Strategies::JWT::ClaimInvalid) + expect { strategy.decoded }.to raise_error(OmniAuth::Strategies::Jwt::ClaimInvalid) end end end From 1e2b6cf514bcefd21520fef63b3fee5a29d334cd Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Fri, 1 Jun 2018 14:05:07 +0800 Subject: [PATCH 3/4] Introduce Gitlab::Auth.omniauth_setup_providers Which could extend from EE --- config/initializers/omniauth.rb | 8 +------- lib/gitlab/auth.rb | 19 +++++++++++++++++-- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb index a93a43d88ee..a7fa926a853 100644 --- a/config/initializers/omniauth.rb +++ b/config/initializers/omniauth.rb @@ -19,11 +19,5 @@ end if Gitlab.config.omniauth.enabled provider_names = Gitlab.config.omniauth.providers.map(&:name) - require 'omniauth-kerberos' if provider_names.include?('kerberos') - - Gitlab::Auth.omniauth_providers.each do |provider| - if provider_names.include?(provider) - require_dependency "omni_auth/strategies/#{provider}" - end - end + Gitlab::Auth.omniauth_setup_providers(provider_names) end diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb index 7047724cfe1..0f7a7b0ce8d 100644 --- a/lib/gitlab/auth.rb +++ b/lib/gitlab/auth.rb @@ -14,8 +14,23 @@ module Gitlab DEFAULT_SCOPES = [:api].freeze class << self - def omniauth_providers - %w[bitbucket jwt] + def omniauth_customized_providers + @omniauth_customized_providers ||= %w[bitbucket jwt] + end + + def omniauth_setup_providers(provider_names) + provider_names.each do |provider| + omniauth_setup_a_provider(provider) + end + end + + def omniauth_setup_a_provider(provider) + case provider + when 'kerberos' + require 'omniauth-kerberos' + when *omniauth_customized_providers + require_dependency "omni_auth/strategies/#{provider}" + end end def find_for_git_client(login, password, project:, ip:) From 7083b355a693e2de91aa7bcd7099c1a1690bc756 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Fri, 1 Jun 2018 14:13:47 +0800 Subject: [PATCH 4/4] Follow Rubocop for scripts/prune-old-flaky-specs --- scripts/prune-old-flaky-specs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/scripts/prune-old-flaky-specs b/scripts/prune-old-flaky-specs index 59f97e833b5..a00a334fd6e 100755 --- a/scripts/prune-old-flaky-specs +++ b/scripts/prune-old-flaky-specs @@ -5,8 +5,9 @@ # gem manually on the CI require 'rubygems' -singleton_class.__send__(:alias_method, :require_dependency, :require) -$LOAD_PATH.unshift(File.expand_path('../lib', __dir__)) +# In newer Ruby, alias_method is not private then we don't need __send__ +singleton_class.__send__(:alias_method, :require_dependency, :require) # rubocop:disable GitlabSecurity/PublicSend +$:.unshift(File.expand_path('../lib', __dir__)) require 'rspec_flaky/report'