diff options
45 files changed, 774 insertions, 139 deletions
diff --git a/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue b/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue index 34aa15856d2..9eaceb8893c 100644 --- a/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue +++ b/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue @@ -1,5 +1,6 @@ <script> import { mapState, mapGetters, mapActions } from 'vuex'; +import { getParameterByName, parseBoolean } from '~/lib/utils/common_utils'; import Icon from '~/vue_shared/components/icon.vue'; import DiffGutterAvatars from './diff_gutter_avatars.vue'; import { LINE_POSITION_RIGHT } from '../constants'; @@ -98,7 +99,8 @@ export default { return this.showCommentButton && this.hasDiscussions; }, shouldRenderCommentButton() { - return this.isLoggedIn && this.showCommentButton; + const isDiffHead = parseBoolean(getParameterByName('diff_head')); + return !isDiffHead && this.isLoggedIn && this.showCommentButton; }, }, methods: { @@ -130,6 +132,7 @@ export default { </button> <a v-if="lineNumber" + ref="lineNumberRef" :data-linenumber="lineNumber" :href="lineHref" @click="setHighlightedRow(lineCode)" diff --git a/app/assets/javascripts/lib/utils/common_utils.js b/app/assets/javascripts/lib/utils/common_utils.js index a2591180039..dd5a52fe1ce 100644 --- a/app/assets/javascripts/lib/utils/common_utils.js +++ b/app/assets/javascripts/lib/utils/common_utils.js @@ -327,7 +327,10 @@ export const getSelectedFragment = restrictToNode => { documentFragment.originalNodes.push(range.commonAncestorContainer); } } - if (documentFragment.textContent.length === 0) return null; + + if (documentFragment.textContent.length === 0 && documentFragment.children.length === 0) { + return null; + } return documentFragment; }; diff --git a/app/models/ci/bridge.rb b/app/models/ci/bridge.rb index 3726c05acd8..bb22cc3e039 100644 --- a/app/models/ci/bridge.rb +++ b/app/models/ci/bridge.rb @@ -8,7 +8,6 @@ module Ci include Importable include AfterCommitQueue include HasRef - include Gitlab::Utils::StrongMemoize InvalidBridgeTypeError = Class.new(StandardError) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index c23b2d81ce3..b0502f7a26e 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -10,7 +10,6 @@ module Ci include ObjectStorage::BackgroundMove include Presentable include Importable - include Gitlab::Utils::StrongMemoize include HasRef include IgnorableColumns diff --git a/app/models/ci/processable.rb b/app/models/ci/processable.rb index 6c4b271cd2c..95fb75688a9 100644 --- a/app/models/ci/processable.rb +++ b/app/models/ci/processable.rb @@ -2,10 +2,14 @@ module Ci class Processable < ::CommitStatus + include Gitlab::Utils::StrongMemoize + has_many :needs, class_name: 'Ci::BuildNeed', foreign_key: :build_id, inverse_of: :build accepts_nested_attributes_for :needs + enum scheduling_type: { stage: 0, dag: 1 }, _prefix: true + scope :preload_needs, -> { preload(:needs) } def self.select_with_aggregated_needs(project) @@ -23,6 +27,7 @@ module Ci end validates :type, presence: true + validates :scheduling_type, presence: true, on: :create, if: :validate_scheduling_type? def aggregated_needs_names read_attribute(:aggregated_needs_names) @@ -47,5 +52,19 @@ module Ci def scoped_variables_hash raise NotImplementedError end + + # scheduling_type column of previous builds/bridges have not been populated, + # so we calculate this value on runtime when we need it. + def find_legacy_scheduling_type + strong_memoize(:find_legacy_scheduling_type) do + needs.exists? ? :dag : :stage + end + end + + private + + def validate_scheduling_type? + !importing? && Feature.enabled?(:validate_scheduling_type_of_processables, project) + end end end diff --git a/app/models/clusters/applications/elastic_stack.rb b/app/models/clusters/applications/elastic_stack.rb index de7d3ab3fdb..ce42bc65579 100644 --- a/app/models/clusters/applications/elastic_stack.rb +++ b/app/models/clusters/applications/elastic_stack.rb @@ -30,7 +30,8 @@ module Clusters version: VERSION, rbac: cluster.platform_kubernetes_rbac?, chart: chart, - files: files + files: files, + postinstall: post_install_script ) end @@ -43,6 +44,10 @@ module Clusters ) end + def files + super.merge('wait-for-elasticsearch.sh': File.read("#{Rails.root}/vendor/elastic_stack/wait-for-elasticsearch.sh")) + end + def elasticsearch_client strong_memoize(:elasticsearch_client) do next unless kube_client @@ -69,10 +74,16 @@ module Clusters private + def post_install_script + [ + "timeout -t60 sh /data/helm/elastic-stack/config/wait-for-elasticsearch.sh http://elastic-stack-elasticsearch-client:9200" + ] + end + def post_delete_script [ Gitlab::Kubernetes::KubectlCmd.delete("pvc", "--selector", "release=elastic-stack") - ].compact + ] end def kube_client diff --git a/app/services/ci/retry_build_service.rb b/app/services/ci/retry_build_service.rb index 1f00d54b6a7..838ed789155 100644 --- a/app/services/ci/retry_build_service.rb +++ b/app/services/ci/retry_build_service.rb @@ -5,7 +5,7 @@ module Ci CLONE_ACCESSORS = %i[pipeline project ref tag options name allow_failure stage stage_id stage_idx trigger_request yaml_variables when environment coverage_regex - description tag_list protected needs resource_group].freeze + description tag_list protected needs resource_group scheduling_type].freeze def execute(build) reprocess!(build).tap do |new_build| @@ -27,9 +27,10 @@ module Ci attributes = CLONE_ACCESSORS.map do |attribute| [attribute, build.public_send(attribute)] # rubocop:disable GitlabSecurity/PublicSend - end + end.to_h - attributes.push([:user, current_user]) + attributes[:user] = current_user + attributes[:scheduling_type] ||= build.find_legacy_scheduling_type Ci::Build.transaction do # mark all other builds of that name as retried @@ -49,7 +50,7 @@ module Ci private def create_build!(attributes) - build = project.builds.new(Hash[attributes]) + build = project.builds.new(attributes) build.deployment = ::Gitlab::Ci::Pipeline::Seed::Deployment.new(build).to_resource build.retried = false build.save! diff --git a/app/views/shared/boards/components/_board.html.haml b/app/views/shared/boards/components/_board.html.haml index 1c2ba6639cf..3db96db73ce 100644 --- a/app/views/shared/boards/components/_board.html.haml +++ b/app/views/shared/boards/components/_board.html.haml @@ -29,7 +29,7 @@ ":title" => '(list.assignee && list.assignee.username || "")' } @{{ list.assignee.username }} - %span.has-tooltip.badge.color-label.title{ "v-if": "list.type === \"label\"", + %span.has-tooltip.badge.color-label.title.d-inline-block.mw-100.text-truncate.align-middle{ "v-if": "list.type === \"label\"", ":title" => '(list.label ? list.label.description : "")', data: { container: "body", placement: "bottom" }, ":style" => "{ backgroundColor: (list.label && list.label.color ? list.label.color : null), color: (list.label && list.label.textColor ? list.label.textColor : \"#2e2e2e\") }" } diff --git a/changelogs/unreleased/24020-copy-markdown-from-comments.yml b/changelogs/unreleased/24020-copy-markdown-from-comments.yml new file mode 100644 index 00000000000..c7d54dead0c --- /dev/null +++ b/changelogs/unreleased/24020-copy-markdown-from-comments.yml @@ -0,0 +1,5 @@ +--- +title: Fix copy markdown with elements with no text content +merge_request: 24020 +author: +type: fixed diff --git a/changelogs/unreleased/30235-support-allow-failure-for-ci-rules.yml b/changelogs/unreleased/30235-support-allow-failure-for-ci-rules.yml new file mode 100644 index 00000000000..ce026b3392a --- /dev/null +++ b/changelogs/unreleased/30235-support-allow-failure-for-ci-rules.yml @@ -0,0 +1,5 @@ +--- +title: Implement support of allow_failure keyword for CI rules +merge_request: 24605 +author: +type: added diff --git a/changelogs/unreleased/ak-wait-for-green-es.yml b/changelogs/unreleased/ak-wait-for-green-es.yml new file mode 100644 index 00000000000..254af698017 --- /dev/null +++ b/changelogs/unreleased/ak-wait-for-green-es.yml @@ -0,0 +1,5 @@ +--- +title: Wait for elasticsearch to be green on install +merge_request: 24489 +author: +type: added diff --git a/changelogs/unreleased/jdb-minimal-merge-ref-diff.yml b/changelogs/unreleased/jdb-minimal-merge-ref-diff.yml new file mode 100644 index 00000000000..1066de2821e --- /dev/null +++ b/changelogs/unreleased/jdb-minimal-merge-ref-diff.yml @@ -0,0 +1,5 @@ +--- +title: Hide comment button if on diff HEAD +merge_request: 24207 +author: +type: changed diff --git a/changelogs/unreleased/ss-fix-text-truncated-on-board-list.yml b/changelogs/unreleased/ss-fix-text-truncated-on-board-list.yml new file mode 100644 index 00000000000..1958be19d91 --- /dev/null +++ b/changelogs/unreleased/ss-fix-text-truncated-on-board-list.yml @@ -0,0 +1,5 @@ +--- +title: Add styles for board list labels when text is too long +merge_request: 24627 +author: +type: fixed diff --git a/db/fixtures/development/14_pipelines.rb b/db/fixtures/development/14_pipelines.rb index 417a68d6ad7..7a9b97dfefa 100644 --- a/db/fixtures/development/14_pipelines.rb +++ b/db/fixtures/development/14_pipelines.rb @@ -165,9 +165,10 @@ class Gitlab::Seeder::Pipelines end def job_attributes(pipeline, opts) - { name: 'test build', stage: 'test', stage_idx: stage_index(opts[:stage]), + { + name: 'test build', stage: 'test', stage_idx: stage_index(opts[:stage]), ref: pipeline.ref, tag: false, user: build_user, project: @project, pipeline: pipeline, - created_at: Time.now, updated_at: Time.now + scheduling_type: :stage, created_at: Time.now, updated_at: Time.now }.merge(opts) end diff --git a/db/migrate/20191223124940_add_scheduling_type_to_ci_builds.rb b/db/migrate/20191223124940_add_scheduling_type_to_ci_builds.rb new file mode 100644 index 00000000000..0cb42cdc328 --- /dev/null +++ b/db/migrate/20191223124940_add_scheduling_type_to_ci_builds.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class AddSchedulingTypeToCiBuilds < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def change + add_column :ci_builds, :scheduling_type, :integer, limit: 2 + end +end diff --git a/db/schema.rb b/db/schema.rb index a6c2edabeaf..07ff6ff0828 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -677,6 +677,7 @@ ActiveRecord::Schema.define(version: 2020_02_11_152410) do t.bigint "resource_group_id" t.datetime_with_timezone "waiting_for_resource_at" t.boolean "processed" + t.integer "scheduling_type", limit: 2 t.index ["artifacts_expire_at"], name: "index_ci_builds_on_artifacts_expire_at", where: "(artifacts_file <> ''::text)" t.index ["auto_canceled_by_id"], name: "index_ci_builds_on_auto_canceled_by_id" t.index ["commit_id", "artifacts_expire_at", "id"], name: "index_ci_builds_on_commit_id_and_artifacts_expireatandidpartial", where: "(((type)::text = 'Ci::Build'::text) AND ((retried = false) OR (retried IS NULL)) AND ((name)::text = ANY (ARRAY[('sast'::character varying)::text, ('dependency_scanning'::character varying)::text, ('sast:container'::character varying)::text, ('container_scanning'::character varying)::text, ('dast'::character varying)::text])))" diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index a516a3f56f7..1c0e6545bda 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -851,7 +851,7 @@ In this example, if the first rule: - Matches, the job will be given the `when:always` attribute. - Does not match, the second and third rules will be evaluated sequentially until a match is found. That is, the job will be given either the: - - `when: manual` attribute if the second rule matches. + - `when: manual` attribute if the second rule matches. **The stage will not complete until this manual job is triggered and completes successfully.** - `when: on_success` attribute if the second rule does not match. The third rule will always match when reached because it has no conditional clauses. @@ -937,6 +937,25 @@ NOTE: **Note:** For performance reasons, using `exists` with patterns is limited to 10000 checks. After the 10000th check, rules with patterned globs will always match. +#### `rules:allow_failure` + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/issues/30235) in GitLab 12.8. + +You can use [`allow_failure: true`](#allow_failure) within `rules:` to allow a job to fail, or a manual job to +wait for action, without stopping the pipeline itself. All jobs using `rules:` default to `allow_failure: false` +if `allow_failure:` is not defined. + +```yaml +job: + script: "echo Hello, Rules!" + rules: + - if: '$CI_MERGE_REQUEST_TARGET_BRANCH_NAME == "master"' + when: manual + allow_failure: true +``` + +In this example, if the first rule matches, then the job will have `when: manual` and `allow_failure: true`. + #### Complex rule clauses To conjoin `if`, `changes`, and `exists` clauses with an AND, use them in the @@ -976,6 +995,7 @@ The only job attributes currently set by `rules` are: - `when`. - `start_in`, if `when` is set to `delayed`. +- `allow_failure`. A job will be included in a pipeline if `when` is evaluated to any value except `never`. diff --git a/doc/user/clusters/applications.md b/doc/user/clusters/applications.md index 1794f707abd..60ce03c2fc4 100644 --- a/doc/user/clusters/applications.md +++ b/doc/user/clusters/applications.md @@ -464,8 +464,8 @@ chart is used to install this application with a file. NOTE: **Note:** -The chart will deploy 4 Elasticsearch nodes: 2 masters, 1 data and 1 client node, -with resource requests totalling 0.1 CPU and 3GB RAM. Each data node requests 1.5GB of memory, +The chart will deploy 5 Elasticsearch nodes: 2 masters, 2 data and 1 client node, +with resource requests totalling 0.125 CPU and 4.5GB RAM. Each data node requests 1.5GB of memory, which makes it incompatible with clusters of `f1-micro` and `g1-small` instance types. ## Install using GitLab CI (alpha) diff --git a/doc/user/group/index.md b/doc/user/group/index.md index a5383da2e1f..6b76e070c41 100644 --- a/doc/user/group/index.md +++ b/doc/user/group/index.md @@ -241,9 +241,10 @@ and give all group members access to the project at once. Alternatively, you can [lock the sharing with group feature](#share-with-group-lock). -## Sharing a group with another group +## Sharing a group with another group **(CORE ONLY)** > [Introduced](https://gitlab.com/gitlab-org/gitlab/issues/18328) in GitLab 12.7. +> This feature has been [disabled on GitLab.com](https://gitlab.com/gitlab-com/gl-infra/production/issues/1635). Similarly to [sharing a project with a group](#sharing-a-project-with-a-group), you can share a group with another group to give direct group members access diff --git a/lib/gitlab/ci/build/rules.rb b/lib/gitlab/ci/build/rules.rb index c705b6f86c7..a500a0cc35d 100644 --- a/lib/gitlab/ci/build/rules.rb +++ b/lib/gitlab/ci/build/rules.rb @@ -6,11 +6,12 @@ module Gitlab class Rules include ::Gitlab::Utils::StrongMemoize - Result = Struct.new(:when, :start_in) do + Result = Struct.new(:when, :start_in, :allow_failure) do def build_attributes { when: self.when, - options: { start_in: start_in }.compact + options: { start_in: start_in }.compact, + allow_failure: allow_failure }.compact end @@ -30,7 +31,8 @@ module Gitlab elsif matched_rule = match_rule(pipeline, context) Result.new( matched_rule.attributes[:when] || @default_when, - matched_rule.attributes[:start_in] + matched_rule.attributes[:start_in], + matched_rule.attributes[:allow_failure] ) else Result.new('never') diff --git a/lib/gitlab/ci/config/entry/bridge.rb b/lib/gitlab/ci/config/entry/bridge.rb index 7a6840218e1..c0247dca73d 100644 --- a/lib/gitlab/ci/config/entry/bridge.rb +++ b/lib/gitlab/ci/config/entry/bridge.rb @@ -132,7 +132,8 @@ module Gitlab variables: (variables_value if variables_defined?), rules: (rules_value if has_rules?), only: only_value, - except: except_value }.compact + except: except_value, + scheduling_type: needs_defined? && !bridge_needs ? :dag : :stage }.compact end def bridge_needs diff --git a/lib/gitlab/ci/config/entry/job.rb b/lib/gitlab/ci/config/entry/job.rb index 124581c961f..ffc8cb887e8 100644 --- a/lib/gitlab/ci/config/entry/job.rb +++ b/lib/gitlab/ci/config/entry/job.rb @@ -258,7 +258,8 @@ module Gitlab after_script: after_script_value, ignore: ignored?, needs: needs_defined? ? needs_value : nil, - resource_group: resource_group } + resource_group: resource_group, + scheduling_type: needs_defined? ? :dag : :stage } end end end diff --git a/lib/gitlab/ci/config/entry/rules/rule.rb b/lib/gitlab/ci/config/entry/rules/rule.rb index 59e0ef583ae..8ffd49b8a93 100644 --- a/lib/gitlab/ci/config/entry/rules/rule.rb +++ b/lib/gitlab/ci/config/entry/rules/rule.rb @@ -9,10 +9,10 @@ module Gitlab include ::Gitlab::Config::Entry::Attributable CLAUSES = %i[if changes exists].freeze - ALLOWED_KEYS = %i[if changes exists when start_in].freeze + ALLOWED_KEYS = %i[if changes exists when start_in allow_failure].freeze ALLOWABLE_WHEN = %w[on_success on_failure always never manual delayed].freeze - attributes :if, :changes, :exists, :when, :start_in + attributes :if, :changes, :exists, :when, :start_in, :allow_failure validations do validates :config, presence: true @@ -26,6 +26,7 @@ module Gitlab validates :if, expression: true validates :changes, :exists, array_of_strings: true, length: { maximum: 50 } validates :when, allowed_values: { in: ALLOWABLE_WHEN } + validates :allow_failure, boolean: true end validate do diff --git a/lib/gitlab/ci/yaml_processor.rb b/lib/gitlab/ci/yaml_processor.rb index 080a8ac107d..ae3ff4a51e2 100644 --- a/lib/gitlab/ci/yaml_processor.rb +++ b/lib/gitlab/ci/yaml_processor.rb @@ -65,6 +65,7 @@ module Gitlab rules: job[:rules], cache: job[:cache], resource_group_key: job[:resource_group], + scheduling_type: job[:scheduling_type], options: { image: job[:image], services: job[:services], 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 diff --git a/vendor/elastic_stack/values.yaml b/vendor/elastic_stack/values.yaml index 8b71e5b2c2c..9355a9b6b81 100644 --- a/vendor/elastic_stack/values.yaml +++ b/vendor/elastic_stack/values.yaml @@ -8,7 +8,7 @@ elasticsearch: client: replicas: 1 data: - replicas: 1 + replicas: 2 kibana: enabled: false diff --git a/vendor/elastic_stack/wait-for-elasticsearch.sh b/vendor/elastic_stack/wait-for-elasticsearch.sh new file mode 100755 index 00000000000..1423af2e10b --- /dev/null +++ b/vendor/elastic_stack/wait-for-elasticsearch.sh @@ -0,0 +1,23 @@ +#!/bin/bash +# http://redsymbol.net/articles/unofficial-bash-strict-mode/ +IFS=$'\n\t' +set -euo pipefail + +HOST="$1" + +printf 'Waiting for ES to be reachable ...' +until $(wget -O- -q "$HOST" &>/dev/null); do + printf '.' + sleep 1 +done +echo " OK!" + +printf 'Waiting for ES to be healthy ...' +while : ; do + HEALTH="$(wget -O- -q "$HOST/_cat/health?h=status" 2> /dev/null)" + HEALTH="$(echo "$HEALTH" | sed -r 's/^[[:space:]]+|[[:space:]]+$//g')" # trim whitespace (otherwise we'll have "green ") + ([ "$HEALTH" != "green" ] && printf '.' && sleep 1) || break +done +echo " OK!" + +echo "Elastic Search is up!" |