Merge pull request #8 from bigcommerce/rubocop-1

splittingred · web-flow · commit f283ce2c77b7 · 2020-10-26T12:55:09.000-05:00
Update to rubocop 1.0
diff --git a/.rubocop.yml b/.rubocop.yml
@@ -8,15 +8,13 @@ AllCops:
     - tmp/**/*
     - log/**/*
     - Rakefile
-    - bc-prometheus-ruby.gemspec
-    - lib/bigcommerce/prometheus/configuration.rb
 
 # Allow *VALID_CONFIG_KEYS.keys
 Lint/AmbiguousOperator:
   Enabled: false
 
 # This cop conflicts with other cops
-Metrics/LineLength:
+Layout/LineLength:
   Enabled: false
 
 Metrics/MethodLength:
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,8 @@ Changelog for the bc-prometheus-ruby gem.
 
 ### Pending Release
 
+- Update rubocop to 1.0
+
 ### 0.3.1
 
 - Update prometheus_exporter dependency to ~> 0.5 to fix memory leaks
diff --git a/bc-prometheus-ruby.gemspec b/bc-prometheus-ruby.gemspec
@@ -15,7 +15,7 @@
 # COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
 # OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 #
-$:.push File.expand_path('../lib', __FILE__)
+$LOAD_PATH.push File.expand_path('lib', __dir__)
 require 'bigcommerce/prometheus/version'
 
 Gem::Specification.new do |spec|
@@ -33,13 +33,13 @@ Gem::Specification.new do |spec|
   spec.require_paths = ['lib']
   spec.required_ruby_version = '>= 2.6'
 
-  spec.add_development_dependency 'rake', '>= 10.0'
-  spec.add_development_dependency 'rspec', '>= 3.8'
-  spec.add_development_dependency 'rspec_junit_formatter', '>= 0.4'
   spec.add_development_dependency 'bundler-audit', '>= 0.6'
   spec.add_development_dependency 'null-logger', '>= 0.1'
   spec.add_development_dependency 'pry', '>= 0.12'
-  spec.add_development_dependency 'rubocop', '>= 0.74'
+  spec.add_development_dependency 'rake', '>= 10.0'
+  spec.add_development_dependency 'rspec', '>= 3.8'
+  spec.add_development_dependency 'rspec_junit_formatter', '>= 0.4'
+  spec.add_development_dependency 'rubocop', '>= 1.0'
   spec.add_development_dependency 'simplecov', '>= 0.16'
 
   spec.add_runtime_dependency 'bigcommerce-multitrap', '~> 0.1'
diff --git a/lib/bigcommerce/prometheus.rb b/lib/bigcommerce/prometheus.rb
@@ -58,6 +58,9 @@ module Bigcommerce
   module Prometheus
     extend Configuration
 
+    ##
+    # @return [Bigcommerce::Prometheus::Client]
+    #
     def self.client
       Client.instance
     end
diff --git a/lib/bigcommerce/prometheus/client.rb b/lib/bigcommerce/prometheus/client.rb
@@ -24,6 +24,14 @@ class Client < ::PrometheusExporter::Client
       include Singleton
       include Loggable
 
