diff options
Diffstat (limited to 'spec')
19 files changed, 626 insertions, 114 deletions
diff --git a/spec/factories/ci/bridge.rb b/spec/factories/ci/bridge.rb index b2e8051eb5e..bacf163896c 100644 --- a/spec/factories/ci/bridge.rb +++ b/spec/factories/ci/bridge.rb @@ -9,6 +9,7 @@ FactoryBot.define do tag { false } created_at { 'Di 29. Okt 09:50:00 CET 2013' } status { :created } + scheduling_type { 'stage' } pipeline factory: :ci_pipeline diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb index 3d65f9065bf..5127d55645c 100644 --- a/spec/factories/ci/builds.rb +++ b/spec/factories/ci/builds.rb @@ -11,6 +11,7 @@ FactoryBot.define do tag { false } add_attribute(:protected) { false } created_at { 'Di 29. Okt 09:50:00 CET 2013' } + scheduling_type { 'stage' } pending options do diff --git a/spec/frontend/diffs/components/diff_line_gutter_content_spec.js b/spec/frontend/diffs/components/diff_line_gutter_content_spec.js index e1c03983ab5..0553498bfa0 100644 --- a/spec/frontend/diffs/components/diff_line_gutter_content_spec.js +++ b/spec/frontend/diffs/components/diff_line_gutter_content_spec.js @@ -1,105 +1,195 @@ -import Vue from 'vue'; -import { createComponentWithStore } from 'helpers/vue_mount_component_helper'; +import Vuex from 'vuex'; +import { shallowMount, createLocalVue } from '@vue/test-utils'; import DiffLineGutterContent from '~/diffs/components/diff_line_gutter_content.vue'; +import DiffGutterAvatars from '~/diffs/components/diff_gutter_avatars.vue'; +import { LINE_POSITION_RIGHT } from '~/diffs/constants'; import { createStore } from '~/mr_notes/stores'; +import { TEST_HOST } from 'helpers/test_constants'; import discussionsMockData from '../mock_data/diff_discussions'; import diffFileMockData from '../mock_data/diff_file'; +const localVue = createLocalVue(); +localVue.use(Vuex); + +const TEST_USER_ID = 'abc123'; +const TEST_USER = { id: TEST_USER_ID }; +const TEST_LINE_NUMBER = 1; +const TEST_LINE_CODE = 'LC_42'; +const TEST_FILE_HASH = diffFileMockData.file_hash; + describe('DiffLineGutterContent', () => { - const getDiffFileMock = () => Object.assign({}, diffFileMockData); - const createComponent = (options = {}) => { - const cmp = Vue.extend(DiffLineGutterContent); - const props = Object.assign({}, options); - props.line = { - line_code: 'LC_42', + let wrapper; + let line; + let store; + + beforeEach(() => { + store = createStore(); + store.state.notes.userData = TEST_USER; + + line = { + line_code: TEST_LINE_CODE, type: 'new', old_line: null, new_line: 1, discussions: [{ ...discussionsMockData }], + discussionsExpanded: true, text: '+<span id="LC1" class="line" lang="plaintext"> - Bad dates</span>\n', rich_text: '+<span id="LC1" class="line" lang="plaintext"> - Bad dates</span>\n', meta_data: null, }; - props.fileHash = getDiffFileMock().file_hash; - props.contextLinesPath = '/context/lines/path'; + }); - return createComponentWithStore(cmp, createStore(), props).$mount(); - }; + afterEach(() => { + wrapper.destroy(); + }); - describe('computed', () => { - describe('lineHref', () => { - it('should prepend # to lineCode', () => { - const lineCode = 'LC_42'; - const component = createComponent(); + const setWindowLocation = value => { + Object.defineProperty(window, 'location', { + writable: true, + value, + }); + }; + const createComponent = (props = {}) => { + wrapper = shallowMount(DiffLineGutterContent, { + localVue, + store, + propsData: { + line, + fileHash: TEST_FILE_HASH, + contextLinesPath: '/context/lines/path', + ...props, + }, + }); + }; + const findNoteButton = () => wrapper.find('.js-add-diff-note-button'); + const findLineNumber = () => wrapper.find({ ref: 'lineNumberRef' }); + const findAvatars = () => wrapper.find(DiffGutterAvatars); + + describe('comment button', () => { + it.each` + showCommentButton | userData | query | expectation + ${true} | ${TEST_USER} | ${'diff_head=false'} | ${true} + ${true} | ${TEST_USER} | ${'diff_head=true'} | ${false} + ${false} | ${TEST_USER} | ${'bogus'} | ${false} + ${true} | ${null} | ${''} | ${false} + `( + 'exists is $expectation - with showCommentButton ($showCommentButton) userData ($userData) query ($query)', + ({ showCommentButton, userData, query, expectation }) => { + store.state.notes.userData = userData; + setWindowLocation({ href: `${TEST_HOST}?${query}` }); + createComponent({ showCommentButton }); + + expect(findNoteButton().exists()).toBe(expectation); + }, + ); + + it.each` + isHover | otherProps | discussions | expectation + ${true} | ${{}} | ${[]} | ${true} + ${false} | ${{}} | ${[]} | ${false} + ${true} | ${{ isMatchLine: true }} | ${[]} | ${false} + ${true} | ${{ isContextLine: true }} | ${[]} | ${false} + ${true} | ${{ isMetaLine: true }} | ${[]} | ${false} + ${true} | ${{}} | ${[{}]} | ${false} + `( + 'visible is $expectation - with isHover ($isHover), discussions ($discussions), otherProps ($otherProps)', + ({ isHover, otherProps, discussions, expectation }) => { + line.discussions = discussions; + createComponent({ + showCommentButton: true, + isHover, + ...otherProps, + }); - expect(component.lineHref).toEqual(`#${lineCode}`); - }); + expect(findNoteButton().isVisible()).toBe(expectation); + }, + ); + }); - it('should return # if there is no lineCode', () => { - const component = createComponent(); - component.line.line_code = ''; + describe('line number', () => { + describe('without lineNumber prop', () => { + it('does not render', () => { + createComponent(); - expect(component.lineHref).toEqual('#'); + expect(findLineNumber().exists()).toBe(false); }); }); - describe('discussions, hasDiscussions, shouldShowAvatarsOnGutter', () => { - it('should return empty array when there is no discussion', () => { - const component = createComponent(); - component.line.discussions = []; - - expect(component.hasDiscussions).toEqual(false); - expect(component.shouldShowAvatarsOnGutter).toEqual(false); - }); - - it('should return discussions for the given lineCode', () => { - const cmp = Vue.extend(DiffLineGutterContent); - const props = { - line: getDiffFileMock().highlighted_diff_lines[1], - fileHash: getDiffFileMock().file_hash, - showCommentButton: true, - contextLinesPath: '/context/lines/path', - }; - props.line.discussions = [Object.assign({}, discussionsMockData)]; - const component = createComponentWithStore(cmp, createStore(), props).$mount(); - - expect(component.hasDiscussions).toEqual(true); - expect(component.shouldShowAvatarsOnGutter).toEqual(true); + describe('with lineNumber prop', () => { + describe.each` + lineProps | expectedHref | expectedClickArg + ${{ line_code: TEST_LINE_CODE }} | ${`#${TEST_LINE_CODE}`} | ${TEST_LINE_CODE} + ${{ line_code: undefined }} | ${'#'} | ${undefined} + ${{ line_code: undefined, left: { line_code: TEST_LINE_CODE } }} | ${'#'} | ${TEST_LINE_CODE} + ${{ line_code: undefined, right: { line_code: TEST_LINE_CODE } }} | ${'#'} | ${TEST_LINE_CODE} + `('with line ($lineProps)', ({ lineProps, expectedHref, expectedClickArg }) => { + beforeEach(() => { + jest.spyOn(store, 'dispatch').mockImplementation(); + Object.assign(line, lineProps); + createComponent({ lineNumber: TEST_LINE_NUMBER }); + }); + + it('renders', () => { + expect(findLineNumber().exists()).toBe(true); + expect(findLineNumber().attributes()).toEqual({ + href: expectedHref, + 'data-linenumber': TEST_LINE_NUMBER.toString(), + }); + }); + + it('on click, dispatches setHighlightedRow', () => { + expect(store.dispatch).not.toHaveBeenCalled(); + + findLineNumber().trigger('click'); + + expect(store.dispatch).toHaveBeenCalledWith('diffs/setHighlightedRow', expectedClickArg); + }); }); }); }); - describe('template', () => { - it('should render comment button', () => { - const component = createComponent({ - showCommentButton: true, - }); - Object.defineProperty(component, 'isLoggedIn', { - get() { - return true; - }, + describe('diff-gutter-avatars', () => { + describe('with showCommentButton', () => { + beforeEach(() => { + jest.spyOn(store, 'dispatch').mockImplementation(); + + createComponent({ showCommentButton: true }); }); - expect(component.$el.querySelector('.js-add-diff-note-button')).toBeDefined(); - }); + it('renders', () => { + expect(findAvatars().props()).toEqual({ + discussions: line.discussions, + discussionsExpanded: line.discussionsExpanded, + }); + }); - it('should render line link', () => { - const lineNumber = 42; - const lineCode = `LC_${lineNumber}`; - const component = createComponent({ lineNumber, lineCode }); - const link = component.$el.querySelector('a'); + it('toggles line discussion', () => { + expect(store.dispatch).not.toHaveBeenCalled(); - expect(link.href.indexOf(`#${lineCode}`)).toBeGreaterThan(-1); - expect(link.dataset.linenumber).toEqual(lineNumber.toString()); - }); + findAvatars().vm.$emit('toggleLineDiscussions'); - it('should render user avatars', () => { - const component = createComponent({ - showCommentButton: true, - lineCode: getDiffFileMock().highlighted_diff_lines[1].line_code, + expect(store.dispatch).toHaveBeenCalledWith('diffs/toggleLineDiscussions', { + lineCode: TEST_LINE_CODE, + fileHash: TEST_FILE_HASH, + expanded: !line.discussionsExpanded, + }); }); - - expect(component.$el.querySelector('.diff-comment-avatar-holders')).not.toBe(null); }); + + it.each` + props | lineProps | expectation + ${{ showCommentButton: true }} | ${{}} | ${true} + ${{ showCommentButton: false }} | ${{}} | ${false} + ${{ showCommentButton: true, linePosition: LINE_POSITION_RIGHT }} | ${{ type: null }} | ${false} + ${{ showCommentButton: true }} | ${{ discussions: [] }} | ${false} + `( + 'exists is $expectation - with props ($props), line ($lineProps)', + ({ props, lineProps, expectation }) => { + Object.assign(line, lineProps); + createComponent(props); + + expect(findAvatars().exists()).toBe(expectation); + }, + ); }); }); diff --git a/spec/javascripts/behaviors/shortcuts/shortcuts_issuable_spec.js b/spec/javascripts/behaviors/shortcuts/shortcuts_issuable_spec.js index 5e457a4e823..f6232026915 100644 --- a/spec/javascripts/behaviors/shortcuts/shortcuts_issuable_spec.js +++ b/spec/javascripts/behaviors/shortcuts/shortcuts_issuable_spec.js @@ -52,6 +52,7 @@ describe('ShortcutsIssuable', function() { return documentFragment; }); }; + describe('with empty selection', () => { it('does not return an error', () => { ShortcutsIssuable.replyWithSelectedText(true); @@ -297,5 +298,18 @@ describe('ShortcutsIssuable', function() { }); }); }); + + describe('with a valid selection with no text content', () => { + it('returns the proper markdown', done => { + stubSelection('<img src="foo" alt="image" />'); + ShortcutsIssuable.replyWithSelectedText(true); + + setTimeout(() => { + expect($(FORM_SELECTOR).val()).toBe('> ![image](http://localhost:9876/foo)\n\n'); + + done(); + }); + }); + }); }); }); diff --git a/spec/lib/gitlab/ci/build/rules_spec.rb b/spec/lib/gitlab/ci/build/rules_spec.rb index 1ebcc4f9414..31a9fa055e1 100644 --- a/spec/lib/gitlab/ci/build/rules_spec.rb +++ b/spec/lib/gitlab/ci/build/rules_spec.rb @@ -102,9 +102,9 @@ describe Gitlab::Ci::Build::Rules do end context 'with one rule without any clauses' do - let(:rule_list) { [{ when: 'manual' }] } + let(:rule_list) { [{ when: 'manual', allow_failure: true }] } - it { is_expected.to eq(described_class::Result.new('manual')) } + it { is_expected.to eq(described_class::Result.new('manual', nil, true)) } end context 'with one matching rule' do @@ -166,5 +166,51 @@ describe Gitlab::Ci::Build::Rules do end end end + + context 'with only allow_failure' do + context 'with matching rule' do + let(:rule_list) { [{ if: '$VAR == null', allow_failure: true }] } + + it { is_expected.to eq(described_class::Result.new('on_success', nil, true)) } + end + + context 'with non-matching rule' do + let(:rule_list) { [{ if: '$VAR != null', allow_failure: true }] } + + it { is_expected.to eq(described_class::Result.new('never')) } + end + end + end + + describe 'Gitlab::Ci::Build::Rules::Result' do + let(:when_value) { 'on_success' } + let(:start_in) { nil } + let(:allow_failure) { nil } + + subject { Gitlab::Ci::Build::Rules::Result.new(when_value, start_in, allow_failure) } + + describe '#build_attributes' do + it 'compacts nil values' do + expect(subject.build_attributes).to eq(options: {}, when: 'on_success') + end + end + + describe '#pass?' do + context "'when' is 'never'" do + let!(:when_value) { 'never' } + + it 'returns false' do + expect(subject.pass?).to eq(false) + end + end + + context "'when' is 'on_success'" do + let!(:when_value) { 'on_success' } + + it 'returns true' do + expect(subject.pass?).to eq(true) + end + end + end end end diff --git a/spec/lib/gitlab/ci/config/entry/bridge_spec.rb b/spec/lib/gitlab/ci/config/entry/bridge_spec.rb index 07590556db8..ad388886681 100644 --- a/spec/lib/gitlab/ci/config/entry/bridge_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/bridge_spec.rb @@ -105,7 +105,8 @@ describe Gitlab::Ci::Config::Entry::Bridge do trigger: { project: 'some/project' }, ignore: false, stage: 'test', - only: { refs: %w[branches tags] }) + only: { refs: %w[branches tags] }, + scheduling_type: :stage) end end end @@ -126,7 +127,8 @@ describe Gitlab::Ci::Config::Entry::Bridge do branch: 'feature' }, ignore: false, stage: 'test', - only: { refs: %w[branches tags] }) + only: { refs: %w[branches tags] }, + scheduling_type: :stage) end end end diff --git a/spec/lib/gitlab/ci/config/entry/job_spec.rb b/spec/lib/gitlab/ci/config/entry/job_spec.rb index 649689f7d3b..313b504ab59 100644 --- a/spec/lib/gitlab/ci/config/entry/job_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/job_spec.rb @@ -110,6 +110,10 @@ describe Gitlab::Ci::Config::Entry::Job do it { expect(entry).to be_valid } + it "returns scheduling_type as :dag" do + expect(entry.value[:scheduling_type]).to eq(:dag) + end + context 'when has dependencies' do let(:config) do { @@ -598,7 +602,8 @@ describe Gitlab::Ci::Config::Entry::Job do ignore: false, after_script: %w[cleanup], only: { refs: %w[branches tags] }, - variables: {}) + variables: {}, + scheduling_type: :stage) end end end diff --git a/spec/lib/gitlab/ci/config/entry/jobs_spec.rb b/spec/lib/gitlab/ci/config/entry/jobs_spec.rb index 05249b7c717..c8c188d71bf 100644 --- a/spec/lib/gitlab/ci/config/entry/jobs_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/jobs_spec.rb @@ -98,7 +98,8 @@ describe Gitlab::Ci::Config::Entry::Jobs do name: :my_trigger, only: { refs: %w[branches tags] }, stage: 'test', - trigger: { project: 'my/project' } + trigger: { project: 'my/project' }, + scheduling_type: :stage }, regular_job: { ignore: false, @@ -106,7 +107,8 @@ describe Gitlab::Ci::Config::Entry::Jobs do only: { refs: %w[branches tags] }, script: ['something'], stage: 'test', - variables: {} + variables: {}, + scheduling_type: :stage }) end end diff --git a/spec/lib/gitlab/ci/config/entry/root_spec.rb b/spec/lib/gitlab/ci/config/entry/root_spec.rb index 95a5b8e88fb..cf0a3cfa963 100644 --- a/spec/lib/gitlab/ci/config/entry/root_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/root_spec.rb @@ -130,7 +130,8 @@ describe Gitlab::Ci::Config::Entry::Root do variables: {}, ignore: false, after_script: ['make clean'], - only: { refs: %w[branches tags] } } + only: { refs: %w[branches tags] }, + scheduling_type: :stage } ) expect(root.jobs_value[:spinach]).to eq( { name: :spinach, @@ -143,7 +144,8 @@ describe Gitlab::Ci::Config::Entry::Root do variables: {}, ignore: false, after_script: ['make clean'], - only: { refs: %w[branches tags] } } + only: { refs: %w[branches tags] }, + scheduling_type: :stage } ) expect(root.jobs_value[:release]).to eq( { name: :release, @@ -157,7 +159,8 @@ describe Gitlab::Ci::Config::Entry::Root do only: { refs: %w(branches tags) }, variables: {}, after_script: [], - ignore: false } + ignore: false, + scheduling_type: :stage } ) end end @@ -203,7 +206,8 @@ describe Gitlab::Ci::Config::Entry::Root do variables: {}, ignore: false, after_script: ['make clean'], - only: { refs: %w[branches tags] } }, + only: { refs: %w[branches tags] }, + scheduling_type: :stage }, spinach: { name: :spinach, before_script: [], script: %w[spinach], @@ -214,7 +218,8 @@ describe Gitlab::Ci::Config::Entry::Root do variables: { 'VAR' => 'AA' }, ignore: false, after_script: ['make clean'], - only: { refs: %w[branches tags] } } + only: { refs: %w[branches tags] }, + scheduling_type: :stage } ) end end diff --git a/spec/lib/gitlab/ci/config/entry/rules/rule_spec.rb b/spec/lib/gitlab/ci/config/entry/rules/rule_spec.rb index 216f5d0c77d..20db5f02fc7 100644 --- a/spec/lib/gitlab/ci/config/entry/rules/rule_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/rules/rule_spec.rb @@ -27,8 +27,14 @@ describe Gitlab::Ci::Config::Entry::Rules::Rule do it { is_expected.to be_valid } end + context 'with an allow_failure: value but no clauses' do + let(:config) { { allow_failure: true } } + + it { is_expected.to be_valid } + end + context 'when specifying an if: clause' do - let(:config) { { if: '$THIS || $THAT', when: 'manual' } } + let(:config) { { if: '$THIS || $THAT', when: 'manual', allow_failure: true } } it { is_expected.to be_valid } @@ -37,6 +43,12 @@ describe Gitlab::Ci::Config::Entry::Rules::Rule do it { is_expected.to eq('manual') } end + + describe '#allow_failure' do + subject { entry.allow_failure } + + it { is_expected.to eq(true) } + end end context 'using a list of multiple expressions' do @@ -328,16 +340,43 @@ describe Gitlab::Ci::Config::Entry::Rules::Rule do end end end + + context 'allow_failure: validation' do + context 'with an invalid string allow_failure:' do + let(:config) do + { if: '$THIS == "that"', allow_failure: 'always' } + end + + it { is_expected.to be_a(described_class) } + it { is_expected.not_to be_valid } + + it 'returns an error about invalid allow_failure:' do + expect(subject.errors).to include(/rule allow failure should be a boolean value/) + end + + context 'when composed' do + before do + subject.compose! + end + + it { is_expected.not_to be_valid } + + it 'returns an error about invalid allow_failure:' do + expect(subject.errors).to include(/rule allow failure should be a boolean value/) + end + end + end + end end describe '#value' do subject { entry.value } context 'when specifying an if: clause' do - let(:config) { { if: '$THIS || $THAT', when: 'manual' } } + let(:config) { { if: '$THIS || $THAT', when: 'manual', allow_failure: true } } it 'stores the expression as "if"' do - expect(subject).to eq(if: '$THIS || $THAT', when: 'manual') + expect(subject).to eq(if: '$THIS || $THAT', when: 'manual', allow_failure: true) end end diff --git a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb index 5526ec9e16f..1f5fc000832 100644 --- a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb @@ -6,7 +6,7 @@ describe Gitlab::Ci::Pipeline::Seed::Build do let(:project) { create(:project, :repository) } let(:head_sha) { project.repository.head_commit.id } let(:pipeline) { create(:ci_empty_pipeline, project: project, sha: head_sha) } - let(:attributes) { { name: 'rspec', ref: 'master' } } + let(:attributes) { { name: 'rspec', ref: 'master', scheduling_type: :stage } } let(:previous_stages) { [] } let(:seed_build) { described_class.new(pipeline, attributes, previous_stages) } @@ -244,7 +244,9 @@ describe Gitlab::Ci::Pipeline::Seed::Build do context 'when job is a bridge' do let(:attributes) do - { name: 'rspec', ref: 'master', options: { trigger: 'my/project' } } + { + name: 'rspec', ref: 'master', options: { trigger: 'my/project' }, scheduling_type: :stage + } end it { is_expected.to be_a(::Ci::Bridge) } diff --git a/spec/lib/gitlab/ci/pipeline/seed/stage_spec.rb b/spec/lib/gitlab/ci/pipeline/seed/stage_spec.rb index a978084876f..875fd457bd0 100644 --- a/spec/lib/gitlab/ci/pipeline/seed/stage_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/seed/stage_spec.rb @@ -10,9 +10,9 @@ describe Gitlab::Ci::Pipeline::Seed::Stage do let(:attributes) do { name: 'test', index: 0, - builds: [{ name: 'rspec' }, - { name: 'spinach' }, - { name: 'deploy', only: { refs: ['feature'] } }] } + builds: [{ name: 'rspec', scheduling_type: :stage }, + { name: 'spinach', scheduling_type: :stage }, + { name: 'deploy', only: { refs: ['feature'] } }], scheduling_type: :stage } end subject do diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb index 11168a969fc..e5c5aaa2265 100644 --- a/spec/lib/gitlab/ci/yaml_processor_spec.rb +++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb @@ -36,7 +36,8 @@ module Gitlab interruptible: true, allow_failure: false, when: "on_success", - yaml_variables: [] + yaml_variables: [], + scheduling_type: :stage }) end end @@ -66,7 +67,8 @@ module Gitlab ], allow_failure: false, when: 'on_success', - yaml_variables: [] + yaml_variables: [], + scheduling_type: :stage }) end end @@ -126,7 +128,8 @@ module Gitlab interruptible: true, allow_failure: false, when: "on_success", - yaml_variables: [] + yaml_variables: [], + scheduling_type: :stage }) end end @@ -282,6 +285,7 @@ module Gitlab allow_failure: false, when: "on_success", yaml_variables: [], + scheduling_type: :stage, options: { script: ["rspec"] }, only: { refs: ["branches"] } }] }, { name: "deploy", @@ -293,6 +297,7 @@ module Gitlab allow_failure: false, when: "on_success", yaml_variables: [], + scheduling_type: :stage, options: { script: ["cap prod"] }, only: { refs: ["tags"] } }] }, { name: ".post", @@ -642,7 +647,8 @@ module Gitlab }, allow_failure: false, when: "on_success", - yaml_variables: [] + yaml_variables: [], + scheduling_type: :stage }) end @@ -674,7 +680,8 @@ module Gitlab }, allow_failure: false, when: "on_success", - yaml_variables: [] + yaml_variables: [], + scheduling_type: :stage }) end end @@ -702,7 +709,8 @@ module Gitlab }, allow_failure: false, when: "on_success", - yaml_variables: [] + yaml_variables: [], + scheduling_type: :stage }) end @@ -728,7 +736,8 @@ module Gitlab }, allow_failure: false, when: "on_success", - yaml_variables: [] + yaml_variables: [], + scheduling_type: :stage }) end end @@ -1250,7 +1259,8 @@ module Gitlab }, when: "on_success", allow_failure: false, - yaml_variables: [] + yaml_variables: [], + scheduling_type: :stage }) end @@ -1604,7 +1614,8 @@ module Gitlab }, when: "on_success", allow_failure: false, - yaml_variables: [] + yaml_variables: [], + scheduling_type: :stage ) expect(subject.builds[4]).to eq( stage: "test", @@ -1618,7 +1629,8 @@ module Gitlab ], when: "on_success", allow_failure: false, - yaml_variables: [] + yaml_variables: [], + scheduling_type: :dag ) end end @@ -1644,7 +1656,8 @@ module Gitlab }, when: "on_success", allow_failure: false, - yaml_variables: [] + yaml_variables: [], + scheduling_type: :stage ) expect(subject.builds[4]).to eq( stage: "test", @@ -1660,7 +1673,8 @@ module Gitlab ], when: "on_success", allow_failure: false, - yaml_variables: [] + yaml_variables: [], + scheduling_type: :dag ) end end @@ -1682,7 +1696,8 @@ module Gitlab ], when: "on_success", allow_failure: false, - yaml_variables: [] + yaml_variables: [], + scheduling_type: :dag ) end end @@ -1712,7 +1727,8 @@ module Gitlab ], when: "on_success", allow_failure: false, - yaml_variables: [] + yaml_variables: [], + scheduling_type: :dag ) end end @@ -1849,7 +1865,8 @@ module Gitlab }, when: "on_success", allow_failure: false, - yaml_variables: [] + yaml_variables: [], + scheduling_type: :stage }) end end @@ -1895,7 +1912,8 @@ module Gitlab }, when: "on_success", allow_failure: false, - yaml_variables: [] + yaml_variables: [], + scheduling_type: :stage }) expect(subject.second).to eq({ stage: "build", @@ -1907,7 +1925,8 @@ module Gitlab }, when: "on_success", allow_failure: false, - yaml_variables: [] + yaml_variables: [], + scheduling_type: :stage }) end end diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index 55e7d6bd1e3..807b017a67c 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -360,6 +360,7 @@ CommitStatus: - upstream_pipeline_id - interruptible - processed +- scheduling_type Ci::Variable: - id - project_id diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index b99f51bb36e..91185446488 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -3007,7 +3007,8 @@ describe Ci::Build do stage: 'test', ref: 'feature', project: project, - pipeline: pipeline + pipeline: pipeline, + scheduling_type: :stage ) end diff --git a/spec/models/ci/processable_spec.rb b/spec/models/ci/processable_spec.rb index 87dbcbf870e..370606a73bc 100644 --- a/spec/models/ci/processable_spec.rb +++ b/spec/models/ci/processable_spec.rb @@ -52,4 +52,72 @@ describe Ci::Processable do end end end + + describe 'validate presence of scheduling_type' do + context 'on create' do + let(:processable) do + build( + :ci_build, :created, project: project, pipeline: pipeline, + importing: importing, scheduling_type: nil + ) + end + + context 'when importing' do + let(:importing) { true } + + context 'when validate_scheduling_type_of_processables is true' do + before do + stub_feature_flags(validate_scheduling_type_of_processables: true) + end + + it 'does not validate' do + expect(processable).to be_valid + end + end + + context 'when validate_scheduling_type_of_processables is false' do + before do + stub_feature_flags(validate_scheduling_type_of_processables: false) + end + + it 'does not validate' do + expect(processable).to be_valid + end + end + end + + context 'when not importing' do + let(:importing) { false } + + context 'when validate_scheduling_type_of_processables is true' do + before do + stub_feature_flags(validate_scheduling_type_of_processables: true) + end + + it 'validates' do + expect(processable).not_to be_valid + end + end + + context 'when validate_scheduling_type_of_processables is false' do + before do + stub_feature_flags(validate_scheduling_type_of_processables: false) + end + + it 'does not validate' do + expect(processable).to be_valid + end + end + end + end + + context 'on update' do + let(:processable) { create(:ci_build, :created, project: project, pipeline: pipeline) } + + it 'does not validate' do + processable.scheduling_type = nil + expect(processable).to be_valid + end + end + end end diff --git a/spec/services/ci/create_pipeline_service/needs_spec.rb b/spec/services/ci/create_pipeline_service/needs_spec.rb index 5ef7e592b36..757c076f9e6 100644 --- a/spec/services/ci/create_pipeline_service/needs_spec.rb +++ b/spec/services/ci/create_pipeline_service/needs_spec.rb @@ -131,6 +131,10 @@ describe Ci::CreatePipelineService do ) end end + + it "sets scheduling_type as 'dag'" do + expect(test_a_build.scheduling_type).to eq('dag') + end end context 'with an invalid config' do diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index d6cc233088d..eb7e0c1c226 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -1750,9 +1750,9 @@ describe Ci::CreatePipelineService do let(:ref_name) { 'refs/heads/master' } let(:pipeline) { execute_service } let(:build_names) { pipeline.builds.pluck(:name) } - let(:regular_job) { pipeline.builds.find_by(name: 'regular-job') } - let(:rules_job) { pipeline.builds.find_by(name: 'rules-job') } - let(:delayed_job) { pipeline.builds.find_by(name: 'delayed-job') } + let(:regular_job) { find_job('regular-job') } + let(:rules_job) { find_job('rules-job') } + let(:delayed_job) { find_job('delayed-job') } shared_examples 'rules jobs are excluded' do it 'only persists the job without rules' do @@ -1763,6 +1763,10 @@ describe Ci::CreatePipelineService do end end + def find_job(name) + pipeline.builds.find_by(name: name) + end + before do stub_ci_pipeline_yaml_file(config) allow_any_instance_of(Ci::BuildScheduleWorker).to receive(:perform).and_return(true) @@ -1782,6 +1786,12 @@ describe Ci::CreatePipelineService do - if: $CI_COMMIT_REF_NAME =~ /master/ when: manual + negligible-job: + script: "exit 1" + rules: + - if: $CI_COMMIT_REF_NAME =~ /master/ + allow_failure: true + delayed-job: script: "echo See you later, World!" rules: @@ -1800,11 +1810,23 @@ describe Ci::CreatePipelineService do context 'with matches' do it 'creates a pipeline with the vanilla and manual jobs' do expect(pipeline).to be_persisted - expect(build_names).to contain_exactly('regular-job', 'delayed-job', 'master-job') + expect(build_names).to contain_exactly( + 'regular-job', 'delayed-job', 'master-job', 'negligible-job' + ) end it 'assigns job:when values to the builds' do - expect(pipeline.builds.pluck(:when)).to contain_exactly('on_success', 'delayed', 'manual') + expect(find_job('regular-job').when).to eq('on_success') + expect(find_job('master-job').when).to eq('manual') + expect(find_job('negligible-job').when).to eq('on_success') + expect(find_job('delayed-job').when).to eq('delayed') + end + + it 'assigns job:allow_failure values to the builds' do + expect(find_job('regular-job').allow_failure).to eq(false) + expect(find_job('master-job').allow_failure).to eq(false) + expect(find_job('negligible-job').allow_failure).to eq(true) + expect(find_job('delayed-job').allow_failure).to eq(false) end it 'assigns start_in for delayed jobs' do @@ -1827,6 +1849,7 @@ describe Ci::CreatePipelineService do rules: - if: $VAR == 'present' && $OTHER || $CI_COMMIT_REF_NAME when: manual + allow_failure: true EOY end @@ -1834,6 +1857,7 @@ describe Ci::CreatePipelineService do expect(pipeline).to be_persisted expect(build_names).to contain_exactly('regular-job') expect(regular_job.when).to eq('manual') + expect(regular_job.allow_failure).to eq(true) end end @@ -1860,6 +1884,13 @@ describe Ci::CreatePipelineService do - README.md when: delayed start_in: 4 hours + + negligible-job: + script: "can be failed sometimes" + rules: + - changes: + - README.md + allow_failure: true EOY end @@ -1872,7 +1903,7 @@ describe Ci::CreatePipelineService do it 'creates two jobs' do expect(pipeline).to be_persisted expect(build_names) - .to contain_exactly('regular-job', 'rules-job', 'delayed-job') + .to contain_exactly('regular-job', 'rules-job', 'delayed-job', 'negligible-job') end it 'sets when: for all jobs' do @@ -1881,6 +1912,10 @@ describe Ci::CreatePipelineService do expect(delayed_job.when).to eq('delayed') expect(delayed_job.options[:start_in]).to eq('4 hours') end + + it 'sets allow_failure: for negligible job' do + expect(find_job('negligible-job').allow_failure).to eq(true) + end end context 'and matches the second rule' do @@ -1922,12 +1957,14 @@ describe Ci::CreatePipelineService do rules-job: script: "echo hello world, $CI_COMMIT_REF_NAME" + allow_failure: true rules: - changes: - README.md when: manual - if: $CI_COMMIT_REF_NAME == "master" when: on_success + allow_failure: false delayed-job: script: "echo See you later, World!" @@ -1936,6 +1973,7 @@ describe Ci::CreatePipelineService do - README.md when: delayed start_in: 4 hours + allow_failure: true - if: $CI_COMMIT_REF_NAME == "master" when: delayed start_in: 1 hour @@ -1960,6 +1998,12 @@ describe Ci::CreatePipelineService do expect(delayed_job.when).to eq('delayed') expect(delayed_job.options[:start_in]).to eq('4 hours') end + + it 'sets allow_failure: for all jobs' do + expect(regular_job.allow_failure).to eq(false) + expect(rules_job.allow_failure).to eq(true) + expect(delayed_job.allow_failure).to eq(true) + end end context 'and if: matches after changes' do @@ -1999,6 +2043,7 @@ describe Ci::CreatePipelineService do - if: $CI_COMMIT_REF_NAME =~ /master/ changes: [README.md] when: on_success + allow_failure: true - if: $CI_COMMIT_REF_NAME =~ /master/ changes: [app.rb] when: manual @@ -2016,6 +2061,7 @@ describe Ci::CreatePipelineService do expect(regular_job).to be_persisted expect(rules_job).to be_persisted expect(rules_job.when).to eq('manual') + expect(rules_job.allow_failure).to eq(false) end end @@ -2040,6 +2086,150 @@ describe Ci::CreatePipelineService do it_behaves_like 'rules jobs are excluded' end end + + context 'with complex if: allow_failure usages' do + let(:config) do + <<-EOY + job-1: + script: "exit 1" + allow_failure: true + rules: + - if: $CI_COMMIT_REF_NAME =~ /master/ + allow_failure: false + + job-2: + script: "exit 1" + allow_failure: true + rules: + - if: $CI_COMMIT_REF_NAME =~ /nonexistant-branch/ + allow_failure: false + + job-3: + script: "exit 1" + rules: + - if: $CI_COMMIT_REF_NAME =~ /nonexistant-branch/ + allow_failure: true + + job-4: + script: "exit 1" + rules: + - if: $CI_COMMIT_REF_NAME =~ /master/ + allow_failure: false + + job-5: + script: "exit 1" + allow_failure: false + rules: + - if: $CI_COMMIT_REF_NAME =~ /master/ + allow_failure: true + + job-6: + script: "exit 1" + rules: + - if: $CI_COMMIT_REF_NAME =~ /nonexistant-branch/ + allow_failure: false + - allow_failure: true + EOY + end + + it 'creates a pipeline' do + expect(pipeline).to be_persisted + expect(build_names).to contain_exactly('job-1', 'job-4', 'job-5', 'job-6') + end + + it 'assigns job:allow_failure values to the builds' do + expect(find_job('job-1').allow_failure).to eq(false) + expect(find_job('job-4').allow_failure).to eq(false) + expect(find_job('job-5').allow_failure).to eq(true) + expect(find_job('job-6').allow_failure).to eq(true) + end + end + + context 'with complex if: allow_failure & when usages' do + let(:config) do + <<-EOY + job-1: + script: "exit 1" + rules: + - if: $CI_COMMIT_REF_NAME =~ /master/ + when: manual + + job-2: + script: "exit 1" + rules: + - if: $CI_COMMIT_REF_NAME =~ /master/ + when: manual + allow_failure: true + + job-3: + script: "exit 1" + allow_failure: true + rules: + - if: $CI_COMMIT_REF_NAME =~ /master/ + when: manual + + job-4: + script: "exit 1" + allow_failure: true + rules: + - if: $CI_COMMIT_REF_NAME =~ /master/ + when: manual + allow_failure: false + + job-5: + script: "exit 1" + rules: + - if: $CI_COMMIT_REF_NAME =~ /nonexistant-branch/ + when: manual + allow_failure: false + - when: always + allow_failure: true + + job-6: + script: "exit 1" + allow_failure: false + rules: + - if: $CI_COMMIT_REF_NAME =~ /master/ + when: manual + + job-7: + script: "exit 1" + allow_failure: false + rules: + - if: $CI_COMMIT_REF_NAME =~ /nonexistant-branch/ + when: manual + - when: :on_failure + allow_failure: true + EOY + end + + it 'creates a pipeline' do + expect(pipeline).to be_persisted + expect(build_names).to contain_exactly( + 'job-1', 'job-2', 'job-3', 'job-4', 'job-5', 'job-6', 'job-7' + ) + end + + it 'assigns job:allow_failure values to the builds' do + expect(find_job('job-1').allow_failure).to eq(false) + expect(find_job('job-2').allow_failure).to eq(true) + expect(find_job('job-3').allow_failure).to eq(true) + expect(find_job('job-4').allow_failure).to eq(false) + expect(find_job('job-5').allow_failure).to eq(true) + expect(find_job('job-6').allow_failure).to eq(false) + expect(find_job('job-7').allow_failure).to eq(true) + end + + it 'assigns job:when values to the builds' do + expect(find_job('job-1').when).to eq('manual') + expect(find_job('job-2').when).to eq('manual') + expect(find_job('job-3').when).to eq('manual') + expect(find_job('job-4').when).to eq('manual') + expect(find_job('job-5').when).to eq('always') + expect(find_job('job-6').when).to eq('manual') + expect(find_job('job-7').when).to eq('on_failure') + end + end end end diff --git a/spec/services/ci/retry_build_service_spec.rb b/spec/services/ci/retry_build_service_spec.rb index 258403db089..8ca9ce86574 100644 --- a/spec/services/ci/retry_build_service_spec.rb +++ b/spec/services/ci/retry_build_service_spec.rb @@ -222,6 +222,28 @@ describe Ci::RetryBuildService do expect { new_build }.to change { Deployment.count }.by(1) end end + + context 'when scheduling_type of build is nil' do + before do + build.update_columns(scheduling_type: nil) + end + + context 'when build has not needs' do + it 'sets scheduling_type as :stage' do + expect(new_build.scheduling_type).to eq('stage') + end + end + + context 'when build has needs' do + before do + create(:ci_build_need, build: build) + end + + it 'sets scheduling_type as :dag' do + expect(new_build.scheduling_type).to eq('dag') + end + end + end end context 'when user does not have ability to execute build' do |