From 30a4bb66281d1d83028b3258156957db23077104 Mon Sep 17 00:00:00 2001
From: Pawel Chojnacki <pawel@chojnacki.ws>
Date: Thu, 19 Oct 2017 12:24:15 +0300
Subject: [PATCH] Fix sidekiq middleware tests

---
 lib/gitlab/metrics/sidekiq_transaction.rb     |  5 ++--
 .../{base_transaction.rb => transaction.rb}   |  4 +--
 lib/gitlab/metrics/web_transaction.rb         |  2 +-
 .../gitlab/metrics/sidekiq_middleware_spec.rb | 28 ++++++-------------
 .../metrics/subscribers/rails_cache_spec.rb   |  2 +-
 5 files changed, 14 insertions(+), 27 deletions(-)
 rename lib/gitlab/metrics/{base_transaction.rb => transaction.rb} (99%)

diff --git a/lib/gitlab/metrics/sidekiq_transaction.rb b/lib/gitlab/metrics/sidekiq_transaction.rb
index 4024b892ecc..797594f12e1 100644
--- a/lib/gitlab/metrics/sidekiq_transaction.rb
+++ b/lib/gitlab/metrics/sidekiq_transaction.rb
@@ -1,14 +1,15 @@
 module Gitlab
   module Metrics
-    class SidekiqTransaction
+    class SidekiqTransaction < Transaction
       def initialize(worker_class)
+        super()
         @worker_class = worker_class
       end
 
       protected
 
       def labels
-        { controller: worker.class.name, action: 'perform' }
+        { controller: @worker_class.name, action: 'perform' }
       end
     end
   end
diff --git a/lib/gitlab/metrics/base_transaction.rb b/lib/gitlab/metrics/transaction.rb
similarity index 99%
rename from lib/gitlab/metrics/base_transaction.rb
rename to lib/gitlab/metrics/transaction.rb
index cf616cb13a3..372922a986e 100644
--- a/lib/gitlab/metrics/base_transaction.rb
+++ b/lib/gitlab/metrics/transaction.rb
@@ -1,7 +1,7 @@
 module Gitlab
   module Metrics
     # Class for storing metrics information of a single transaction.
-    class BaseTransaction
+    class Transaction
       # base labels shared among all transactions
       BASE_LABELS = { controller: nil, action: nil }.freeze
 
@@ -131,8 +131,6 @@ module Gitlab
         "#{labels[:controller]}##{labels[:action]}" if labels && !labels.empty?
       end
 
-      protected
-
       def self.metric_transaction_duration_seconds
         @metric_transaction_duration_seconds ||= Gitlab::Metrics.histogram(
           :gitlab_transaction_duration_seconds,
diff --git a/lib/gitlab/metrics/web_transaction.rb b/lib/gitlab/metrics/web_transaction.rb
index f85929b1c4e..89ff02a96d6 100644
--- a/lib/gitlab/metrics/web_transaction.rb
+++ b/lib/gitlab/metrics/web_transaction.rb
@@ -1,6 +1,6 @@
 module Gitlab
   module Metrics
-    class WebTransaction < BaseTransaction
+    class WebTransaction < Transaction
       CONTROLLER_KEY = 'action_controller.instance'.freeze
       ENDPOINT_KEY = 'api.endpoint'.freeze
 
diff --git a/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb b/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb
index 0803ce42fac..4fa5b64abbf 100644
--- a/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb
+++ b/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb
@@ -6,7 +6,7 @@ describe Gitlab::Metrics::SidekiqMiddleware do
 
   def run(worker, message)
     expect(Gitlab::Metrics::Transaction).to receive(:new)
-      .with('TestWorker#perform')
+      .with(worker.class)
       .and_call_original
 
     expect_any_instance_of(Gitlab::Metrics::Transaction).to receive(:set)
@@ -18,21 +18,22 @@ describe Gitlab::Metrics::SidekiqMiddleware do
   end
 
   describe '#call' do
-    it 'tracks the transaction' do
-      worker = double(:worker, class: double(:class, name: 'TestWorker'))
+    let(:test_worker_class) { double(:class, name: 'TestWorker') }
+    let(:worker) { double(:worker, class: test_worker_class) }
+
+    it 'reports correct action based on worker class' do
+
+    end
 
+    it 'tracks the transaction' do
       run(worker, message)
     end
 
     it 'tracks the transaction (for messages without `enqueued_at`)' do
-      worker = double(:worker, class: double(:class, name: 'TestWorker'))
-
       run(worker, {})
     end
 
     it 'tracks any raised exceptions' do
-      worker = double(:worker, class: double(:class, name: 'TestWorker'))
-
       expect_any_instance_of(Gitlab::Metrics::Transaction)
         .to receive(:run).and_raise(RuntimeError)
 
@@ -45,18 +46,5 @@ describe Gitlab::Metrics::SidekiqMiddleware do
       expect { middleware.call(worker, message, :test) }
         .to raise_error(RuntimeError)
     end
-
-    it 'tags the metrics accordingly' do
-      tags = { one: 1, two: 2 }
-      worker = double(:worker, class: double(:class, name: 'TestWorker'))
-      allow(worker).to receive(:metrics_tags).and_return(tags)
-
-      tags.each do |tag, value|
-        expect_any_instance_of(Gitlab::Metrics::Transaction).to receive(:add_tag)
-          .with(tag, value)
-      end
-
-      run(worker, message)
-    end
   end
 end
diff --git a/spec/lib/gitlab/metrics/subscribers/rails_cache_spec.rb b/spec/lib/gitlab/metrics/subscribers/rails_cache_spec.rb
index 453a93d90ac..4203be40332 100644
--- a/spec/lib/gitlab/metrics/subscribers/rails_cache_spec.rb
+++ b/spec/lib/gitlab/metrics/subscribers/rails_cache_spec.rb
@@ -2,7 +2,7 @@ require 'spec_helper'
 
 describe Gitlab::Metrics::Subscribers::RailsCache do
   let(:env) { {} }
-  let(:transaction) { Gitlab::Metrics::Transaction.new(env) }
+  let(:transaction) { Gitlab::Metrics::WebTransaction.new(env) }
   let(:subscriber) { described_class.new }
 
   let(:event) { double(:event, duration: 15.2) }
-- 
GitLab