summaryrefslogtreecommitdiff
path: root/spec
diff options
context:
space:
mode:
authorKamil Trzciński <ayufan@ayufan.eu>2017-09-05 09:49:55 +0000
committerKamil Trzciński <ayufan@ayufan.eu>2017-09-05 09:49:55 +0000
commit3d61421fb2ed22d64a6b20701d600a38db1458f5 (patch)
tree09865dc5eb46f29b7ee43d3c6918c814caedbec1 /spec
parent89efaf2aa2d65ae41ce5502059d40fa068be945f (diff)
parent48f017d1e84498eec38d276d94918021a985bfee (diff)
downloadgitlab-ce-3d61421fb2ed22d64a6b20701d600a38db1458f5.tar.gz
Merge branch 'fix/sm/35650-remove-createtriggerrequestservice-and-forbid-to-persist-variables-on-ci-triggerrequest' into 'master'
Removes `CreateTriggerRequestService` and add a blocker to prevent saving variables on `Ci::TriggerRequest` Closes #35650 See merge request !13792
Diffstat (limited to 'spec')
-rw-r--r--spec/factories/ci/builds.rb2
-rw-r--r--spec/factories/ci/pipeline_variables.rb (renamed from spec/factories/ci/pipeline_variable_variables.rb)0
-rw-r--r--spec/factories/ci/trigger_requests.rb9
-rw-r--r--spec/features/projects/jobs_spec.rb40
-rw-r--r--spec/models/ci/build_spec.rb28
-rw-r--r--spec/models/ci/trigger_request_spec.rb17
-rw-r--r--spec/presenters/ci/build_presenter_spec.rb34
-rw-r--r--spec/requests/api/runner_spec.rb31
-rw-r--r--spec/requests/api/v3/triggers_spec.rb5
-rw-r--r--spec/services/ci/create_trigger_request_service_spec.rb52
-rw-r--r--spec/views/projects/jobs/show.html.haml_spec.rb16
11 files changed, 133 insertions, 101 deletions
diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb
index 25ec63de94a..c2b59239af9 100644
--- a/spec/factories/ci/builds.rb
+++ b/spec/factories/ci/builds.rb
@@ -107,7 +107,7 @@ FactoryGirl.define do
end
trait :triggered do
- trigger_request factory: :ci_trigger_request_with_variables
+ trigger_request factory: :ci_trigger_request
end
after(:build) do |build, evaluator|
diff --git a/spec/factories/ci/pipeline_variable_variables.rb b/spec/factories/ci/pipeline_variables.rb
index 7c1a7faec08..7c1a7faec08 100644
--- a/spec/factories/ci/pipeline_variable_variables.rb
+++ b/spec/factories/ci/pipeline_variables.rb
diff --git a/spec/factories/ci/trigger_requests.rb b/spec/factories/ci/trigger_requests.rb
index 10e0ab4fd3c..40b8848920e 100644
--- a/spec/factories/ci/trigger_requests.rb
+++ b/spec/factories/ci/trigger_requests.rb
@@ -1,14 +1,5 @@
FactoryGirl.define do
factory :ci_trigger_request, class: Ci::TriggerRequest do
trigger factory: :ci_trigger
-
- factory :ci_trigger_request_with_variables do
- variables do
- {
- TRIGGER_KEY_1: 'TRIGGER_VALUE_1',
- TRIGGER_KEY_2: 'TRIGGER_VALUE_2'
- }
- end
- end
end
end
diff --git a/spec/features/projects/jobs_spec.rb b/spec/features/projects/jobs_spec.rb
index 037ac00d39f..3b5c6966287 100644
--- a/spec/features/projects/jobs_spec.rb
+++ b/spec/features/projects/jobs_spec.rb
@@ -292,26 +292,44 @@ feature 'Jobs' do
end
feature 'Variables' do
- let(:trigger_request) { create(:ci_trigger_request_with_variables) }
+ let(:trigger_request) { create(:ci_trigger_request) }
let(:job) do
create :ci_build, pipeline: pipeline, trigger_request: trigger_request
end
- before do
- visit project_job_path(project, job)
+ shared_examples 'expected variables behavior' do
+ it 'shows variable key and value after click', js: true do
+ expect(page).to have_css('.reveal-variables')
+ expect(page).not_to have_css('.js-build-variable')
+ expect(page).not_to have_css('.js-build-value')
+
+ click_button 'Reveal Variables'
+
+ expect(page).not_to have_css('.reveal-variables')
+ expect(page).to have_selector('.js-build-variable', text: 'TRIGGER_KEY_1')
+ expect(page).to have_selector('.js-build-value', text: 'TRIGGER_VALUE_1')
+ end
end
- it 'shows variable key and value after click', js: true do
- expect(page).to have_css('.reveal-variables')
- expect(page).not_to have_css('.js-build-variable')
- expect(page).not_to have_css('.js-build-value')
+ context 'when variables are stored in trigger_request' do
+ before do
+ trigger_request.update_attribute(:variables, { 'TRIGGER_KEY_1' => 'TRIGGER_VALUE_1' } )
- click_button 'Reveal Variables'
+ visit project_job_path(project, job)
+ end
+
+ it_behaves_like 'expected variables behavior'
+ end
+
+ context 'when variables are stored in pipeline_variables' do
+ before do
+ create(:ci_pipeline_variable, pipeline: pipeline, key: 'TRIGGER_KEY_1', value: 'TRIGGER_VALUE_1')
+
+ visit project_job_path(project, job)
+ end
- expect(page).not_to have_css('.reveal-variables')
- expect(page).to have_selector('.js-build-variable', text: 'TRIGGER_KEY_1')
- expect(page).to have_selector('.js-build-value', text: 'TRIGGER_VALUE_1')
+ it_behaves_like 'expected variables behavior'
end
end
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 3fe3ec17d36..08d22f166e4 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -1492,10 +1492,12 @@ describe Ci::Build do
context 'when build is for triggers' do
let(:trigger) { create(:ci_trigger, project: project) }
- let(:trigger_request) { create(:ci_trigger_request_with_variables, pipeline: pipeline, trigger: trigger) }
+ let(:trigger_request) { create(:ci_trigger_request, pipeline: pipeline, trigger: trigger) }
+
let(:user_trigger_variable) do
- { key: :TRIGGER_KEY_1, value: 'TRIGGER_VALUE_1', public: false }
+ { key: 'TRIGGER_KEY_1', value: 'TRIGGER_VALUE_1', public: false }
end
+
let(:predefined_trigger_variable) do
{ key: 'CI_PIPELINE_TRIGGERED', value: 'true', public: true }
end
@@ -1504,8 +1506,26 @@ describe Ci::Build do
build.trigger_request = trigger_request
end
- it { is_expected.to include(user_trigger_variable) }
- it { is_expected.to include(predefined_trigger_variable) }
+ shared_examples 'returns variables for triggers' do
+ it { is_expected.to include(user_trigger_variable) }
+ it { is_expected.to include(predefined_trigger_variable) }
+ end
+
+ context 'when variables are stored in trigger_request' do
+ before do
+ trigger_request.update_attribute(:variables, { 'TRIGGER_KEY_1' => 'TRIGGER_VALUE_1' } )
+ end
+
+ it_behaves_like 'returns variables for triggers'
+ end
+
+ context 'when variables are stored in pipeline_variables' do
+ before do
+ create(:ci_pipeline_variable, pipeline: pipeline, key: 'TRIGGER_KEY_1', value: 'TRIGGER_VALUE_1')
+ end
+
+ it_behaves_like 'returns variables for triggers'
+ end
end
context 'when pipeline has a variable' do
diff --git a/spec/models/ci/trigger_request_spec.rb b/spec/models/ci/trigger_request_spec.rb
new file mode 100644
index 00000000000..7dcf3528f73
--- /dev/null
+++ b/spec/models/ci/trigger_request_spec.rb
@@ -0,0 +1,17 @@
+require 'spec_helper'
+
+describe Ci::TriggerRequest do
+ describe 'validation' do
+ it 'be invalid if saving a variable' do
+ trigger = build(:ci_trigger_request, variables: { TRIGGER_KEY_1: 'TRIGGER_VALUE_1' } )
+
+ expect(trigger).not_to be_valid
+ end
+
+ it 'be valid if not saving a variable' do
+ trigger = build(:ci_trigger_request)
+
+ expect(trigger).to be_valid
+ end
+ end
+end
diff --git a/spec/presenters/ci/build_presenter_spec.rb b/spec/presenters/ci/build_presenter_spec.rb
index a7a34ecac72..1a8001be6ab 100644
--- a/spec/presenters/ci/build_presenter_spec.rb
+++ b/spec/presenters/ci/build_presenter_spec.rb
@@ -100,4 +100,38 @@ describe Ci::BuildPresenter do
end
end
end
+
+ describe '#trigger_variables' do
+ let(:build) { create(:ci_build, pipeline: pipeline, trigger_request: trigger_request) }
+ let(:trigger) { create(:ci_trigger, project: project) }
+ let(:trigger_request) { create(:ci_trigger_request, pipeline: pipeline, trigger: trigger) }
+
+ context 'when variable is stored in ci_pipeline_variables' do
+ let!(:pipeline_variable) { create(:ci_pipeline_variable, pipeline: pipeline) }
+
+ context 'when pipeline is triggered by trigger API' do
+ it 'returns variables' do
+ expect(presenter.trigger_variables).to eq([pipeline_variable.to_runner_variable])
+ end
+ end
+
+ context 'when pipeline is not triggered by trigger API' do
+ let(:build) { create(:ci_build, pipeline: pipeline) }
+
+ it 'does not return variables' do
+ expect(presenter.trigger_variables).to eq([])
+ end
+ end
+ end
+
+ context 'when variable is stored in ci_trigger_requests.variables' do
+ before do
+ trigger_request.update_attribute(:variables, { 'TRIGGER_KEY_1' => 'TRIGGER_VALUE_1' } )
+ end
+
+ it 'returns variables' do
+ expect(presenter.trigger_variables).to eq(trigger_request.user_variables)
+ end
+ end
+ end
end
diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb
index 419a65c69b5..12720355a6d 100644
--- a/spec/requests/api/runner_spec.rb
+++ b/spec/requests/api/runner_spec.rb
@@ -557,17 +557,36 @@ describe API::Runner do
{ 'key' => 'TRIGGER_KEY_1', 'value' => 'TRIGGER_VALUE_1', 'public' => false }]
end
+ let(:trigger) { create(:ci_trigger, project: project) }
+ let!(:trigger_request) { create(:ci_trigger_request, pipeline: pipeline, builds: [job], trigger: trigger) }
+
before do
- trigger = create(:ci_trigger, project: project)
- create(:ci_trigger_request_with_variables, pipeline: pipeline, builds: [job], trigger: trigger)
project.variables << Ci::Variable.new(key: 'SECRET_KEY', value: 'secret_value')
end
- it 'returns variables for triggers' do
- request_job
+ shared_examples 'expected variables behavior' do
+ it 'returns variables for triggers' do
+ request_job
- expect(response).to have_http_status(201)
- expect(json_response['variables']).to include(*expected_variables)
+ expect(response).to have_http_status(201)
+ expect(json_response['variables']).to include(*expected_variables)
+ end
+ end
+
+ context 'when variables are stored in trigger_request' do
+ before do
+ trigger_request.update_attribute(:variables, { TRIGGER_KEY_1: 'TRIGGER_VALUE_1' } )
+ end
+
+ it_behaves_like 'expected variables behavior'
+ end
+
+ context 'when variables are stored in pipeline_variables' do
+ before do
+ create(:ci_pipeline_variable, pipeline: pipeline, key: :TRIGGER_KEY_1, value: 'TRIGGER_VALUE_1')
+ end
+
+ it_behaves_like 'expected variables behavior'
end
end
diff --git a/spec/requests/api/v3/triggers_spec.rb b/spec/requests/api/v3/triggers_spec.rb
index d4648136841..7ccf387f2dc 100644
--- a/spec/requests/api/v3/triggers_spec.rb
+++ b/spec/requests/api/v3/triggers_spec.rb
@@ -37,7 +37,7 @@ describe API::V3::Triggers do
it 'returns unauthorized if token is for different project' do
post v3_api("/projects/#{project2.id}/trigger/builds"), options.merge(ref: 'master')
- expect(response).to have_http_status(401)
+ expect(response).to have_http_status(404)
end
end
@@ -80,7 +80,8 @@ describe API::V3::Triggers do
post v3_api("/projects/#{project.id}/trigger/builds"), options.merge(variables: variables, ref: 'master')
expect(response).to have_http_status(201)
pipeline.builds.reload
- expect(pipeline.builds.first.trigger_request.variables).to eq(variables)
+ expect(pipeline.variables.map { |v| { v.key => v.value } }.first).to eq(variables)
+ expect(json_response['variables']).to eq(variables)
end
end
end
diff --git a/spec/services/ci/create_trigger_request_service_spec.rb b/spec/services/ci/create_trigger_request_service_spec.rb
deleted file mode 100644
index 8295813a1ca..00000000000
--- a/spec/services/ci/create_trigger_request_service_spec.rb
+++ /dev/null
@@ -1,52 +0,0 @@
-require 'spec_helper'
-
-describe Ci::CreateTriggerRequestService do
- let(:service) { described_class }
- let(:project) { create(:project, :repository) }
- let(:trigger) { create(:ci_trigger, project: project, owner: owner) }
- let(:owner) { create(:user) }
-
- before do
- stub_ci_pipeline_to_return_yaml_file
-
- project.add_developer(owner)
- end
-
- describe '#execute' do
- context 'valid params' do
- subject { service.execute(project, trigger, 'master') }
-
- context 'without owner' do
- it { expect(subject.trigger_request).to be_kind_of(Ci::TriggerRequest) }
- it { expect(subject.trigger_request.builds.first).to be_kind_of(Ci::Build) }
- it { expect(subject.pipeline).to be_kind_of(Ci::Pipeline) }
- it { expect(subject.pipeline).to be_trigger }
- end
-
- context 'with owner' do
- it { expect(subject.trigger_request).to be_kind_of(Ci::TriggerRequest) }
- it { expect(subject.trigger_request.builds.first).to be_kind_of(Ci::Build) }
- it { expect(subject.trigger_request.builds.first.user).to eq(owner) }
- it { expect(subject.pipeline).to be_kind_of(Ci::Pipeline) }
- it { expect(subject.pipeline).to be_trigger }
- it { expect(subject.pipeline.user).to eq(owner) }
- end
- end
-
- context 'no commit for ref' do
- subject { service.execute(project, trigger, 'other-branch') }
-
- it { expect(subject.pipeline).not_to be_persisted }
- end
-
- context 'no builds created' do
- subject { service.execute(project, trigger, 'master') }
-
- before do
- stub_ci_pipeline_yaml_file('script: { only: [develop], script: hello World }')
- end
-
- it { expect(subject.pipeline).not_to be_persisted }
- end
- end
-end
diff --git a/spec/views/projects/jobs/show.html.haml_spec.rb b/spec/views/projects/jobs/show.html.haml_spec.rb
index 117f48450e2..d4279626e75 100644
--- a/spec/views/projects/jobs/show.html.haml_spec.rb
+++ b/spec/views/projects/jobs/show.html.haml_spec.rb
@@ -195,20 +195,4 @@ describe 'projects/jobs/show' do
text: /\A\n#{Regexp.escape(commit_title)}\n\Z/)
end
end
-
- describe 'shows trigger variables in sidebar' do
- let(:trigger_request) { create(:ci_trigger_request_with_variables, pipeline: pipeline) }
-
- before do
- build.trigger_request = trigger_request
- render
- end
-
- it 'shows trigger variables in separate lines' do
- expect(rendered).to have_css('.js-build-variable', visible: false, text: 'TRIGGER_KEY_1')
- expect(rendered).to have_css('.js-build-variable', visible: false, text: 'TRIGGER_KEY_2')
- expect(rendered).to have_css('.js-build-value', visible: false, text: 'TRIGGER_VALUE_1')
- expect(rendered).to have_css('.js-build-value', visible: false, text: 'TRIGGER_VALUE_2')
- end
- end
end