+      ##
+      # @param [String] host
+      # @param [Integer] port
+      # @param [Integer] max_queue_size
+      # @param [Integer|Float] thread_sleep
+      # @param [Hash] custom_labels
+      # @param [String] process_name
+      #
       def initialize(host: nil, port: nil, max_queue_size: nil, thread_sleep: nil, custom_labels: nil, process_name: nil)
         super(
           host: host || Bigcommerce::Prometheus.server_host,
diff --git a/lib/bigcommerce/prometheus/configuration.rb b/lib/bigcommerce/prometheus/configuration.rb
@@ -76,6 +76,7 @@ def configure
         reset unless @configured
         yield self
         @configured = true
+        self
       end
 
       ##
@@ -118,8 +119,8 @@ def determine_logger
           self.logger = Application.logger
         else
           require 'logger'
-          self.logger = ::Logger.new(STDOUT)
-          self.logger.level = ::Logger::Severity::INFO
+          self.logger = ::Logger.new($stdout)
+          logger.level = ::Logger::Severity::INFO
         end
       end
 
diff --git a/lib/bigcommerce/prometheus/server.rb b/lib/bigcommerce/prometheus/server.rb
@@ -78,7 +78,7 @@ def stop
         @server.stop!
         @run_thread.kill
         @running = false
-        STDOUT.puts "[bigcommerce-prometheus][#{@process_name}] Prometheus exporter cleanly shut down"
+        $stdout.puts "[bigcommerce-prometheus][#{@process_name}] Prometheus exporter cleanly shut down"
       rescue ::StandardError => e
         warn "[bigcommerce-prometheus][#{@process_name}] Failed to stop exporter: #{e.message}"
       end
diff --git a/lib/bigcommerce/prometheus/type_collectors/base.rb b/lib/bigcommerce/prometheus/type_collectors/base.rb
@@ -29,6 +29,7 @@ class Base < PrometheusExporter::Server::TypeCollector
         # @param [Hash] default_labels
         #
         def initialize(type: nil, default_labels: {})
+          super()
           @type = type || self.class.to_s.downcase.gsub('::', '_').gsub('typecollector', '')
           @default_labels = default_labels || {}
           @metrics = build_metrics
diff --git a/lib/bigcommerce/prometheus/type_collectors/resque.rb b/lib/bigcommerce/prometheus/type_collectors/resque.rb
@@ -25,6 +25,8 @@ class Resque < Bigcommerce::Prometheus::TypeCollectors::Base
         ##
         # Initialize the collector
         #
+        # @return [Hash]
+        #
         def build_metrics
           {
             workers_total: PrometheusExporter::Metric::Gauge.new('resque_workers_total', 'Number of active workers'),
diff --git a/spec/bigcommerce/prometheus/client_spec.rb b/spec/bigcommerce/prometheus/client_spec.rb
@@ -20,14 +20,14 @@
 describe Bigcommerce::Prometheus::Client do
   let(:client) { described_class.instance }
 
-  describe '.initialize' do
+  describe '#initialize' do
     subject { client }
 
-    it 'should initialize the client object from the singleton' do
+    it 'initializes the client object from the singleton' do
       expect(subject).to be_a(described_class)
     end
 
-    it 'should behave like a singleton' do
+    it 'behaves like a singleton' do
       ref1 = described_class.instance
       ref2 = described_class.instance
       expect(ref1).to eq ref2
diff --git a/spec/bigcommerce/prometheus/collectors/base_spec.rb b/spec/bigcommerce/prometheus/collectors/base_spec.rb
@@ -22,10 +22,10 @@
   let(:client) { double(:client, send_json: true) }
   let(:collector) { AppCollector.new(client: client, frequency: 0) }
 
-  describe '.run' do
+  describe '#run' do
     subject { collector.run }
 
-    it 'should collect and push the metrics, then sleep the frequency' do
+    it 'collects and pushes the metrics, then sleeps the frequency' do
       expect(client).to receive(:send_json).with(
         type: 'app',
         points: 42
@@ -34,10 +34,10 @@
     end
   end
 
-  describe '.honk!' do
+  describe '#honk!' do
     subject { collector.honk! }
 
-    it 'should push the metric dynamically' do
+    it 'pushes the metric dynamically' do
       expect(client).to receive(:send_json).with(
         type: 'app',
         honks: 1,
diff --git a/spec/bigcommerce/prometheus/servers/thin/controllers/metrics_controller_spec.rb b/spec/bigcommerce/prometheus/servers/thin/controllers/metrics_controller_spec.rb
@@ -38,24 +38,24 @@
     allow(server_metrics).to receive(:to_prometheus_text).and_return(server_metrics_text)
   end
 
-  describe '.call' do
+  describe '#call' do
     subject { controller.call }
 
-    it 'should succeed' do
+    it 'succeeds' do
       expect(subject.status).to eq 200
       expect(subject.body.first).to eq 'collector_working 1
 collector_rss 60440
 ruby_collector_metrics_total 19
 ruby_collector_sessions_total 19'
     end
 
-    context 'if the metrics fail to parse' do
+    context 'when the metrics fail to parse' do
       let(:timeout_exception) { ::Timeout::Error }
       before do
         allow(collector).to receive(:prometheus_metrics_text).and_raise(timeout_exception)
       end
 
-      it 'should still respond 200 but log an error' do
+      it 'still responds 200, but logs an error' do
         expect(logger).to receive(:error).with('Generating Prometheus metrics text timed out')
         expect(subject.status).to eq 200
         expect(subject.body.first).to eq 'collector_working 1
diff --git a/spec/bigcommerce/prometheus/servers/thin/controllers/send_metrics_controller_spec.rb b/spec/bigcommerce/prometheus/servers/thin/controllers/send_metrics_controller_spec.rb
@@ -30,29 +30,29 @@
   let(:server_metrics) { Bigcommerce::Prometheus::Servers::Thin::ServerMetrics.new }
   let(:controller) { described_class.new(request: request, response: response, server_metrics: server_metrics, collector: collector, logger: logger) }
 
-  describe '.call' do
+  describe '#call' do
     subject { controller.call }
 
     context 'when a post request' do
-      context 'and the collector properly parses the metrics' do
+      context 'when the collector properly parses the metrics' do
         before do
           expect(collector).to receive(:process).once
         end
 
-        it 'should succeed' do
+        it 'succeeds' do
           expect(subject.status).to eq 200
           expect(subject.body.first).to eq 'OK'
         end
       end
 
-      context 'and the collector fails to parse the metrics' do
+      context 'when the collector fails to parse the metrics' do
         let(:error_message) { 'failure!' }
         let(:exception) { StandardError.new(error_message) }
         before do
           expect(collector).to receive(:process).once.and_raise(exception)
         end
 
-        it 'should fail' do
+        it 'fails' do
           expect(subject.status).to eq 500
           parsed_body = JSON.parse(subject.body.first)
           expect(parsed_body).to eq [error_message]
@@ -63,7 +63,7 @@
     context 'when not a post request' do
       let(:request_method) { 'GET' }
 
-      it 'should fail' do
+      it 'fails' do
         expect(subject.status).to eq 500
         parsed_body = JSON.parse(subject.body.first)
         expect(parsed_body).to eq ['Invalid request type. Only POST is supported.']
diff --git a/spec/bigcommerce/prometheus/type_collectors/resque_spec.rb b/spec/bigcommerce/prometheus/type_collectors/resque_spec.rb
@@ -20,10 +20,10 @@
 describe Bigcommerce::Prometheus::TypeCollectors::Resque do
   let(:type_collector) { described_class.new }
 
-  describe '.build_metrics' do
+  describe '#build_metrics' do
     subject { type_collector.build_metrics }
 
-    it 'should return a hash of prometheus metric objects' do
+    it 'returns a hash of prometheus metric objects' do
       expect(subject).to be_a(Hash)
       expect(subject.count).to eq 6
     end
@@ -61,7 +61,7 @@
       },
 
     }.each do |hash_key, config|
-      it "should build the #{hash_key} with stat key of #{config[:key]}" do
+      it "builds the #{hash_key} with stat key of #{config[:key]}" do
         metric = subject[hash_key]
         expect(metric).to be_a config[:class]
         expect(metric.name).to eq config[:name]
@@ -70,7 +70,9 @@
     end
   end
 
-  describe '.collect_metrics' do
+  describe '#collect_metrics' do
+    subject { type_collector.collect_metrics(data: data, labels: labels) }
+
     let(:labels) { { environment: 'development' } }
     let(:data) do
       {
@@ -87,9 +89,7 @@
       }
     end
 
-    subject { type_collector.collect_metrics(data: data, labels: labels) }
-
-    it 'should properly log metrics for all passed values' do
+    it 'properly logs metrics for all passed values' do
       metrics = type_collector.instance_variable_get(:@metrics)
 
       expect(metrics[:workers_total]).to receive(:observe).with(data['workers_total'], labels)
diff --git a/spec/bigcommerce/prometheus/version_spec.rb b/spec/bigcommerce/prometheus/version_spec.rb
@@ -17,7 +17,7 @@
 
 describe Bigcommerce::Prometheus do
   describe 'version' do
-    it 'should have a version' do
+    it 'has a version' do
       expect(Bigcommerce::Prometheus::VERSION).to be_a(String)
     end
   end
diff --git a/spec/simplecov_helper.rb b/spec/simplecov_helper.rb
@@ -22,7 +22,7 @@
 end
 
 SimpleCov.command_name 'Unit Tests'
-SimpleCov.minimum_coverage 20
+SimpleCov.minimum_coverage 30
 SimpleCov.start do
   add_filter '/bin/'
   add_filter '/coverage/'
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
@@ -14,17 +14,13 @@
 # OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 #
 ENV['RACK_ENV'] = 'test'
-$LOAD_PATH.unshift File.expand_path('../../lib', __FILE__)
+$LOAD_PATH.unshift File.expand_path('../lib', __dir__)
 require_relative 'simplecov_helper'
 require 'bigcommerce/prometheus'
 require 'pry'
-Dir["#{File.join(File.dirname(__FILE__), 'support')}/**/*.rb"].each {|f| require f }
+Dir["#{File.join(File.dirname(__FILE__), 'support')}/**/*.rb"].sort.each { |f| require f }
 
 RSpec.configure do |config|
-  config.alias_example_to :fit, focus: true
-  config.filter_run focus: true
-  config.filter_run_excluding broken: true
-  config.run_all_when_everything_filtered = true
   config.expose_current_running_example_as :example
   config.mock_with :rspec do |mocks|
     mocks.allow_message_expectations_on_nil = true

Original file line number	Diff line number	Diff line change
`@@ -29,6 +29,7 @@ class Base < PrometheusExporter::Server::TypeCollector`
`29`	`29`	`# @param [Hash] default_labels`
`30`	`30`	`#`
`31`	`31`	`def initialize(type: nil, default_labels: {})`
	`32`	`+ super()`
`32`	`33`	`@type = type \|\| self.class.to_s.downcase.gsub('::', '_').gsub('typecollector', '')`
`33`	`34`	`@default_labels = default_labels \|\| {}`
`34`	`35`	`@metrics = build_metrics`
Original file line number	Diff line number	Diff line change
`@@ -25,6 +25,8 @@ class Resque < Bigcommerce::Prometheus::TypeCollectors::Base`
`25`	`25`	`##`
`26`	`26`	`# Initialize the collector`
`27`	`27`	`#`
	`28`	`+ # @return [Hash]`
	`29`	`+ #`
`28`	`30`	`def build_metrics`
`29`	`31`	`{`
`30`	`32`	`workers_total: PrometheusExporter::Metric::Gauge.new('resque_workers_total', 'Number of active workers'),`