From 336635f283eab06c561134f2147a3bd1983090e1 Mon Sep 17 00:00:00 2001 From: "Z.J. van de Weg" Date: Mon, 22 May 2017 20:22:50 +0200 Subject: [PATCH] Test the ExpireJobCacheWorker and related changes These were untested by the cherry picked commit. --- app/models/commit_status.rb | 2 +- app/workers/expire_job_cache_worker.rb | 14 ++++----- spec/models/commit_status_spec.rb | 10 ++++++ spec/workers/expire_job_cache_worker_spec.rb | 31 +++++++++++++++++++ .../expire_pipeline_cache_worker_spec.rb | 2 ++ 5 files changed, 50 insertions(+), 9 deletions(-) create mode 100644 spec/workers/expire_job_cache_worker_spec.rb diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index 957f707a733..fe63728ea23 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -89,7 +89,7 @@ class CommitStatus < ActiveRecord::Base else PipelineUpdateWorker.perform_async(pipeline.id) end - ExpireJobCacheWorker.perform_async(pipeline.id, commit_status.id) + ExpireJobCacheWorker.perform_async(commit_status.id) end end end diff --git a/app/workers/expire_job_cache_worker.rb b/app/workers/expire_job_cache_worker.rb index e3930ee9d41..08e281e7350 100644 --- a/app/workers/expire_job_cache_worker.rb +++ b/app/workers/expire_job_cache_worker.rb @@ -2,15 +2,17 @@ class ExpireJobCacheWorker include Sidekiq::Worker include BuildQueue - def perform(pipeline_id, job_id) - job = CommitStatus.joins(:pipeline, :project).find_by(id: job) + def perform(job_id) + job = CommitStatus.joins(:pipeline, :project).find_by(id: job_id) return unless job pipeline = job.pipeline project = job.project - store.touch(project_pipeline_path(project, pipeline)) - store.touch(project_job_path(project, job)) + Gitlab::EtagCaching::Store.new.tap do |store| + store.touch(project_pipeline_path(project, pipeline)) + store.touch(project_job_path(project, job)) + end end private @@ -30,8 +32,4 @@ class ExpireJobCacheWorker job.id, format: :json) end - - def store - @store ||= Gitlab::EtagCaching::Store.new - end end diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index 6947affcc1e..c50b8bf7b13 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -36,6 +36,16 @@ describe CommitStatus, :models do it { is_expected.to eq(commit_status.user) } end + describe 'status state machine' do + let!(:commit_status) { create(:commit_status, :running, project: project) } + + it 'invalidates the cache after a transition' do + expect(ExpireJobCacheWorker).to receive(:perform_async).with(commit_status.id) + + commit_status.success! + end + end + describe '#started?' do subject { commit_status.started? } diff --git a/spec/workers/expire_job_cache_worker_spec.rb b/spec/workers/expire_job_cache_worker_spec.rb new file mode 100644 index 00000000000..1b614342a18 --- /dev/null +++ b/spec/workers/expire_job_cache_worker_spec.rb @@ -0,0 +1,31 @@ +require 'spec_helper' + +describe ExpireJobCacheWorker do + set(:pipeline) { create(:ci_empty_pipeline) } + let(:project) { pipeline.project } + subject { described_class.new } + + describe '#perform' do + context 'with a job in the pipeline' do + let(:job) { create(:ci_build, pipeline: pipeline) } + + it 'invalidates Etag caching for the job path' do + pipeline_path = "/#{project.full_path}/pipelines/#{pipeline.id}.json" + job_path = "/#{project.full_path}/builds/#{job.id}.json" + + expect_any_instance_of(Gitlab::EtagCaching::Store).to receive(:touch).with(pipeline_path) + expect_any_instance_of(Gitlab::EtagCaching::Store).to receive(:touch).with(job_path) + + subject.perform(job.id) + end + end + + context 'when there is no job in the pipeline' do + it 'does not change the etag store' do + expect(Gitlab::EtagCaching::Store).not_to receive(:new) + + subject.perform(9999) + end + end + end +end diff --git a/spec/workers/expire_pipeline_cache_worker_spec.rb b/spec/workers/expire_pipeline_cache_worker_spec.rb index ceba604dea2..28e5b706803 100644 --- a/spec/workers/expire_pipeline_cache_worker_spec.rb +++ b/spec/workers/expire_pipeline_cache_worker_spec.rb @@ -10,9 +10,11 @@ describe ExpirePipelineCacheWorker do it 'invalidates Etag caching for project pipelines path' do pipelines_path = "/#{project.full_path}/pipelines.json" new_mr_pipelines_path = "/#{project.full_path}/merge_requests/new.json" + pipeline_path = "/#{project.full_path}/pipelines/#{pipeline.id}.json" expect_any_instance_of(Gitlab::EtagCaching::Store).to receive(:touch).with(pipelines_path) expect_any_instance_of(Gitlab::EtagCaching::Store).to receive(:touch).with(new_mr_pipelines_path) + expect_any_instance_of(Gitlab::EtagCaching::Store).to receive(:touch).with(pipeline_path) subject.perform(pipeline.id) end