diff options
author | Alessio Caiazza <acaiazza@gitlab.com> | 2018-07-06 12:25:42 +0300 |
---|---|---|
committer | Alessio Caiazza <acaiazza@gitlab.com> | 2018-07-06 12:25:42 +0300 |
commit | ca3ea76c350cad18d86c5e1f6fb17782a8cdafb2 (patch) | |
tree | 3562a633cc648b5580b80fb3e0287ec586a14ef1 | |
parent | 7240d175fc70b4988b351c1733a3b4e33ecb7947 (diff) | |
parent | 68d138e85e3263959700d16eab7d9ab3e883f7f8 (diff) |
Merge branch 'master' into 11-1-stable-prepare-rc5
90 files changed, 1124 insertions, 235 deletions
diff --git a/README.md b/README.md index 295e1d6c6cc..77f03b791f2 100644 --- a/README.md +++ b/README.md @@ -120,6 +120,10 @@ All documentation can be found on [docs.gitlab.com/ce/](https://docs.gitlab.com/ Please see [Getting help for GitLab](https://about.gitlab.com/getting-help/) on our website for the many options to get help. +## Why? + +[Read here](https://about.gitlab.com/why/) + ## Is it any good? [Yes](https://news.ycombinator.com/item?id=3067434) diff --git a/app/assets/javascripts/diffs/components/diff_file_header.vue b/app/assets/javascripts/diffs/components/diff_file_header.vue index fba1d1af7cd..a8e8732053b 100644 --- a/app/assets/javascripts/diffs/components/diff_file_header.vue +++ b/app/assets/javascripts/diffs/components/diff_file_header.vue @@ -2,6 +2,7 @@ import _ from 'underscore'; import ClipboardButton from '~/vue_shared/components/clipboard_button.vue'; import Icon from '~/vue_shared/components/icon.vue'; +import FileIcon from '~/vue_shared/components/file_icon.vue'; import Tooltip from '~/vue_shared/directives/tooltip'; import { truncateSha } from '~/lib/utils/text_utility'; import { __, s__, sprintf } from '~/locale'; @@ -12,6 +13,7 @@ export default { ClipboardButton, EditButton, Icon, + FileIcon, }, directives: { Tooltip, @@ -139,18 +141,20 @@ export default { :name="collapseIcon" :size="16" aria-hidden="true" - class="diff-toggle-caret" + class="diff-toggle-caret append-right-5" @click.stop="handleToggle" /> <a ref="titleWrapper" :href="titleLink" + class="append-right-4" > - <i - :class="`fa-${icon}`" - class="fa fa-fw" + <file-icon + :file-name="filePath" + :size="18" aria-hidden="true" - ></i> + css-classes="js-file-icon append-right-5" + /> <span v-if="diffFile.renamedFile"> <strong v-tooltip diff --git a/app/assets/javascripts/diffs/components/parallel_diff_view.vue b/app/assets/javascripts/diffs/components/parallel_diff_view.vue index d7280338b68..52561e197e6 100644 --- a/app/assets/javascripts/diffs/components/parallel_diff_view.vue +++ b/app/assets/javascripts/diffs/components/parallel_diff_view.vue @@ -24,19 +24,21 @@ export default { ...mapGetters(['commit']), parallelDiffLines() { return this.diffLines.map(line => { + const parallelLine = Object.assign({}, line); + if (line.left) { - Object.assign(line, { left: trimFirstCharOfLineContent(line.left) }); + parallelLine.left = trimFirstCharOfLineContent(line.left); } else { - Object.assign(line, { left: { type: EMPTY_CELL_TYPE } }); + parallelLine.left = { type: EMPTY_CELL_TYPE }; } if (line.right) { - Object.assign(line, { right: trimFirstCharOfLineContent(line.right) }); + parallelLine.right = trimFirstCharOfLineContent(line.right); } else { - Object.assign(line, { right: { type: EMPTY_CELL_TYPE } }); + parallelLine.right = { type: EMPTY_CELL_TYPE }; } - return line; + return parallelLine; }); }, diffLinesLength() { diff --git a/app/assets/javascripts/diffs/store/utils.js b/app/assets/javascripts/diffs/store/utils.js index da7ae16aaf1..d9589baa76e 100644 --- a/app/assets/javascripts/diffs/store/utils.js +++ b/app/assets/javascripts/diffs/store/utils.js @@ -155,18 +155,21 @@ export function addContextLines(options) { } } -export function trimFirstCharOfLineContent(line) { - if (!line.richText) { - return line; - } - - const firstChar = line.richText.charAt(0); - - if (firstChar === ' ' || firstChar === '+' || firstChar === '-') { - Object.assign(line, { - richText: line.richText.substring(1), - }); +/** + * Trims the first char of the `richText` property when it's either a space or a diff symbol. + * @param {Object} line + * @returns {Object} + */ +export function trimFirstCharOfLineContent(line = {}) { + const parsedLine = Object.assign({}, line); + + if (line.richText) { + const firstChar = parsedLine.richText.charAt(0); + + if (firstChar === ' ' || firstChar === '+' || firstChar === '-') { + parsedLine.richText = line.richText.substring(1); + } } - return line; + return parsedLine; } diff --git a/app/assets/javascripts/notes/components/diff_with_note.vue b/app/assets/javascripts/notes/components/diff_with_note.vue index d321f2ce15e..9c2908c477e 100644 --- a/app/assets/javascripts/notes/components/diff_with_note.vue +++ b/app/assets/javascripts/notes/components/diff_with_note.vue @@ -1,89 +1,94 @@ <script> -import { mapState, mapActions } from 'vuex'; -import imageDiffHelper from '~/image_diff/helpers/index'; -import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils'; -import DiffFileHeader from '~/diffs/components/diff_file_header.vue'; -import SkeletonLoadingContainer from '~/vue_shared/components/skeleton_loading_container.vue'; -import { trimFirstCharOfLineContent } from '~/diffs/store/utils'; + import { mapState, mapActions } from 'vuex'; + import imageDiffHelper from '~/image_diff/helpers/index'; + import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils'; + import DiffFileHeader from '~/diffs/components/diff_file_header.vue'; + import SkeletonLoadingContainer from '~/vue_shared/components/skeleton_loading_container.vue'; + import { trimFirstCharOfLineContent } from '~/diffs/store/utils'; -export default { - components: { - DiffFileHeader, - SkeletonLoadingContainer, - }, - props: { - discussion: { - type: Object, - required: true, + export default { + components: { + DiffFileHeader, + SkeletonLoadingContainer, }, - }, - data() { - return { - error: false, - }; - }, - computed: { - ...mapState({ - noteableData: state => state.notes.noteableData, - }), - hasTruncatedDiffLines() { - return this.discussion.truncatedDiffLines && this.discussion.truncatedDiffLines.length !== 0; + props: { + discussion: { + type: Object, + required: true, + }, }, - isDiscussionsExpanded() { - return true; // TODO: @fatihacet - Fix this. + data() { + return { + error: false, + }; }, - isCollapsed() { - return this.diffFile.collapsed || false; - }, - isImageDiff() { - return !this.diffFile.text; - }, - diffFileClass() { - const { text } = this.diffFile; - return text ? 'text-file' : 'js-image-file'; - }, - diffFile() { - return convertObjectPropsToCamelCase(this.discussion.diffFile, { deep: true }); - }, - imageDiffHtml() { - return this.discussion.imageDiffHtml; - }, - currentUser() { - return this.noteableData.current_user; - }, - userColorScheme() { - return window.gon.user_color_scheme; - }, - normalizedDiffLines() { - const lines = this.discussion.truncatedDiffLines || []; + computed: { + ...mapState({ + noteableData: state => state.notes.noteableData, + }), + hasTruncatedDiffLines() { + return this.discussion.truncatedDiffLines && + this.discussion.truncatedDiffLines.length !== 0; + }, + isDiscussionsExpanded() { + return true; // TODO: @fatihacet - Fix this. + }, + isCollapsed() { + return this.diffFile.collapsed || false; + }, + isImageDiff() { + return !this.diffFile.text; + }, + diffFileClass() { + const { text } = this.diffFile; + return text ? 'text-file' : 'js-image-file'; + }, + diffFile() { + return convertObjectPropsToCamelCase(this.discussion.diffFile, { deep: true }); + }, + imageDiffHtml() { + return this.discussion.imageDiffHtml; + }, + currentUser() { + return this.noteableData.current_user; + }, + userColorScheme() { + return window.gon.user_color_scheme; + }, + normalizedDiffLines() { + if (this.discussion.truncatedDiffLines) { + return this.discussion.truncatedDiffLines.map(line => + trimFirstCharOfLineContent(convertObjectPropsToCamelCase(line)), + ); + } - return lines.map(line => trimFirstCharOfLineContent(convertObjectPropsToCamelCase(line))); + return []; + }, }, - }, - mounted() { - if (this.isImageDiff) { - const canCreateNote = false; - const renderCommentBadge = true; - imageDiffHelper.initImageDiff(this.$refs.fileHolder, canCreateNote, renderCommentBadge); - } else if (!this.hasTruncatedDiffLines) { - this.fetchDiff(); - } - }, - methods: { - ...mapActions(['fetchDiscussionDiffLines']), - rowTag(html) { - return html.outerHTML ? 'tr' : 'template'; + mounted() { + if (this.isImageDiff) { + const canCreateNote = false; + const renderCommentBadge = true; + imageDiffHelper.initImageDiff(this.$refs.fileHolder, canCreateNote, renderCommentBadge); + } else if (!this.hasTruncatedDiffLines) { + this.fetchDiff(); + } }, - fetchDiff() { - this.error = false; - this.fetchDiscussionDiffLines(this.discussion) - .then(this.highlight) - .catch(() => { - this.error = true; - }); + methods: { + ...mapActions(['fetchDiscussionDiffLines']), + rowTag(html) { + return html.outerHTML ? 'tr' : 'template'; + }, + fetchDiff() { + this.error = false; + this.fetchDiscussionDiffLines(this.discussion) + .then(this.highlight) + .catch(() => { + this.error = true; + }); + }, }, - }, -}; + }; </script> <template> diff --git a/app/assets/javascripts/notes/stores/getters.js b/app/assets/javascripts/notes/stores/getters.js index a5518383d44..5c65e1c3bb5 100644 --- a/app/assets/javascripts/notes/stores/getters.js +++ b/app/assets/javascripts/notes/stores/getters.js @@ -85,9 +85,9 @@ export const allDiscussions = (state, getters) => { export const resolvedDiscussionsById = state => { const map = {}; - state.discussions.forEach(n => { + state.discussions.filter(d => d.resolvable).forEach(n => { if (n.notes) { - const resolved = n.notes.every(note => note.resolved && !note.system); + const resolved = n.notes.filter(note => note.resolvable).every(note => note.resolved); if (resolved) { map[n.id] = n; diff --git a/app/assets/javascripts/pages/projects/jobs/terminal/index.js b/app/assets/javascripts/pages/projects/jobs/terminal/index.js new file mode 100644 index 00000000000..7129e24cee1 --- /dev/null +++ b/app/assets/javascripts/pages/projects/jobs/terminal/index.js @@ -0,0 +1,3 @@ +import initTerminal from '~/terminal/'; + +document.addEventListener('DOMContentLoaded', initTerminal); diff --git a/app/assets/javascripts/vue_shared/components/markdown/header.vue b/app/assets/javascripts/vue_shared/components/markdown/header.vue index 83171ae50b8..8c22f3f6536 100644 --- a/app/assets/javascripts/vue_shared/components/markdown/header.vue +++ b/app/assets/javascripts/vue_shared/components/markdown/header.vue @@ -29,8 +29,8 @@ methods: { isValid(form) { return !form || - form.find('.js-vue-markdown-field').length || - $(this.$el).closest('form') === form[0]; + form.find('.js-vue-markdown-field').length && + $(this.$el).closest('form')[0] === form[0]; }, previewMarkdownTab(event, form) { diff --git a/app/assets/stylesheets/pages/diff.scss b/app/assets/stylesheets/pages/diff.scss index a90a9c6e486..7e89f8998fb 100644 --- a/app/assets/stylesheets/pages/diff.scss +++ b/app/assets/stylesheets/pages/diff.scss @@ -15,7 +15,7 @@ } svg { - vertical-align: text-bottom; + vertical-align: middle; } } diff --git a/app/controllers/concerns/preview_markdown.rb b/app/controllers/concerns/preview_markdown.rb index 90bb7a87b45..7ac63c914fa 100644 --- a/app/controllers/concerns/preview_markdown.rb +++ b/app/controllers/concerns/preview_markdown.rb @@ -10,6 +10,7 @@ module PreviewMarkdown when 'wikis' then { pipeline: :wiki, project_wiki: @project_wiki, page_slug: params[:id] } when 'snippets' then { skip_project_check: true } when 'groups' then { group: group } + when 'projects' then { issuable_state_filter_enabled: true } else {} end diff --git a/app/controllers/dashboard/projects_controller.rb b/app/controllers/dashboard/projects_controller.rb index 4d4ac025f8c..ccfcbbdc776 100644 --- a/app/controllers/dashboard/projects_controller.rb +++ b/app/controllers/dashboard/projects_controller.rb @@ -7,7 +7,7 @@ class Dashboard::ProjectsController < Dashboard::ApplicationController skip_cross_project_access_check :index, :starred def index - @projects = load_projects(params.merge(non_public: true)).page(params[:page]) + @projects = load_projects(params.merge(non_public: true)) respond_to do |format| format.html @@ -25,7 +25,7 @@ class Dashboard::ProjectsController < Dashboard::ApplicationController def starred @projects = load_projects(params.merge(starred: true)) - .includes(:forked_from_project, :tags).page(params[:page]) + .includes(:forked_from_project, :tags) @groups = [] @@ -51,6 +51,7 @@ class Dashboard::ProjectsController < Dashboard::ApplicationController .new(params: finder_params, current_user: current_user) .execute .includes(:route, :creator, namespace: [:route, :owner]) + .page(finder_params[:page]) prepare_projects_for_rendering(projects) end diff --git a/app/controllers/projects/jobs_controller.rb b/app/controllers/projects/jobs_controller.rb index 02cac862c3d..e69faae754a 100644 --- a/app/controllers/projects/jobs_controller.rb +++ b/app/controllers/projects/jobs_controller.rb @@ -2,11 +2,12 @@ class Projects::JobsController < Projects::ApplicationController include SendFileUpload before_action :build, except: [:index, :cancel_all] - before_action :authorize_read_build!, - only: [:index, :show, :status, :raw, :trace] + before_action :authorize_read_build! before_action :authorize_update_build!, except: [:index, :show, :status, :raw, :trace, :cancel_all, :erase] before_action :authorize_erase_build!, only: [:erase] + before_action :authorize_use_build_terminal!, only: [:terminal, :terminal_workhorse_authorize] + before_action :verify_api_request!, only: :terminal_websocket_authorize layout 'project' @@ -134,6 +135,15 @@ class Projects::JobsController < Projects::ApplicationController end end + def terminal + end + + # GET .../terminal.ws : implemented in gitlab-workhorse + def terminal_websocket_authorize + set_workhorse_internal_api_content_type + render json: Gitlab::Workhorse.terminal_websocket(@build.terminal_specification) + end + private def authorize_update_build! @@ -144,6 +154,14 @@ class Projects::JobsController < Projects::ApplicationController return access_denied! unless can?(current_user, :erase_build, build) end + def authorize_use_build_terminal! + return access_denied! unless can?(current_user, :create_build_terminal, build) + end + + def verify_api_request! + Gitlab::Workhorse.verify_api_request!(request.headers) + end + def raw_send_params { type: 'text/plain; charset=utf-8', disposition: 'inline' } end diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index c7a434ea092..b0f381db5ab 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -177,6 +177,7 @@ module ProjectsHelper controller.action_name, Gitlab::CurrentSettings.cache_key, "cross-project:#{can?(current_user, :read_cross_project)}", + max_project_member_access_cache_key(project), 'v2.6' ] diff --git a/app/helpers/users_helper.rb b/app/helpers/users_helper.rb index ce9373f5883..4d17b22a4a1 100644 --- a/app/helpers/users_helper.rb +++ b/app/helpers/users_helper.rb @@ -31,6 +31,14 @@ module UsersHelper current_user_menu_items.include?(item) end + def max_project_member_access(project) + current_user&.max_member_access_for_project(project.id) || Gitlab::Access::NO_ACCESS + end + + def max_project_member_access_cache_key(project) + "access:#{max_project_member_access(project)}" + end + private def get_profile_tabs diff --git a/app/models/board.rb b/app/models/board.rb index 3cede6fc99a..bb6bb753daf 100644 --- a/app/models/board.rb +++ b/app/models/board.rb @@ -26,4 +26,8 @@ class Board < ActiveRecord::Base def closed_list lists.merge(List.closed).take end + + def scoped? + false + end end diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 41446946a5e..19949f83351 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -27,7 +27,13 @@ module Ci has_one :job_artifacts_trace, -> { where(file_type: Ci::JobArtifact.file_types[:trace]) }, class_name: 'Ci::JobArtifact', inverse_of: :job, foreign_key: :job_id has_one :metadata, class_name: 'Ci::BuildMetadata' + has_one :runner_session, class_name: 'Ci::BuildRunnerSession', validate: true, inverse_of: :build + + accepts_nested_attributes_for :runner_session + delegate :timeout, to: :metadata, prefix: true, allow_nil: true + delegate :url, to: :runner_session, prefix: true, allow_nil: true + delegate :terminal_specification, to: :runner_session, allow_nil: true delegate :gitlab_deploy_token, to: :project ## @@ -174,6 +180,10 @@ module Ci after_transition pending: :running do |build| build.ensure_metadata.update_timeout_state end + + after_transition running: any do |build| + Ci::BuildRunnerSession.where(build: build).delete_all + end end def ensure_metadata @@ -376,6 +386,10 @@ module Ci trace.exist? end + def has_old_trace? + old_trace.present? + end + def trace=(data) raise NotImplementedError end @@ -385,6 +399,8 @@ module Ci end def erase_old_trace! + return unless has_old_trace? + update_column(:trace, nil) end @@ -584,6 +600,10 @@ module Ci super(options).merge(when: read_attribute(:when)) end + def has_terminal? + running? && runner_session_url.present? + end + private def update_artifacts_size diff --git a/app/models/ci/build_runner_session.rb b/app/models/ci/build_runner_session.rb new file mode 100644 index 00000000000..6f3be31d8e1 --- /dev/null +++ b/app/models/ci/build_runner_session.rb @@ -0,0 +1,25 @@ +module Ci + # The purpose of this class is to store Build related runner session. + # Data will be removed after transitioning from running to any state. + class BuildRunnerSession < ActiveRecord::Base + extend Gitlab::Ci::Model + + self.table_name = 'ci_builds_runner_session' + + belongs_to :build, class_name: 'Ci::Build', inverse_of: :runner_session + + validates :build, presence: true + validates :url, url: { protocols: %w(https) } + + def terminal_specification + return {} unless url.present? + + { + subprotocols: ['terminal.gitlab.com'].freeze, + url: "#{url}/exec".sub("https://", "wss://"), + headers: { Authorization: authorization.presence }.compact, + ca_pem: certificate.presence + } + end + end +end diff --git a/app/models/project_services/kubernetes_service.rb b/app/models/project_services/kubernetes_service.rb index ddd4026019b..722642f6da7 100644 --- a/app/models/project_services/kubernetes_service.rb +++ b/app/models/project_services/kubernetes_service.rb @@ -240,7 +240,7 @@ class KubernetesService < DeploymentService end def deprecation_validation - return if active_changed?(from: true, to: false) + return if active_changed?(from: true, to: false) || (new_record? && !active?) if deprecated? errors[:base] << deprecation_message diff --git a/app/models/service.rb b/app/models/service.rb index 1d259bcfec7..ad835293b46 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -281,9 +281,9 @@ class Service < ActiveRecord::Base def self.build_from_template(project_id, template) service = template.dup - service.active = false unless service.valid? service.template = false service.project_id = project_id + service.active = false if service.active? && !service.valid? service end diff --git a/app/policies/ci/build_policy.rb b/app/policies/ci/build_policy.rb index 1c0cc7425ec..75c7e529902 100644 --- a/app/policies/ci/build_policy.rb +++ b/app/policies/ci/build_policy.rb @@ -18,6 +18,10 @@ module Ci @subject.project.branch_allows_collaboration?(@user, @subject.ref) end + condition(:terminal, scope: :subject) do + @subject.has_terminal? + end + rule { protected_ref }.policy do prevent :update_build prevent :erase_build @@ -29,5 +33,7 @@ module Ci enable :update_build enable :update_commit_status end + + rule { can?(:update_build) & terminal }.enable :create_build_terminal end end diff --git a/app/services/ci/register_job_service.rb b/app/services/ci/register_job_service.rb index c0dce45e2e7..6eb1c4f52de 100644 --- a/app/services/ci/register_job_service.rb +++ b/app/services/ci/register_job_service.rb @@ -13,7 +13,7 @@ module Ci @runner = runner end - def execute + def execute(params = {}) builds = if runner.instance_type? builds_for_shared_runner @@ -41,6 +41,8 @@ module Ci # with StateMachines::InvalidTransition or StaleObjectError when doing run! or save method. begin build.runner_id = runner.id + build.runner_session_attributes = params[:session] if params[:session].present? + build.run! register_success(build) diff --git a/app/services/issues/base_service.rb b/app/services/issues/base_service.rb index 9f6cfc0f6d3..cbfef175af0 100644 --- a/app/services/issues/base_service.rb +++ b/app/services/issues/base_service.rb @@ -32,8 +32,9 @@ module Issues def filter_assignee(issuable) return if params[:assignee_ids].blank? - # The number of assignees is limited by one for GitLab CE - params[:assignee_ids] = params[:assignee_ids][0, 1] + unless issuable.allows_multiple_assignees? + params[:assignee_ids] = params[:assignee_ids].take(1) + end assignee_ids = params[:assignee_ids].select { |assignee_id| assignee_can_read?(issuable, assignee_id) } diff --git a/app/views/layouts/header/_current_user_dropdown.html.haml b/app/views/layouts/header/_current_user_dropdown.html.haml index a74ea246eaf..9ed05d6e3d0 100644 --- a/app/views/layouts/header/_current_user_dropdown.html.haml +++ b/app/views/layouts/header/_current_user_dropdown.html.haml @@ -17,11 +17,7 @@ = link_to _("Help"), help_path - if current_user_menu?(:help) || current_user_menu?(:settings) || current_user_menu?(:profile) %li.divider - %li - = link_to "https://about.gitlab.com/contributing", target: '_blank', class: 'text-nowrap' do - = _("Contribute to GitLab") - = sprite_icon('external-link', size: 16) - %li.divider + = render 'shared/user_dropdown_contributing_link' - if current_user_menu?(:sign_out) %li = link_to _("Sign out"), destroy_user_session_path, class: "sign-out-link" diff --git a/app/views/projects/jobs/_sidebar.html.haml b/app/views/projects/jobs/_sidebar.html.haml index 459150c1067..b88fe47726d 100644 --- a/app/views/projects/jobs/_sidebar.html.haml +++ b/app/views/projects/jobs/_sidebar.html.haml @@ -1,6 +1,10 @@ %aside.right-sidebar.right-sidebar-expanded.build-sidebar.js-build-sidebar.js-right-sidebar{ data: { "offset-top" => "101", "spy" => "affix" } } .sidebar-container .blocks-container + - if can?(current_user, :create_build_terminal, @build) + .block + = link_to terminal_project_job_path(@project, @build), class: 'terminal-button pull-right btn visible-md-block visible-lg-block', title: 'Terminal' do + Terminal #js-details-block-vue{ data: { can_user_retry: can?(current_user, :update_build, @build) && @build.retryable? } } @@ -14,8 +18,8 @@ #{time_ago_with_tooltip(@build.artifacts_expire_at)} - elsif @build.has_expiring_artifacts? %p.build-detail-row - The artifacts will be removed in - %span= time_ago_with_tooltip @build.artifacts_expire_at + The artifacts will be removed + #{time_ago_with_tooltip(@build.artifacts_expire_at)} - if @build.artifacts? .btn-group.d-flex{ role: :group } diff --git a/app/views/projects/jobs/terminal.html.haml b/app/views/projects/jobs/terminal.html.haml new file mode 100644 index 00000000000..efea666a4d9 --- /dev/null +++ b/app/views/projects/jobs/terminal.html.haml @@ -0,0 +1,11 @@ +- @no_container = true +- add_to_breadcrumbs 'Jobs', project_jobs_path(@project) +- add_to_breadcrumbs "##{@build.id}", project_job_path(@project, @build) +- breadcrumb_title 'Terminal' +- page_title 'Terminal', "#{@build.name} (##{@build.id})", 'Jobs' + +- content_for :page_specific_javascripts do + = stylesheet_link_tag "xterm/xterm" + +.terminal-container{ class: container_class } + #terminal{ data: { project_path: terminal_project_job_path(@project, @build, format: :ws) } } diff --git a/app/views/projects/settings/ci_cd/_form.html.haml b/app/views/projects/settings/ci_cd/_form.html.haml index 5025460a2d0..fb113aa7639 100644 --- a/app/views/projects/settings/ci_cd/_form.html.haml +++ b/app/views/projects/settings/ci_cd/_form.html.haml @@ -117,6 +117,9 @@ %li JaCoCo (Java/Kotlin) %code Total.*?([0-9]{1,3})% + %li + go test -cover (Go) + %code coverage: \d+.\d+% of statements = f.submit _('Save changes'), class: "btn btn-save" diff --git a/app/views/shared/_user_dropdown_contributing_link.html.haml b/app/views/shared/_user_dropdown_contributing_link.html.haml new file mode 100644 index 00000000000..333d6fa3489 --- /dev/null +++ b/app/views/shared/_user_dropdown_contributing_link.html.haml @@ -0,0 +1,5 @@ +%li + = link_to "https://about.gitlab.com/contributing", target: '_blank', class: 'text-nowrap' do + = _("Contribute to GitLab") + = sprite_icon('external-link', size: 16) +%li.divider diff --git a/app/views/shared/boards/_show.html.haml b/app/views/shared/boards/_show.html.haml index a88d8f61fb4..28e6fe1b16d 100644 --- a/app/views/shared/boards/_show.html.haml +++ b/app/views/shared/boards/_show.html.haml @@ -2,8 +2,8 @@ - group = local_assigns.fetch(:group, false) - @no_breadcrumb_container = true - @no_container = true -- @content_class = "issue-boards-content" -- breadcrumb_title _("Issue Board") +- @content_class = "issue-boards-content js-focus-mode-board" +- breadcrumb_title _("Issue Boards") - page_title _("Boards") - content_for :page_specific_javascripts do @@ -11,10 +11,11 @@ -# haml-lint:disable InlineJavaScript %script#js-board-template{ type: "text/x-template" }= render "shared/boards/components/board" %script#js-board-modal-filter{ type: "text/x-template" }= render "shared/issuable/search_bar", type: :boards_modal + %script#js-board-promotion{ type: "text/x-template" }= render_if_exists "shared/promotions/promote_issue_board" #board-app.boards-app{ "v-cloak" => true, data: board_data, ":class" => "{ 'is-compact': detailIssueVisible }" } .d-none.d-sm-none.d-md-block - = render 'shared/issuable/search_bar', type: :boards + = render 'shared/issuable/search_bar', type: :boards, board: board .boards-list .boards-app-loading.text-center{ "v-if" => "loading" } diff --git a/app/views/shared/issuable/_search_bar.html.haml b/app/views/shared/issuable/_search_bar.html.haml index ef9ea2194ee..9ce7f6fe269 100644 --- a/app/views/shared/issuable/_search_bar.html.haml +++ b/app/views/shared/issuable/_search_bar.html.haml @@ -1,9 +1,14 @@ - type = local_assigns.fetch(:type) +- board = local_assigns.fetch(:board, nil) - block_css_class = type != :boards_modal ? 'row-content-block second-block' : '' - full_path = @project.present? ? @project.full_path : @group.full_path +- user_can_admin_list = board && can?(current_user, :admin_list, board.parent) .issues-filters .issues-details-filters.filtered-search-block{ class: block_css_class, "v-pre" => type == :boards_modal } + - if type == :boards + #js-multiple-boards-switcher.inline.boards-switcher{ "v-cloak" => true } + = render_if_exists "shared/boards/switcher", board: board = form_tag page_filter_path(without: [:assignee_id, :author_id, :milestone_title, :label_name, :search]), method: :get, class: 'filter-form js-filter-form' do - if params[:search].present? = hidden_field_tag :search, params[:search] @@ -99,13 +104,18 @@ %gl-emoji %span.js-data-value.prepend-left-10 {{name}} + + = render_if_exists 'shared/issuable/filter_weight', type: type + %button.clear-search.hidden{ type: 'button' } = icon('times') .filter-dropdown-container - if type == :boards - - if can?(current_user, :admin_list, board.parent) - = render_if_exists 'shared/issuable/board_create_list_dropdown', board: board + .js-board-config{ data: { can_admin_list: user_can_admin_list, has_scope: board.scoped? } } + - if user_can_admin_list + = render 'shared/issuable/board_create_list_dropdown', board: board - if @project #js-add-issues-btn.prepend-left-10{ data: { can_admin_list: can?(current_user, :admin_list, @project) } } + #js-toggle-focus-btn - elsif type != :boards_modal = render 'shared/sort_dropdown' diff --git a/app/views/shared/projects/_project.html.haml b/app/views/shared/projects/_project.html.haml index 88f0675f795..6be1fb485a4 100644 --- a/app/views/shared/projects/_project.html.haml +++ b/app/views/shared/projects/_project.html.haml @@ -4,7 +4,7 @@ - ci = false unless local_assigns[:ci] == true - skip_namespace = false unless local_assigns[:skip_namespace] == true - user = local_assigns[:user] -- access = user&.max_member_access_for_project(project.id) unless user.nil? +- access = max_project_member_access(project) - css_class = '' unless local_assigns[:css_class] - show_last_commit_as_description = false unless local_assigns[:show_last_commit_as_description] == true && can_show_last_commit_in_list?(project) - css_class += " no-description" if project.description.blank? && !show_last_commit_as_description diff --git a/app/workers/archive_trace_worker.rb b/app/workers/archive_trace_worker.rb index 9169f21af2a..c6f89a17729 100644 --- a/app/workers/archive_trace_worker.rb +++ b/app/workers/archive_trace_worker.rb @@ -5,7 +5,7 @@ class ArchiveTraceWorker include PipelineBackgroundQueue def perform(job_id) - Ci::Build.find_by(id: job_id).try do |job| + Ci::Build.without_archived_trace.find_by(id: job_id).try do |job| job.trace.archive! end end diff --git a/app/workers/ci/archive_traces_cron_worker.rb b/app/workers/ci/archive_traces_cron_worker.rb index 7016edde698..7d4e9660a4e 100644 --- a/app/workers/ci/archive_traces_cron_worker.rb +++ b/app/workers/ci/archive_traces_cron_worker.rb @@ -12,6 +12,7 @@ module Ci Ci::Build.finished.with_live_trace.find_each(batch_size: 100) do |build| begin build.trace.archive! + rescue ::Gitlab::Ci::Trace::AlreadyArchivedError rescue => e failed_archive_counter.increment Rails.logger.error "Failed to archive stale live trace. id: #{build.id} message: #{e.message}" diff --git a/changelogs/unreleased/20357.yml b/changelogs/unreleased/20357.yml new file mode 100644 index 00000000000..b4ce686eece --- /dev/null +++ b/changelogs/unreleased/20357.yml @@ -0,0 +1,5 @@ +--- +title: Fix double "in" in time to artifact deletion message +merge_request: 20357 +author: "@bbodenmiller" +type: fixed diff --git a/changelogs/unreleased/31583-osw-gfm-complete-status-indication.yml b/changelogs/unreleased/31583-osw-gfm-complete-status-indication.yml new file mode 100644 index 00000000000..6f2cf275592 --- /dev/null +++ b/changelogs/unreleased/31583-osw-gfm-complete-status-indication.yml @@ -0,0 +1,5 @@ +--- +title: Present state indication on GFM preview +merge_request: +author: +type: added diff --git a/changelogs/unreleased/44697-when-editing-a-comment-in-an-issue-the-preview-mode-is-toggled-in-the-main-textarea.yml b/changelogs/unreleased/44697-when-editing-a-comment-in-an-issue-the-preview-mode-is-toggled-in-the-main-textarea.yml new file mode 100644 index 00000000000..750e28f1a8d --- /dev/null +++ b/changelogs/unreleased/44697-when-editing-a-comment-in-an-issue-the-preview-mode-is-toggled-in-the-main-textarea.yml @@ -0,0 +1,6 @@ +--- +title: Fixed bug when editing a comment in an issue,the preview mode is toggled in + the main textarea +merge_request: 20112 +author: Constance Okoghenun +type: fixed diff --git a/changelogs/unreleased/48825-performance.yml b/changelogs/unreleased/48825-performance.yml new file mode 100644 index 00000000000..428852f6f8b --- /dev/null +++ b/changelogs/unreleased/48825-performance.yml @@ -0,0 +1,8 @@ +--- +title: Improves performance of mr code, by fixing the state being mutated outside + of the store in the util function trimFirstCharOfLineContent and in map operations. + Avoids map operation in an empty array. Adds specs to the trimFirstCharOfLineContent + function +merge_request: 20380 +author: filipa +type: performance diff --git a/changelogs/unreleased/dm-invalid-active-service-template.yml b/changelogs/unreleased/dm-invalid-active-service-template.yml new file mode 100644 index 00000000000..8b77fac55b9 --- /dev/null +++ b/changelogs/unreleased/dm-invalid-active-service-template.yml @@ -0,0 +1,5 @@ +--- +title: Deactivate new KubernetesService created from active template to prevent project creation from failing +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/fix-trace-archive-cron-worker-race-condition.yml b/changelogs/unreleased/fix-trace-archive-cron-worker-race-condition.yml new file mode 100644 index 00000000000..ca8f4252008 --- /dev/null +++ b/changelogs/unreleased/fix-trace-archive-cron-worker-race-condition.yml @@ -0,0 +1,5 @@ +--- +title: Check if archived trace exist before archive it +merge_request: 20297 +author: +type: fixed diff --git a/changelogs/unreleased/fj-43565-wrong-role-displayed.yml b/changelogs/unreleased/fj-43565-wrong-role-displayed.yml new file mode 100644 index 00000000000..67ff25bc50c --- /dev/null +++ b/changelogs/unreleased/fj-43565-wrong-role-displayed.yml @@ -0,0 +1,5 @@ +--- +title: Fix wrong role badge displayed in projects dashboard +merge_request: 20374 +author: +type: fixed diff --git a/changelogs/unreleased/fj-web-terminal-ci-build.yml b/changelogs/unreleased/fj-web-terminal-ci-build.yml new file mode 100644 index 00000000000..c3608d4203b --- /dev/null +++ b/changelogs/unreleased/fj-web-terminal-ci-build.yml @@ -0,0 +1,5 @@ +--- +title: Add Web Terminal for Ci Builds +merge_request: +author: Vicky Chijwani +type: added diff --git a/changelogs/unreleased/gitaly-timeouts.yml b/changelogs/unreleased/gitaly-timeouts.yml new file mode 100644 index 00000000000..ac8008faa2d --- /dev/null +++ b/changelogs/unreleased/gitaly-timeouts.yml @@ -0,0 +1,5 @@ +--- +title: Updated Gitaly fail-fast timeout values +merge_request: !20259 +author: +type: performance diff --git a/changelogs/unreleased/remove-trace-efficiently.yml b/changelogs/unreleased/remove-trace-efficiently.yml new file mode 100644 index 00000000000..a6ba6d28dce --- /dev/null +++ b/changelogs/unreleased/remove-trace-efficiently.yml @@ -0,0 +1,5 @@ +--- +title: Remove redundant query when removing trace +merge_request: 20324 +author: +type: performance diff --git a/config/routes/project.rb b/config/routes/project.rb index 2ebf84f2ecf..5057e937941 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -279,6 +279,8 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do post :erase get :trace, defaults: { format: 'json' } get :raw + get :terminal + get '/terminal.ws/authorize', to: 'jobs#terminal_websocket_authorize', constraints: { format: nil } end resource :artifacts, only: [] do diff --git a/db/migrate/20180613081317_create_ci_builds_runner_session.rb b/db/migrate/20180613081317_create_ci_builds_runner_session.rb new file mode 100644 index 00000000000..e550c07b9ab --- /dev/null +++ b/db/migrate/20180613081317_create_ci_builds_runner_session.rb @@ -0,0 +1,21 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class CreateCiBuildsRunnerSession < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + def change + create_table :ci_builds_runner_session, id: :bigserial do |t| + t.integer :build_id, null: false + t.string :url, null: false + t.string :certificate + t.string :authorization + + t.foreign_key :ci_builds, column: :build_id, on_delete: :cascade + t.index :build_id, unique: true + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 9a4e3fe5555..c9aaf80f059 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -358,6 +358,15 @@ ActiveRecord::Schema.define(version: 20180629191052) do add_index "ci_builds_metadata", ["build_id"], name: "index_ci_builds_metadata_on_build_id", unique: true, using: :btree add_index "ci_builds_metadata", ["project_id"], name: "index_ci_builds_metadata_on_project_id", using: :btree + create_table "ci_builds_runner_session", id: :bigserial, force: :cascade do |t| + t.integer "build_id", null: false + t.string "url", null: false + t.string "certificate" + t.string "authorization" + end + + add_index "ci_builds_runner_session", ["build_id"], name: "index_ci_builds_runner_session_on_build_id", unique: true, using: :btree + create_table "ci_group_variables", force: :cascade do |t| t.string "key", null: false t.text "value" @@ -2191,6 +2200,7 @@ ActiveRecord::Schema.define(version: 20180629191052) do add_foreign_key "ci_builds", "projects", name: "fk_befce0568a", on_delete: :cascade add_foreign_key "ci_builds_metadata", "ci_builds", column: "build_id", on_delete: :cascade add_foreign_key "ci_builds_metadata", "projects", on_delete: :cascade + add_foreign_key "ci_builds_runner_session", "ci_builds", column: "build_id", on_delete: :cascade add_foreign_key "ci_group_variables", "namespaces", column: "group_id", name: "fk_33ae4d58d8", on_delete: :cascade add_foreign_key "ci_job_artifacts", "ci_builds", column: "job_id", on_delete: :cascade add_foreign_key "ci_job_artifacts", "projects", on_delete: :cascade diff --git a/doc/api/projects.md b/doc/api/projects.md index b4599fdc97e..1e06f6d01f3 100644 --- a/doc/api/projects.md +++ b/doc/api/projects.md @@ -511,6 +511,39 @@ GET /projects/:id } ``` +If the project is a fork, and you provide a valid token to authenticate, the +`forked_from_project` field will appear in the response. + +```json +{ + "id":3, + + ... + + "forked_from_project":{ + "id":13083, + "description":"GitLab Community Edition", + "name":"GitLab Community Edition", + "name_with_namespace":"GitLab.org / GitLab Community Edition", + "path":"gitlab-ce", + "path_with_namespace":"gitlab-org/gitlab-ce", + "created_at":"2013-09-26T06:02:36.000Z", + "default_branch":"master", + "tag_list":[], + "ssh_url_to_repo":"git@gitlab.com:gitlab-org/gitlab-ce.git", + "http_url_to_repo":"https://gitlab.com/gitlab-org/gitlab-ce.git", + "web_url":"https://gitlab.com/gitlab-org/gitlab-ce", + "avatar_url":"https://assets.gitlab-static.net/uploads/-/system/project/avatar/13083/logo-extra-whitespace.png", + "star_count":3812, + "forks_count":3561, + "last_activity_at":"2018-01-02T11:40:26.570Z" + } + + ... + +} +``` + ## Get project users Get the users list of a project. diff --git a/doc/ci/examples/container_scanning.md b/doc/ci/examples/container_scanning.md index af87c83a4e5..0f79f7d1b17 100644 --- a/doc/ci/examples/container_scanning.md +++ b/doc/ci/examples/container_scanning.md @@ -57,9 +57,9 @@ so, the CI/CD job must be named `container_scanning` and the artifact path must [Learn more on container scanning results shown in merge requests](https://docs.gitlab.com/ee/user/project/merge_requests/container_scanning.html). CAUTION: **Caution:** -Container Scanning was previously using `sast:container` for job name and +Before GitLab 11.0, Container Scanning was previously using `sast:container` for job name and `gl-sast-container-report.json` for the artifact name. While these old names -are still maintained they have been deprecated with GitLab 11.0 and may be removed +are still maintained, they have been deprecated with GitLab 11.0 and may be removed in next major release, GitLab 12.0. You are advised to update your current `.gitlab-ci.yml` configuration to reflect that change. diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 930b5ef37a3..3a6e707fd5b 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -1203,6 +1203,7 @@ module API class RunnerInfo < Grape::Entity expose :metadata_timeout, as: :timeout + expose :runner_session_url end class Step < Grape::Entity diff --git a/lib/api/runner.rb b/lib/api/runner.rb index b4b984f7b8f..d0cc0945a5f 100644 --- a/lib/api/runner.rb +++ b/lib/api/runner.rb @@ -81,6 +81,11 @@ module API requires :token, type: String, desc: %q(Runner's authentication token) optional :last_update, type: String, desc: %q(Runner's queue last_update token) optional :info, type: Hash, desc: %q(Runner's metadata) + optional :session, type: Hash, desc: %q(Runner's session data) do + optional :url, type: String, desc: %q(Session's url) + optional :certificate, type: String, desc: %q(Session's certificate) + optional :authorization, type: String, desc: %q(Session's authorization) + end end post '/request' do authenticate_runner! @@ -90,14 +95,16 @@ module API break no_content! end - if current_runner.runner_queue_value_latest?(params[:last_update]) - header 'X-GitLab-Last-Update', params[:last_update] + runner_params = declared_params(include_missing: false) + + if current_runner.runner_queue_value_latest?(runner_params[:last_update]) + header 'X-GitLab-Last-Update', runner_params[:last_update] Gitlab::Metrics.add_event(:build_not_found_cached) break no_content! end new_update = current_runner.ensure_runner_queue_value - result = ::Ci::RegisterJobService.new(current_runner).execute + result = ::Ci::RegisterJobService.new(current_runner).execute(runner_params) if result.valid? if result.build diff --git a/lib/gitlab/ci/trace.rb b/lib/gitlab/ci/trace.rb index a52d71225bb..ee54b893598 100644 --- a/lib/gitlab/ci/trace.rb +++ b/lib/gitlab/ci/trace.rb @@ -6,6 +6,7 @@ module Gitlab LEASE_TIMEOUT = 1.hour ArchiveError = Class.new(StandardError) + AlreadyArchivedError = Class.new(StandardError) attr_reader :job @@ -81,7 +82,9 @@ module Gitlab def write(mode) stream = Gitlab::Ci::Trace::Stream.new do - if current_path + if trace_artifact + raise AlreadyArchivedError, 'Could not write to the archived trace' + elsif current_path File.open(current_path, mode) elsif Feature.enabled?('ci_enable_live_trace') Gitlab::Ci::Trace::ChunkedIO.new(job) @@ -98,14 +101,17 @@ module Gitlab end def erase! - trace_artifact&.destroy - - paths.each do |trace_path| - FileUtils.rm(trace_path, force: true) - end - - job.trace_chunks.fast_destroy_all - job.erase_old_trace! + ## + # Erase the archived trace + trace_artifact&.destroy! + + ## + # Erase the live trace + job.trace_chunks.fast_destroy_all # Destroy chunks of a live trace + FileUtils.rm_f(current_path) if current_path # Remove a trace file of a live trace + job.erase_old_trace! if job.has_old_trace? # Remove a trace in database of a live trace + ensure + @current_path = nil end def archive! @@ -117,7 +123,7 @@ module Gitlab private def unsafe_archive! - raise ArchiveError, 'Already archived' if trace_artifact + raise AlreadyArchivedError, 'Could not archive again' if trace_artifact raise ArchiveError, 'Job is not finished yet' unless job.complete? if job.trace_chunks.any? diff --git a/lib/gitlab/git/commit.rb b/lib/gitlab/git/commit.rb index 36d56e411d8..c67826da1d2 100644 --- a/lib/gitlab/git/commit.rb +++ b/lib/gitlab/git/commit.rb @@ -63,8 +63,12 @@ module Gitlab # This saves us an RPC round trip. return nil if commit_id.include?(':') - commit = repo.wrapped_gitaly_errors do - repo.gitaly_commit_client.find_commit(commit_id) + commit = repo.gitaly_migrate(:find_commit) do |is_enabled| + if is_enabled + repo.gitaly_commit_client.find_commit(commit_id) + else + rugged_find(repo, commit_id) + end end decorate(repo, commit) if commit @@ -74,6 +78,12 @@ module Gitlab nil end + def rugged_find(repo, commit_id) + obj = repo.rev_parse_target(commit_id) + + obj.is_a?(Rugged::Commit) ? obj : nil + end + # Get last commit for HEAD # # Ex. diff --git a/lib/gitlab/git/conflict/resolver.rb b/lib/gitlab/git/conflict/resolver.rb index 0e4a973301f..c3cb0264112 100644 --- a/lib/gitlab/git/conflict/resolver.rb +++ b/lib/gitlab/git/conflict/resolver.rb @@ -12,8 +12,14 @@ module Gitlab end def conflicts - @conflicts ||= @target_repository.wrapped_gitaly_errors do - gitaly_conflicts_client(@target_repository).list_conflict_files.to_a + @conflicts ||= begin + @target_repository.gitaly_migrate(:conflicts_list_conflict_files) do |is_enabled| + if is_enabled + gitaly_conflicts_client(@target_repository).list_conflict_files.to_a + else + rugged_list_conflict_files + end + end end rescue GRPC::FailedPrecondition => e raise Gitlab::Git::Conflict::Resolver::ConflictSideMissing.new(e.message) @@ -22,8 +28,12 @@ module Gitlab end def resolve_conflicts(source_repository, resolution, source_branch:, target_branch:) - source_repository.wrapped_gitaly_errors do - gitaly_conflicts_client(source_repository).resolve_conflicts(@target_repository, resolution, source_branch, target_branch) + source_repository.gitaly_migrate(:conflicts_resolve_conflicts) do |is_enabled| + if is_enabled + gitaly_conflicts_client(source_repository).resolve_conflicts(@target_repository, resolution, source_branch, target_branch) + else + rugged_resolve_conflicts(source_repository, resolution, source_branch, target_branch) + end end end @@ -51,6 +61,57 @@ module Gitlab def gitaly_conflicts_client(repository) repository.gitaly_conflicts_client(@our_commit_oid, @their_commit_oid) end + + def write_resolved_file_to_index(repository, index, file, params) + if params[:sections] + resolved_lines = file.resolve_lines(params[:sections]) + new_file = resolved_lines.map { |line| line[:full_line] }.join("\n") + + new_file << "\n" if file.our_blob.data.end_with?("\n") + elsif params[:content] + new_file = file.resolve_content(params[:content]) + end + + our_path = file.our_path + + oid = repository.rugged.write(new_file, :blob) + index.add(path: our_path, oid: oid, mode: file.our_mode) + index.conflict_remove(our_path) + end + + def rugged_list_conflict_files + target_index = @target_repository.rugged.merge_commits(@our_commit_oid, @their_commit_oid) + + # We don't need to do `with_repo_branch_commit` here, because the target + # project always fetches source refs when creating merge request diffs. + conflict_files(@target_repository, target_index) + end + + def rugged_resolve_conflicts(source_repository, resolution, source_branch, target_branch) + source_repository.with_repo_branch_commit(@target_repository, target_branch) do + index = source_repository.rugged.merge_commits(@our_commit_oid, @their_commit_oid) + conflicts = conflict_files(source_repository, index) + + resolution.files.each do |file_params| + conflict_file = conflict_for_path(conflicts, file_params[:old_path], file_params[:new_path]) + + write_resolved_file_to_index(source_repository, index, conflict_file, file_params) + end + + unless index.conflicts.empty? + missing_files = index.conflicts.map { |file| file[:ours][:path] } + + raise ResolutionError, "Missing resolutions for the following files: #{missing_files.join(', ')}" + end + + commit_params = { + message: resolution.commit_message, + parents: [@our_commit_oid, @their_commit_oid] + } + + source_repository.commit_index(resolution.user, source_branch, index, commit_params) + end + end end end end diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 647f54d5b84..bbfe6ab1d95 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -555,8 +555,12 @@ module Gitlab # diff options. The +options+ hash can also include :break_rewrites to # split larger rewrites into delete/add pairs. def diff(from, to, options = {}, *paths) - iterator = wrapped_gitaly_errors do - gitaly_commit_client.diff(from, to, options.merge(paths: paths)) + iterator = gitaly_migrate(:diff_between) do |is_enabled| + if is_enabled + gitaly_commit_client.diff(from, to, options.merge(paths: paths)) + else + diff_patches(from, to, options, *paths) + end end Gitlab::Git::DiffCollection.new(iterator, options) @@ -1587,6 +1591,17 @@ module Gitlab tmp_entry end + # Return the Rugged patches for the diff between +from+ and +to+. + def diff_patches(from, to, options = {}, *paths) + options ||= {} + break_rewrites = options[:break_rewrites] + actual_options = Gitlab::Git::Diff.filter_diff_options(options.merge(paths: paths)) + + diff = rugged.diff(from, to, actual_options) + diff.find_similar!(break_rewrites: break_rewrites) + diff.each_patch + end + def sort_branches(branches, sort_by) case sort_by when 'name' diff --git a/lib/gitlab/gitaly_client/blob_service.rb b/lib/gitlab/gitaly_client/blob_service.rb index 28554208984..1840bf45154 100644 --- a/lib/gitlab/gitaly_client/blob_service.rb +++ b/lib/gitlab/gitaly_client/blob_service.rb @@ -13,7 +13,7 @@ module Gitlab oid: oid, limit: limit ) - response = GitalyClient.call(@gitaly_repo.storage_name, :blob_service, :get_blob, request) + response = GitalyClient.call(@gitaly_repo.storage_name, :blob_service, :get_blob, request, timeout: GitalyClient.fast_timeout) data = '' blob = nil @@ -43,7 +43,7 @@ module Gitlab blob_ids: blob_ids ) - response = GitalyClient.call(@gitaly_repo.storage_name, :blob_service, :get_lfs_pointers, request) + response = GitalyClient.call(@gitaly_repo.storage_name, :blob_service, :get_lfs_pointers, request, timeout: GitalyClient.medium_timeout) map_lfs_pointers(response) end @@ -66,7 +66,7 @@ module Gitlab :blob_service, :get_blobs, request, - timeout: GitalyClient.default_timeout + timeout: GitalyClient.fast_timeout ) GitalyClient::BlobsStitcher.new(response) @@ -85,7 +85,7 @@ module Gitlab request.not_in_refs += not_in end - response = GitalyClient.call(@gitaly_repo.storage_name, :blob_service, :get_new_lfs_pointers, request) + response = GitalyClient.call(@gitaly_repo.storage_name, :blob_service, :get_new_lfs_pointers, request, timeout: GitalyClient.medium_timeout) map_lfs_pointers(response) end @@ -96,7 +96,7 @@ module Gitlab revision: encode_binary(revision) ) - response = GitalyClient.call(@gitaly_repo.storage_name, :blob_service, :get_all_lfs_pointers, request) + response = GitalyClient.call(@gitaly_repo.storage_name, :blob_service, :get_all_lfs_pointers, request, timeout: GitalyClient.medium_timeout) map_lfs_pointers(response) end diff --git a/lib/gitlab/gitaly_client/commit_service.rb b/lib/gitlab/gitaly_client/commit_service.rb index d979ba0eb14..72e1e59d8df 100644 --- a/lib/gitlab/gitaly_client/commit_service.rb +++ b/lib/gitlab/gitaly_client/commit_service.rb @@ -70,7 +70,7 @@ module Gitlab def commit_deltas(commit) request = Gitaly::CommitDeltaRequest.new(diff_from_parent_request_params(commit)) - response = GitalyClient.call(@repository.storage, :diff_service, :commit_delta, request) + response = GitalyClient.call(@repository.storage, :diff_service, :commit_delta, request, timeout: GitalyClient.fast_timeout) response.flat_map { |msg| msg.deltas } end @@ -302,7 +302,7 @@ module Gitlab end end - response = GitalyClient.call(@repository.storage, :commit_service, :filter_shas_with_signatures, enum) + response = GitalyClient.call(@repository.storage, :commit_service, :filter_shas_with_signatures, enum, timeout: GitalyClient.fast_timeout) response.flat_map do |msg| msg.shas.map { |sha| EncodingHelper.encode!(sha) } @@ -330,7 +330,7 @@ module Gitlab def get_commit_signatures(commit_ids) request = Gitaly::GetCommitSignaturesRequest.new(repository: @gitaly_repo, commit_ids: commit_ids) - response = GitalyClient.call(@repository.storage, :commit_service, :get_commit_signatures, request) + response = GitalyClient.call(@repository.storage, :commit_service, :get_commit_signatures, request, timeout: GitalyClient.fast_timeout) signatures = Hash.new { |h, k| h[k] = [''.b, ''.b] } current_commit_id = nil @@ -349,7 +349,7 @@ module Gitlab def get_commit_messages(commit_ids) request = Gitaly::GetCommitMessagesRequest.new(repository: @gitaly_repo, commit_ids: commit_ids) - response = GitalyClient.call(@repository.storage, :commit_service, :get_commit_messages, request) + response = GitalyClient.call(@repository.storage, :commit_service, :get_commit_messages, request, timeout: GitalyClient.fast_timeout) messages = Hash.new { |h, k| h[k] = ''.b } current_commit_id = nil diff --git a/lib/gitlab/gitaly_client/conflicts_service.rb b/lib/gitlab/gitaly_client/conflicts_service.rb index e14734495a8..b1a01b185e6 100644 --- a/lib/gitlab/gitaly_client/conflicts_service.rb +++ b/lib/gitlab/gitaly_client/conflicts_service.rb @@ -46,7 +46,7 @@ module Gitlab end end - response = GitalyClient.call(@repository.storage, :conflicts_service, :resolve_conflicts, req_enum, remote_storage: target_repository.storage) + response = GitalyClient.call(@repository.storage, :conflicts_service, :resolve_conflicts, req_enum, remote_storage: target_repository.storage, timeout: GitalyClient.medium_timeout) if response.resolution_error.present? raise Gitlab::Git::Conflict::Resolver::ResolutionError, response.resolution_error diff --git a/lib/gitlab/gitaly_client/namespace_service.rb b/lib/gitlab/gitaly_client/namespace_service.rb index bd7c345ac01..d4e982b649a 100644 --- a/lib/gitlab/gitaly_client/namespace_service.rb +++ b/lib/gitlab/gitaly_client/namespace_service.rb @@ -8,31 +8,31 @@ module Gitlab def exists?(name) request = Gitaly::NamespaceExistsRequest.new(storage_name: @storage, name: name) - gitaly_client_call(:namespace_exists, request).exists + gitaly_client_call(:namespace_exists, request, timeout: GitalyClient.fast_timeout).exists end def add(name) request = Gitaly::AddNamespaceRequest.new(storage_name: @storage, name: name) - gitaly_client_call(:add_namespace, request) + gitaly_client_call(:add_namespace, request, timeout: GitalyClient.fast_timeout) end def remove(name) request = Gitaly::RemoveNamespaceRequest.new(storage_name: @storage, name: name) - gitaly_client_call(:remove_namespace, request) + gitaly_client_call(:remove_namespace, request, timeout: nil) end def rename(from, to) request = Gitaly::RenameNamespaceRequest.new(storage_name: @storage, from: from, to: to) - gitaly_client_call(:rename_namespace, request) + gitaly_client_call(:rename_namespace, request, timeout: GitalyClient.fast_timeout) end private - def gitaly_client_call(type, request) - GitalyClient.call(@storage, :namespace_service, type, request) + def gitaly_client_call(type, request, timeout: nil) + GitalyClient.call(@storage, :namespace_service, type, request, timeout: timeout) end end end diff --git a/lib/gitlab/gitaly_client/operation_service.rb b/lib/gitlab/gitaly_client/operation_service.rb index c04183a348f..ab2c61f6782 100644 --- a/lib/gitlab/gitaly_client/operation_service.rb +++ b/lib/gitlab/gitaly_client/operation_service.rb @@ -17,7 +17,7 @@ module Gitlab user: Gitlab::Git::User.from_gitlab(user).to_gitaly ) - response = GitalyClient.call(@repository.storage, :operation_service, :user_delete_tag, request) + response = GitalyClient.call(@repository.storage, :operation_service, :user_delete_tag, request, timeout: GitalyClient.medium_timeout) if pre_receive_error = response.pre_receive_error.presence raise Gitlab::Git::PreReceiveError, pre_receive_error @@ -33,7 +33,7 @@ module Gitlab message: encode_binary(message.to_s) ) - response = GitalyClient.call(@repository.storage, :operation_service, :user_create_tag, request) + response = GitalyClient.call(@repository.storage, :operation_service, :user_create_tag, request, timeout: GitalyClient.medium_timeout) if pre_receive_error = response.pre_receive_error.presence raise Gitlab::Git::PreReceiveError, pre_receive_error elsif response.exists @@ -276,7 +276,8 @@ module Gitlab :operation_service, :"user_#{rpc}", request, - remote_storage: start_repository.storage + remote_storage: start_repository.storage, + timeout: GitalyClient.medium_timeout ) handle_cherry_pick_or_revert_response(response) diff --git a/lib/gitlab/gitaly_client/ref_service.rb b/lib/gitlab/gitaly_client/ref_service.rb index 3ac46be6208..7f4eed9222a 100644 --- a/lib/gitlab/gitaly_client/ref_service.rb +++ b/lib/gitlab/gitaly_client/ref_service.rb @@ -12,7 +12,7 @@ module Gitlab def branches request = Gitaly::FindAllBranchesRequest.new(repository: @gitaly_repo) - response = GitalyClient.call(@storage, :ref_service, :find_all_branches, request) + response = GitalyClient.call(@storage, :ref_service, :find_all_branches, request, timeout: GitalyClient.fast_timeout) consume_find_all_branches_response(response) end @@ -23,26 +23,26 @@ module Gitlab merged_only: true, merged_branches: branch_names.map { |s| encode_binary(s) } ) - response = GitalyClient.call(@storage, :ref_service, :find_all_branches, request) + response = GitalyClient.call(@storage, :ref_service, :find_all_branches, request, timeout: GitalyClient.fast_timeout) consume_find_all_branches_response(response) end def default_branch_name request = Gitaly::FindDefaultBranchNameRequest.new(repository: @gitaly_repo) - response = GitalyClient.call(@storage, :ref_service, :find_default_branch_name, request) + response = GitalyClient.call(@storage, :ref_service, :find_default_branch_name, request, timeout: GitalyClient.fast_timeout) Gitlab::Git.branch_name(response.name) end def branch_names request = Gitaly::FindAllBranchNamesRequest.new(repository: @gitaly_repo) - response = GitalyClient.call(@storage, :ref_service, :find_all_branch_names, request) + response = GitalyClient.call(@storage, :ref_service, :find_all_branch_names, request, timeout: GitalyClient.fast_timeout) consume_refs_response(response) { |name| Gitlab::Git.branch_name(name) } end def tag_names request = Gitaly::FindAllTagNamesRequest.new(repository: @gitaly_repo) - response = GitalyClient.call(@storage, :ref_service, :find_all_tag_names, request) + response = GitalyClient.call(@storage, :ref_service, :find_all_tag_names, request, timeout: GitalyClient.fast_timeout) consume_refs_response(response) { |name| Gitlab::Git.tag_name(name) } end @@ -67,19 +67,19 @@ module Gitlab def local_branches(sort_by: nil) request = Gitaly::FindLocalBranchesRequest.new(repository: @gitaly_repo) request.sort_by = sort_by_param(sort_by) if sort_by - response = GitalyClient.call(@storage, :ref_service, :find_local_branches, request) + response = GitalyClient.call(@storage, :ref_service, :find_local_branches, request, timeout: GitalyClient.fast_timeout) consume_find_local_branches_response(response) end def tags request = Gitaly::FindAllTagsRequest.new(repository: @gitaly_repo) - response = GitalyClient.call(@storage, :ref_service, :find_all_tags, request) + response = GitalyClient.call(@storage, :ref_service, :find_all_tags, request, timeout: GitalyClient.medium_timeout) consume_tags_response(response) end def ref_exists?(ref_name) request = Gitaly::RefExistsRequest.new(repository: @gitaly_repo, ref: encode_binary(ref_name)) - response = GitalyClient.call(@storage, :ref_service, :ref_exists, request) + response = GitalyClient.call(@storage, :ref_service, :ref_exists, request, timeout: GitalyClient.fast_timeout) response.value rescue GRPC::InvalidArgument => e raise ArgumentError, e.message @@ -91,7 +91,7 @@ module Gitlab name: encode_binary(branch_name) ) - response = GitalyClient.call(@repository.storage, :ref_service, :find_branch, request) + response = GitalyClient.call(@repository.storage, :ref_service, :find_branch, request, timeout: GitalyClient.medium_timeout) branch = response.branch return unless branch @@ -140,7 +140,7 @@ module Gitlab except_with_prefix: except_with_prefixes.map { |r| encode_binary(r) } ) - response = GitalyClient.call(@repository.storage, :ref_service, :delete_refs, request) + response = GitalyClient.call(@repository.storage, :ref_service, :delete_refs, request, timeout: GitalyClient.fast_timeout) raise Gitlab::Git::Repository::GitError, response.git_error if response.git_error.present? end @@ -153,7 +153,7 @@ module Gitlab limit: limit ) - stream = GitalyClient.call(@repository.storage, :ref_service, :list_tag_names_containing_commit, request) + stream = GitalyClient.call(@repository.storage, :ref_service, :list_tag_names_containing_commit, request, timeout: GitalyClient.medium_timeout) consume_ref_contains_sha_response(stream, :tag_names) end @@ -166,14 +166,14 @@ module Gitlab limit: limit ) - stream = GitalyClient.call(@repository.storage, :ref_service, :list_branch_names_containing_commit, request) + stream = GitalyClient.call(@repository.storage, :ref_service, :list_branch_names_containing_commit, request, timeout: GitalyClient.medium_timeout) consume_ref_contains_sha_response(stream, :branch_names) end def get_tag_messages(tag_ids) request = Gitaly::GetTagMessagesRequest.new(repository: @gitaly_repo, tag_ids: tag_ids) - response = GitalyClient.call(@repository.storage, :ref_service, :get_tag_messages, request) + response = GitalyClient.call(@repository.storage, :ref_service, :get_tag_messages, request, timeout: GitalyClient.fast_timeout) messages = Hash.new { |h, k| h[k] = ''.b } current_tag_id = nil diff --git a/lib/gitlab/gitaly_client/remote_service.rb b/lib/gitlab/gitaly_client/remote_service.rb index f2d699d9dfb..1381e033d4b 100644 --- a/lib/gitlab/gitaly_client/remote_service.rb +++ b/lib/gitlab/gitaly_client/remote_service.rb @@ -28,13 +28,13 @@ module Gitlab mirror_refmaps: Array.wrap(mirror_refmaps).map(&:to_s) ) - GitalyClient.call(@storage, :remote_service, :add_remote, request) + GitalyClient.call(@storage, :remote_service, :add_remote, request, timeout: GitalyClient.fast_timeout) end def remove_remote(name) request = Gitaly::RemoveRemoteRequest.new(repository: @gitaly_repo, name: name) - response = GitalyClient.call(@storage, :remote_service, :remove_remote, request) + response = GitalyClient.call(@storage, :remote_service, :remove_remote, request, timeout: GitalyClient.fast_timeout) response.result end diff --git a/lib/gitlab/gitaly_client/repository_service.rb b/lib/gitlab/gitaly_client/repository_service.rb index cd0da0f6e88..982f8d0963b 100644 --- a/lib/gitlab/gitaly_client/repository_service.rb +++ b/lib/gitlab/gitaly_client/repository_service.rb @@ -21,7 +21,7 @@ module Gitlab def cleanup request = Gitaly::CleanupRequest.new(repository: @gitaly_repo) - GitalyClient.call(@storage, :repository_service, :cleanup, request) + GitalyClient.call(@storage, :repository_service, :cleanup, request, timeout: GitalyClient.fast_timeout) end def garbage_collect(create_bitmap) @@ -41,13 +41,13 @@ module Gitlab def repository_size request = Gitaly::RepositorySizeRequest.new(repository: @gitaly_repo) - response = GitalyClient.call(@storage, :repository_service, :repository_size, request) + response = GitalyClient.call(@storage, :repository_service, :repository_size, request, timeout: GitalyClient.medium_timeout) response.size end def apply_gitattributes(revision) request = Gitaly::ApplyGitattributesRequest.new(repository: @gitaly_repo, revision: encode_binary(revision)) - GitalyClient.call(@storage, :repository_service, :apply_gitattributes, request) + GitalyClient.call(@storage, :repository_service, :apply_gitattributes, request, timeout: GitalyClient.fast_timeout) rescue GRPC::InvalidArgument => ex raise Gitlab::Git::Repository::InvalidRef, ex end @@ -55,7 +55,7 @@ module Gitlab def info_attributes request = Gitaly::GetInfoAttributesRequest.new(repository: @gitaly_repo) - response = GitalyClient.call(@storage, :repository_service, :get_info_attributes, request) + response = GitalyClient.call(@storage, :repository_service, :get_info_attributes, request, timeout: GitalyClient.fast_timeout) response.each_with_object("") do |message, attributes| attributes << message.attributes end @@ -82,7 +82,7 @@ module Gitlab def create_repository request = Gitaly::CreateRepositoryRequest.new(repository: @gitaly_repo) - GitalyClient.call(@storage, :repository_service, :create_repository, request) + GitalyClient.call(@storage, :repository_service, :create_repository, request, timeout: GitalyClient.medium_timeout) end def has_local_branches? @@ -98,7 +98,7 @@ module Gitlab revisions: revisions.map { |r| encode_binary(r) } ) - response = GitalyClient.call(@storage, :repository_service, :find_merge_base, request) + response = GitalyClient.call(@storage, :repository_service, :find_merge_base, request, timeout: GitalyClient.fast_timeout) response.base.presence end @@ -258,7 +258,7 @@ module Gitlab ) request.old_revision = old_ref.b unless old_ref.nil? - response = GitalyClient.call(@storage, :repository_service, :write_ref, request) + response = GitalyClient.call(@storage, :repository_service, :write_ref, request, timeout: GitalyClient.fast_timeout) raise Gitlab::Git::CommandError, encode!(response.error) if response.error.present? @@ -288,7 +288,7 @@ module Gitlab def calculate_checksum request = Gitaly::CalculateChecksumRequest.new(repository: @gitaly_repo) - response = GitalyClient.call(@storage, :repository_service, :calculate_checksum, request) + response = GitalyClient.call(@storage, :repository_service, :calculate_checksum, request, timeout: GitalyClient.fast_timeout) response.checksum.presence rescue GRPC::DataLoss => e raise Gitlab::Git::Repository::InvalidRepository.new(e) @@ -297,12 +297,12 @@ module Gitlab def raw_changes_between(from, to) request = Gitaly::GetRawChangesRequest.new(repository: @gitaly_repo, from_revision: from, to_revision: to) - GitalyClient.call(@storage, :repository_service, :get_raw_changes, request) + GitalyClient.call(@storage, :repository_service, :get_raw_changes, request, timeout: GitalyClient.fast_timeout) end def search_files_by_name(ref, query) request = Gitaly::SearchFilesByNameRequest.new(repository: @gitaly_repo, ref: ref, query: query) - GitalyClient.call(@storage, :repository_service, :search_files_by_name, request).flat_map(&:files) + GitalyClient.call(@storage, :repository_service, :search_files_by_name, request, timeout: GitalyClient.fast_timeout).flat_map(&:files) end def search_files_by_content(ref, query) diff --git a/lib/gitlab/gitaly_client/wiki_service.rb b/lib/gitlab/gitaly_client/wiki_service.rb index 2dfe055a496..6cb049c1f68 100644 --- a/lib/gitlab/gitaly_client/wiki_service.rb +++ b/lib/gitlab/gitaly_client/wiki_service.rb @@ -69,7 +69,7 @@ module Gitlab commit_details: gitaly_commit_details(commit_details) ) - GitalyClient.call(@repository.storage, :wiki_service, :wiki_delete_page, request) + GitalyClient.call(@repository.storage, :wiki_service, :wiki_delete_page, request, timeout: GitalyClient.medium_timeout) end def find_page(title:, version: nil, dir: nil) @@ -80,14 +80,14 @@ module Gitlab directory: encode_binary(dir) ) - response = GitalyClient.call(@repository.storage, :wiki_service, :wiki_find_page, request) + response = GitalyClient.call(@repository.storage, :wiki_service, :wiki_find_page, request, timeout: GitalyClient.fast_timeout) wiki_page_from_iterator(response) end def get_all_pages request = Gitaly::WikiGetAllPagesRequest.new(repository: @gitaly_repo) - response = GitalyClient.call(@repository.storage, :wiki_service, :wiki_get_all_pages, request) + response = GitalyClient.call(@repository.storage, :wiki_service, :wiki_get_all_pages, request, timeout: GitalyClient.medium_timeout) pages = [] loop do @@ -113,7 +113,7 @@ module Gitlab per_page: options[:per_page] || Gollum::Page.per_page ) - stream = GitalyClient.call(@repository.storage, :wiki_service, :wiki_get_page_versions, request) + stream = GitalyClient.call(@repository.storage, :wiki_service, :wiki_get_page_versions, request, timeout: GitalyClient.medium_timeout) versions = [] stream.each do |message| @@ -132,7 +132,7 @@ module Gitlab revision: encode_binary(revision) ) - response = GitalyClient.call(@repository.storage, :wiki_service, :wiki_find_file, request) + response = GitalyClient.call(@repository.storage, :wiki_service, :wiki_find_file, request, timeout: GitalyClient.fast_timeout) wiki_file = nil response.each do |message| diff --git a/qa/qa/runtime/browser.rb b/qa/qa/runtime/browser.rb index ecd273c6db8..cee381f3379 100644 --- a/qa/qa/runtime/browser.rb +++ b/qa/qa/runtime/browser.rb @@ -32,6 +32,12 @@ module QA end def self.configure! + RSpec.configure do |config| + config.define_derived_metadata(file_path: %r{/qa/specs/features/}) do |metadata| + metadata[:type] = :feature + end + end + return if Capybara.drivers.include?(:chrome) Capybara.register_driver :chrome do |app| diff --git a/spec/controllers/projects/jobs_controller_spec.rb b/spec/controllers/projects/jobs_controller_spec.rb index b10421b8f26..e6332a10265 100644 --- a/spec/controllers/projects/jobs_controller_spec.rb +++ b/spec/controllers/projects/jobs_controller_spec.rb @@ -562,4 +562,105 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do end end end + + describe 'GET #terminal' do + before do + project.add_developer(user) + sign_in(user) + end + + context 'when job exists' do + context 'and it has a terminal' do + let!(:job) { create(:ci_build, :running, :with_runner_session, pipeline: pipeline) } + + it 'has a job' do + get_terminal(id: job.id) + + expect(response).to have_gitlab_http_status(:ok) + expect(assigns(:build).id).to eq(job.id) + end + end + + context 'and does not have a terminal' do + let!(:job) { create(:ci_build, :running, pipeline: pipeline) } + + it 'returns not_found' do + get_terminal(id: job.id) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + context 'when job does not exist' do + it 'renders not_found' do + get_terminal(id: 1234) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + def get_terminal(**extra_params) + params = { + namespace_id: project.namespace.to_param, + project_id: project + } + + get :terminal, params.merge(extra_params) + end + end + + describe 'GET #terminal_websocket_authorize' do + let!(:job) { create(:ci_build, :running, :with_runner_session, pipeline: pipeline) } + + before do + project.add_developer(user) + sign_in(user) + end + + context 'with valid workhorse signature' do + before do + allow(Gitlab::Workhorse).to receive(:verify_api_request!).and_return(nil) + end + + context 'and valid id' do + it 'returns the terminal for the job' do + expect(Gitlab::Workhorse) + .to receive(:terminal_websocket) + .and_return(workhorse: :response) + + get_terminal_websocket(id: job.id) + + expect(response).to have_gitlab_http_status(200) + expect(response.headers["Content-Type"]).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE) + expect(response.body).to eq('{"workhorse":"response"}') + end + end + + context 'and invalid id' do + it 'returns 404' do + get_terminal_websocket(id: 1234) + + expect(response).to have_gitlab_http_status(404) + end + end + end + + context 'with invalid workhorse signature' do + it 'aborts with an exception' do + allow(Gitlab::Workhorse).to receive(:verify_api_request!).and_raise(JWT::DecodeError) + + expect { get_terminal_websocket(id: job.id) }.to raise_error(JWT::DecodeError) + end + end + + def get_terminal_websocket(**extra_params) + params = { + namespace_id: project.namespace.to_param, + project_id: project + } + + get :terminal_websocket_authorize, params.merge(extra_params) + end + end end diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb index 1cc7f33b57a..290fcd4f8e6 100644 --- a/spec/controllers/projects/pipelines_controller_spec.rb +++ b/spec/controllers/projects/pipelines_controller_spec.rb @@ -74,7 +74,7 @@ describe Projects::PipelinesController do expect(stages.count).to eq 3 end - expect(queries.count).to be_within(3).of(30) + expect(queries.count).to be_within(5).of(30) end end diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 27f04be3fdf..34ed835a388 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -616,13 +616,40 @@ describe ProjectsController do end describe 'POST #preview_markdown' do - it 'renders json in a correct format' do + before do sign_in(user) + end + it 'renders json in a correct format' do post :preview_markdown, namespace_id: public_project.namespace, id: public_project, text: '*Markdown* text' expect(JSON.parse(response.body).keys).to match_array(%w(body references)) end + + context 'state filter on references' do + let(:issue) { create(:issue, :closed, project: public_project) } + let(:merge_request) { create(:merge_request, :closed, target_project: public_project) } + + it 'renders JSON body with state filter for issues' do + post :preview_markdown, namespace_id: public_project.namespace, + id: public_project, + text: issue.to_reference + + json_response = JSON.parse(response.body) + + expect(json_response['body']).to match(/\##{issue.iid} \(closed\)/) + end + + it 'renders JSON body with state filter for MRs' do + post :preview_markdown, namespace_id: public_project.namespace, + id: public_project, + text: merge_request.to_reference + + json_response = JSON.parse(response.body) + + expect(json_response['body']).to match(/\!#{merge_request.iid} \(closed\)/) + end + end end describe '#ensure_canonical_path' do diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb index 4acc008ed38..83cb4750741 100644 --- a/spec/factories/ci/builds.rb +++ b/spec/factories/ci/builds.rb @@ -248,5 +248,11 @@ FactoryBot.define do failed failure_reason 2 end + + trait :with_runner_session do + after(:build) do |build| + build.build_runner_session(url: 'ws://localhost') + end + end end end diff --git a/spec/features/dashboard/projects_spec.rb b/spec/features/dashboard/projects_spec.rb index 46935662288..8647616101b 100644 --- a/spec/features/dashboard/projects_spec.rb +++ b/spec/features/dashboard/projects_spec.rb @@ -29,6 +29,34 @@ describe 'Dashboard Projects' do end end + context 'when user has access to the project' do + it 'shows role badge' do + visit dashboard_projects_path + + page.within '.user-access-role' do + expect(page).to have_content('Developer') + end + end + + context 'when role changes', :use_clean_rails_memory_store_fragment_caching do + it 'displays the right role' do + visit dashboard_projects_path + + page.within '.user-access-role' do + expect(page).to have_content('Developer') + end + + project.members.last.update(access_level: 40) + + visit dashboard_projects_path + + page.within '.user-access-role' do + expect(page).to have_content('Maintainer') + end + end + end + end + context 'when last_repository_updated_at, last_activity_at and update_at are present' do it 'shows the last_repository_updated_at attribute as the update date' do project.update_attributes!(last_repository_updated_at: Time.now, last_activity_at: 1.hour.ago) diff --git a/spec/features/projects/issues/user_comments_on_issue_spec.rb b/spec/features/projects/issues/user_comments_on_issue_spec.rb index 353f487485d..ba5b80ed04b 100644 --- a/spec/features/projects/issues/user_comments_on_issue_spec.rb +++ b/spec/features/projects/issues/user_comments_on_issue_spec.rb @@ -63,6 +63,14 @@ describe "User comments on issue", :js do page.within(".current-note-edit-form") do fill_in("note[note]", with: comment) + find('textarea').send_keys [:control, :shift, 'p'] + expect(page).to have_selector('.current-note-edit-form .md-preview-holder') + expect(page.find('.current-note-edit-form .md-preview-holder p')).to have_content(comment) + end + + expect(page).to have_selector('.new-note .note-textarea') + + page.within(".current-note-edit-form") do click_button("Save comment") end diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb index 80147b13739..beb6e8ea273 100644 --- a/spec/helpers/projects_helper_spec.rb +++ b/spec/helpers/projects_helper_spec.rb @@ -80,6 +80,7 @@ describe ProjectsHelper do before do allow(helper).to receive(:current_user).and_return(user) allow(helper).to receive(:can?).with(user, :read_cross_project) { true } + allow(user).to receive(:max_member_access_for_project).and_return(40) end it "includes the route" do @@ -125,6 +126,10 @@ describe ProjectsHelper do expect(helper.project_list_cache_key(project)).to include("pipeline-status/#{project.commit.sha}-success") end + + it "includes the user max member access" do + expect(helper.project_list_cache_key(project)).to include('access:40') + end end describe '#load_pipeline_status' do diff --git a/spec/javascripts/diffs/components/diff_file_header_spec.js b/spec/javascripts/diffs/components/diff_file_header_spec.js index d0f1700bee6..05f5d47ce42 100644 --- a/spec/javascripts/diffs/components/diff_file_header_spec.js +++ b/spec/javascripts/diffs/components/diff_file_header_spec.js @@ -280,11 +280,11 @@ describe('diff_file_header', () => { }); }); - it('displays an icon in the title', () => { + it('displays an file icon in the title', () => { vm = mountComponent(Component, props); - - const icon = vm.$el.querySelector(`i[class="fa fa-fw fa-${vm.icon}"]`); - expect(icon).not.toBe(null); + expect(vm.$el.querySelector('svg.js-file-icon use').getAttribute('xlink:href')).toContain( + 'ruby', + ); }); describe('file paths', () => { diff --git a/spec/javascripts/diffs/store/utils_spec.js b/spec/javascripts/diffs/store/utils_spec.js index 5a024a0f2ad..32136d9ebff 100644 --- a/spec/javascripts/diffs/store/utils_spec.js +++ b/spec/javascripts/diffs/store/utils_spec.js @@ -176,4 +176,35 @@ describe('DiffsStoreUtils', () => { expect(linesWithReferences[1].metaData.newPos).toEqual(3); }); }); + + describe('trimFirstCharOfLineContent', () => { + it('trims the line when it starts with a space', () => { + expect(utils.trimFirstCharOfLineContent({ richText: ' diff' })).toEqual({ richText: 'diff' }); + }); + + it('trims the line when it starts with a +', () => { + expect(utils.trimFirstCharOfLineContent({ richText: '+diff' })).toEqual({ richText: 'diff' }); + }); + + it('trims the line when it starts with a -', () => { + expect(utils.trimFirstCharOfLineContent({ richText: '-diff' })).toEqual({ richText: 'diff' }); + }); + + it('does not trims the line when it starts with a letter', () => { + expect(utils.trimFirstCharOfLineContent({ richText: 'diff' })).toEqual({ richText: 'diff' }); + }); + + it('does not modify the provided object', () => { + const lineObj = { + richText: ' diff', + }; + + utils.trimFirstCharOfLineContent(lineObj); + expect(lineObj).toEqual({ richText: ' diff' }); + }); + + it('handles a undefined or null parameter', () => { + expect(utils.trimFirstCharOfLineContent()).toEqual({}); + }); + }); }); diff --git a/spec/javascripts/fixtures/merge_requests.rb b/spec/javascripts/fixtures/merge_requests.rb index ee60489eb7c..7257d0c8556 100644 --- a/spec/javascripts/fixtures/merge_requests.rb +++ b/spec/javascripts/fixtures/merge_requests.rb @@ -80,6 +80,13 @@ describe Projects::MergeRequestsController, '(JavaScript fixtures)', type: :cont render_discussions_json(merge_request, example.description) end + it 'merge_requests/resolved_diff_discussion.json' do |example| + note = create(:discussion_note_on_merge_request, :resolved, project: project, author: admin, position: position, noteable: merge_request) + create(:system_note, project: project, author: admin, noteable: merge_request, discussion_id: note.discussion.id) + + render_discussions_json(merge_request, example.description) + end + context 'with image diff' do let(:merge_request2) { create(:merge_request_with_diffs, :with_image_diffs, source_project: project, title: "Added images") } let(:image_path) { "files/images/ee_repo_logo.png" } diff --git a/spec/javascripts/notes/components/discussion_counter_spec.js b/spec/javascripts/notes/components/discussion_counter_spec.js index 7b2302e6f47..a3869cc6498 100644 --- a/spec/javascripts/notes/components/discussion_counter_spec.js +++ b/spec/javascripts/notes/components/discussion_counter_spec.js @@ -32,12 +32,12 @@ describe('DiscussionCounter component', () => { { ...discussionMock, id: discussionMock.id, - notes: [{ ...discussionMock.notes[0], resolved: true }], + notes: [{ ...discussionMock.notes[0], resolvable: true, resolved: true }], }, { ...discussionMock, id: discussionMock.id + 1, - notes: [{ ...discussionMock.notes[0], resolved: false }], + notes: [{ ...discussionMock.notes[0], resolvable: true, resolved: false }], }, ]; const firstDiscussionId = discussionMock.id + 1; diff --git a/spec/javascripts/notes/components/noteable_discussion_spec.js b/spec/javascripts/notes/components/noteable_discussion_spec.js index 058ddb6202f..7da931fd9cb 100644 --- a/spec/javascripts/notes/components/noteable_discussion_spec.js +++ b/spec/javascripts/notes/components/noteable_discussion_spec.js @@ -4,22 +4,23 @@ import noteableDiscussion from '~/notes/components/noteable_discussion.vue'; import '~/behaviors/markdown/render_gfm'; import { noteableDataMock, discussionMock, notesDataMock } from '../mock_data'; +const discussionWithTwoUnresolvedNotes = 'merge_requests/resolved_diff_discussion.json'; + describe('noteable_discussion component', () => { + const Component = Vue.extend(noteableDiscussion); let store; let vm; - beforeEach(() => { - const Component = Vue.extend(noteableDiscussion); + preloadFixtures(discussionWithTwoUnresolvedNotes); + beforeEach(() => { store = createStore(); store.dispatch('setNoteableData', noteableDataMock); store.dispatch('setNotesData', notesDataMock); vm = new Component({ store, - propsData: { - discussion: discussionMock, - }, + propsData: { discussion: discussionMock }, }).$mount(); }); @@ -84,7 +85,9 @@ describe('noteable_discussion component', () => { }); it('is true if there are two unresolved discussions', done => { - spyOnProperty(vm, 'unresolvedDiscussions').and.returnValue([{}, {}]); + const discussion = getJSONFixture(discussionWithTwoUnresolvedNotes)[0]; + discussion.notes[0].resolved = false; + vm.$store.dispatch('setInitialNotes', [discussion, discussion]); Vue.nextTick() .then(() => { @@ -105,12 +108,12 @@ describe('noteable_discussion component', () => { { ...discussionMock, id: discussionMock.id + 1, - notes: [{ ...discussionMock.notes[0], resolved: true }], + notes: [{ ...discussionMock.notes[0], resolvable: true, resolved: true }], }, { ...discussionMock, id: discussionMock.id + 2, - notes: [{ ...discussionMock.notes[0], resolved: false }], + notes: [{ ...discussionMock.notes[0], resolvable: true, resolved: false }], }, ]; const nextDiscussionId = discussionMock.id + 2; diff --git a/spec/javascripts/notes/mock_data.js b/spec/javascripts/notes/mock_data.js index 547efa32694..1b040c924aa 100644 --- a/spec/javascripts/notes/mock_data.js +++ b/spec/javascripts/notes/mock_data.js @@ -303,6 +303,7 @@ export const discussionMock = { }, ], individual_note: false, + resolvable: true, }; export const loggedOutnoteableData = { diff --git a/spec/javascripts/notes/stores/getters_spec.js b/spec/javascripts/notes/stores/getters_spec.js index 815cc09621f..41599e00122 100644 --- a/spec/javascripts/notes/stores/getters_spec.js +++ b/spec/javascripts/notes/stores/getters_spec.js @@ -7,9 +7,13 @@ import { collapseNotesMock, } from '../mock_data'; +const discussionWithTwoUnresolvedNotes = 'merge_requests/resolved_diff_discussion.json'; + describe('Getters Notes Store', () => { let state; + preloadFixtures(discussionWithTwoUnresolvedNotes); + beforeEach(() => { state = { discussions: [individualNote], @@ -22,12 +26,26 @@ describe('Getters Notes Store', () => { noteableData: noteableDataMock, }; }); + describe('discussions', () => { it('should return all discussions in the store', () => { expect(getters.discussions(state)).toEqual([individualNote]); }); }); + describe('resolvedDiscussionsById', () => { + it('ignores unresolved system notes', () => { + const [discussion] = getJSONFixture(discussionWithTwoUnresolvedNotes); + discussion.notes[0].resolved = true; + discussion.notes[1].resolved = false; + state.discussions.push(discussion); + + expect(getters.resolvedDiscussionsById(state)).toEqual({ + [discussion.id]: discussion, + }); + }); + }); + describe('Collapsed notes', () => { const stateCollapsedNotes = { discussions: collapseNotesMock, diff --git a/spec/javascripts/vue_shared/components/markdown/header_spec.js b/spec/javascripts/vue_shared/components/markdown/header_spec.js index 02117638b63..488575df401 100644 --- a/spec/javascripts/vue_shared/components/markdown/header_spec.js +++ b/spec/javascripts/vue_shared/components/markdown/header_spec.js @@ -51,7 +51,7 @@ describe('Markdown field header component', () => { spyOn(vm, '$emit'); $(document).triggerHandler('markdown-preview:show', [ - $('<form><textarea class="markdown-area"></textarea></textarea></form>'), + $('<form><div class="js-vue-markdown-field"><textarea class="markdown-area"></textarea></div></form>'), ]); expect(vm.$emit).not.toHaveBeenCalled(); diff --git a/spec/models/ci/build_runner_session_spec.rb b/spec/models/ci/build_runner_session_spec.rb new file mode 100644 index 00000000000..7183957aa50 --- /dev/null +++ b/spec/models/ci/build_runner_session_spec.rb @@ -0,0 +1,36 @@ +require 'spec_helper' + +describe Ci::BuildRunnerSession, model: true do + let!(:build) { create(:ci_build, :with_runner_session) } + + subject { build.runner_session } + + it { is_expected.to belong_to(:build) } + + it { is_expected.to validate_presence_of(:build) } + it { is_expected.to validate_presence_of(:url).with_message('must be a valid URL') } + + describe '#terminal_specification' do + let(:terminal_specification) { subject.terminal_specification } + + it 'returns empty hash if no url' do + subject.url = '' + + expect(terminal_specification).to be_empty + end + + context 'when url is present' do + it 'returns ca_pem nil if empty certificate' do + subject.certificate = '' + + expect(terminal_specification[:ca_pem]).to be_nil + end + + it 'adds Authorization header if authorization is present' do + subject.authorization = 'whatever' + + expect(terminal_specification[:headers]).to include(Authorization: 'whatever') + end + end + end +end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 6758adc59eb..090ca159e08 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -19,6 +19,7 @@ describe Ci::Build do it { is_expected.to belong_to(:erased_by) } it { is_expected.to have_many(:deployments) } it { is_expected.to have_many(:trace_sections)} + it { is_expected.to have_one(:runner_session)} it { is_expected.to validate_presence_of(:ref) } it { is_expected.to respond_to(:has_trace?) } it { is_expected.to respond_to(:trace) } @@ -42,6 +43,20 @@ describe Ci::Build do end end + describe 'status' do + context 'when transitioning to any state from running' do + it 'removes runner_session' do + %w(success drop cancel).each do |event| + build = FactoryBot.create(:ci_build, :running, :with_runner_session, pipeline: pipeline) + + build.fire_events!(event) + + expect(build.reload.runner_session).to be_nil + end + end + end + end + describe '.manual_actions' do let!(:manual_but_created) { create(:ci_build, :manual, status: :created, pipeline: pipeline) } let!(:manual_but_succeeded) { create(:ci_build, :manual, status: :success, pipeline: pipeline) } @@ -499,6 +514,22 @@ describe Ci::Build do end end + describe '#has_old_trace?' do + subject { build.has_old_trace? } + + context 'when old trace exists' do + before do + build.update_column(:trace, 'old trace') + end + + it { is_expected.to be_truthy } + end + + context 'when old trace does not exist' do + it { is_expected.to be_falsy } + end + end + describe '#trace=' do it "expect to fail trace=" do expect { build.trace = "new" }.to raise_error(NotImplementedError) @@ -518,16 +549,32 @@ describe Ci::Build do end describe '#erase_old_trace!' do - subject { build.send(:read_attribute, :trace) } + subject { build.erase_old_trace! } - before do - build.send(:write_attribute, :trace, 'old trace') + context 'when old trace exists' do + before do + build.update_column(:trace, 'old trace') + end + + it "erases old trace" do + subject + + expect(build.old_trace).to be_nil + end + + it "executes UPDATE query" do + recorded = ActiveRecord::QueryRecorder.new { subject } + + expect(recorded.log.select { |l| l.match?(/UPDATE.*ci_builds/) }.count).to eq(1) + end end - it "expect to receive data from database" do - build.erase_old_trace! + context 'when old trace does not exist' do + it 'does not execute UPDATE query' do + recorded = ActiveRecord::QueryRecorder.new { subject } - is_expected.to be_nil + expect(recorded.log.select { |l| l.match?(/UPDATE.*ci_builds/) }.count).to eq(0) + end end end @@ -2605,4 +2652,39 @@ describe Ci::Build do end end end + + describe '#has_terminal?' do + let(:states) { described_class.state_machines[:status].states.keys - [:running] } + + subject { build.has_terminal? } + + it 'returns true if the build is running and it has a runner_session_url' do + build.build_runner_session(url: 'whatever') + build.status = :running + + expect(subject).to be_truthy + end + + context 'returns false' do + it 'when runner_session_url is empty' do + build.status = :running + + expect(subject).to be_falsey + end + + context 'unless the build is running' do + before do + build.build_runner_session(url: 'whatever') + end + + it do + states.each do |state| + build.status = state + + is_expected.to be_falsey + end + end + end + end + end end diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb index 28c908ea425..a849af062c5 100644 --- a/spec/models/service_spec.rb +++ b/spec/models/service_spec.rb @@ -78,7 +78,7 @@ describe Service do context 'when template is invalid' do it 'sets service template to inactive when template is invalid' do project = create(:project) - template = JiraService.new(template: true, active: true) + template = KubernetesService.new(template: true, active: true) template.save(validate: false) service = described_class.build_from_template(project.id, template) diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb index e7639599874..d57993ab454 100644 --- a/spec/requests/api/runner_spec.rb +++ b/spec/requests/api/runner_spec.rb @@ -851,6 +851,7 @@ describe API::Runner, :clean_gitlab_redis_shared_state do it 'does not update job status and job trace' do update_job(state: 'success', trace: 'BUILD TRACE UPDATED') + job.reload expect(response).to have_gitlab_http_status(403) expect(response.header['Job-Status']).to eq 'failed' expect(job.trace.raw).to eq 'Job failed' diff --git a/spec/services/ci/register_job_service_spec.rb b/spec/services/ci/register_job_service_spec.rb index 3816bd0deb5..dbb5e33bbdc 100644 --- a/spec/services/ci/register_job_service_spec.rb +++ b/spec/services/ci/register_job_service_spec.rb @@ -548,8 +548,21 @@ module Ci end end - def execute(runner) - described_class.new(runner).execute.build + context 'when runner_session params are' do + it 'present sets runner session configuration in the build' do + runner_session_params = { session: { 'url' => 'https://example.com' } } + + expect(execute(specific_runner, runner_session_params).runner_session.attributes) + .to include(runner_session_params[:session]) + end + + it 'not present it does not configure the runner session' do + expect(execute(specific_runner).runner_session).to be_nil + end + end + + def execute(runner, params = {}) + described_class.new(runner).execute(params).build end end end diff --git a/spec/services/ci/retry_build_service_spec.rb b/spec/services/ci/retry_build_service_spec.rb index e1cb7ed8110..decb5d22f59 100644 --- a/spec/services/ci/retry_build_service_spec.rb +++ b/spec/services/ci/retry_build_service_spec.rb @@ -32,7 +32,7 @@ describe Ci::RetryBuildService do runner_id tag_taggings taggings tags trigger_request_id user_id auto_canceled_by_id retried failure_reason artifacts_file_store artifacts_metadata_store - metadata trace_chunks].freeze + metadata runner_session trace_chunks].freeze shared_examples 'build duplication' do let(:another_pipeline) { create(:ci_empty_pipeline, project: project) } diff --git a/spec/services/merge_requests/conflicts/list_service_spec.rb b/spec/services/merge_requests/conflicts/list_service_spec.rb index 97da8a88660..d57852615d9 100644 --- a/spec/services/merge_requests/conflicts/list_service_spec.rb +++ b/spec/services/merge_requests/conflicts/list_service_spec.rb @@ -84,5 +84,23 @@ describe MergeRequests::Conflicts::ListService do expect(service.can_be_resolved_in_ui?).to be_falsey end + + context 'with gitaly disabled', :skip_gitaly_mock do + it 'returns a falsey value when the MR has a missing ref after a force push' do + merge_request = create_merge_request('conflict-resolvable') + service = conflicts_service(merge_request) + allow_any_instance_of(Rugged::Repository).to receive(:merge_commits).and_raise(Rugged::OdbError) + + expect(service.can_be_resolved_in_ui?).to be_falsey + end + + it 'returns a falsey value when the MR has a missing revision after a force push' do + merge_request = create_merge_request('conflict-resolvable') + service = conflicts_service(merge_request) + allow(merge_request).to receive_message_chain(:target_branch_head, :raw, :id).and_return(Gitlab::Git::BLANK_SHA) + + expect(service.can_be_resolved_in_ui?).to be_falsey + end + end end end diff --git a/spec/services/merge_requests/conflicts/resolve_service_spec.rb b/spec/services/merge_requests/conflicts/resolve_service_spec.rb index 7edf8a96c94..cff09237005 100644 --- a/spec/services/merge_requests/conflicts/resolve_service_spec.rb +++ b/spec/services/merge_requests/conflicts/resolve_service_spec.rb @@ -123,6 +123,17 @@ describe MergeRequests::Conflicts::ResolveService do expect(merge_request_from_fork.source_branch_head.parents.map(&:id)) .to eq(['404fa3fc7c2c9b5dacff102f353bdf55b1be2813', target_head]) end + + context 'when gitaly is disabled', :skip_gitaly_mock do + it 'gets conflicts from the source project' do + # REFACTOR NOTE: We used to test that `project.repository.rugged` wasn't + # used in this case, but since the refactor, for simplification, + # we always use that repository for read only operations. + expect(forked_project.repository.rugged).to receive(:merge_commits).and_call_original + + subject + end + end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index fdce8e84620..46ec1bcef24 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -170,6 +170,17 @@ RSpec.configure do |config| redis_queues_cleanup! end + config.around(:each, :use_clean_rails_memory_store_fragment_caching) do |example| + caching_store = ActionController::Base.cache_store + ActionController::Base.cache_store = ActiveSupport::Cache::MemoryStore.new + ActionController::Base.perform_caching = true + + example.run + + ActionController::Base.perform_caching = false + ActionController::Base.cache_store = caching_store + end + # The :each scope runs "inside" the example, so this hook ensures the DB is in the # correct state before any examples' before hooks are called. This prevents a # problem where `ScheduleIssuesClosedAtTypeChange` (or any migration that depends diff --git a/spec/support/helpers/wait_for_requests.rb b/spec/support/helpers/wait_for_requests.rb index fda0e29f983..c7f878b7371 100644 --- a/spec/support/helpers/wait_for_requests.rb +++ b/spec/support/helpers/wait_for_requests.rb @@ -24,7 +24,9 @@ module WaitForRequests # Wait for client-side AJAX requests def wait_for_requests - wait_for('JS requests complete') { finished_all_js_requests? } + wait_for('JS requests complete', max_wait_time: 2 * Capybara.default_max_wait_time) do + finished_all_js_requests? + end end # Wait for active Rack requests and client-side AJAX requests diff --git a/spec/support/shared_examples/ci_trace_shared_examples.rb b/spec/support/shared_examples/ci_trace_shared_examples.rb index db723a323f8..94e82b8ce90 100644 --- a/spec/support/shared_examples/ci_trace_shared_examples.rb +++ b/spec/support/shared_examples/ci_trace_shared_examples.rb @@ -138,6 +138,28 @@ shared_examples_for 'common trace features' do end end + describe '#write' do + subject { trace.send(:write, mode) { } } + + let(:mode) { 'wb' } + + context 'when arhicved trace does not exist yet' do + it 'does not raise an error' do + expect { subject }.not_to raise_error + end + end + + context 'when arhicved trace already exists' do + before do + create(:ci_job_artifact, :trace, job: build) + end + + it 'raises an error' do + expect { subject }.to raise_error(Gitlab::Ci::Trace::AlreadyArchivedError) + end + end + end + describe '#set' do before do trace.set("12") @@ -574,7 +596,7 @@ shared_examples_for 'trace with disabled live trace feature' do it 'does not archive' do expect_any_instance_of(described_class).not_to receive(:archive_stream!) - expect { subject }.to raise_error('Already archived') + expect { subject }.to raise_error(Gitlab::Ci::Trace::AlreadyArchivedError) expect(build.job_artifacts_trace.file.exists?).to be_truthy end end @@ -589,6 +611,55 @@ shared_examples_for 'trace with disabled live trace feature' do end end end + + describe '#erase!' do + subject { trace.erase! } + + context 'when it is a live trace' do + context 'when trace is stored in database' do + let(:build) { create(:ci_build) } + + before do + build.update_column(:trace, 'sample trace') + end + + it { expect(trace.raw).not_to be_nil } + + it "removes trace" do + subject + + expect(trace.raw).to be_nil + end + end + + context 'when trace is stored in file storage' do + let(:build) { create(:ci_build, :trace_live) } + + it { expect(trace.raw).not_to be_nil } + + it "removes trace" do + subject + + expect(trace.raw).to be_nil + end + end + end + + context 'when it is an archived trace' do + let(:build) { create(:ci_build, :trace_artifact) } + + it "has trace at first" do + expect(trace.raw).not_to be_nil + end + + it "removes trace" do + subject + + build.reload + expect(trace.raw).to be_nil + end + end + end end shared_examples_for 'trace with enabled live trace feature' do @@ -761,7 +832,7 @@ shared_examples_for 'trace with enabled live trace feature' do it 'does not archive' do expect_any_instance_of(described_class).not_to receive(:archive_stream!) - expect { subject }.to raise_error('Already archived') + expect { subject }.to raise_error(Gitlab::Ci::Trace::AlreadyArchivedError) expect(build.job_artifacts_trace.file.exists?).to be_truthy end end @@ -776,4 +847,35 @@ shared_examples_for 'trace with enabled live trace feature' do end end end + + describe '#erase!' do + subject { trace.erase! } + + context 'when it is a live trace' do + let(:build) { create(:ci_build, :trace_live) } + + it { expect(trace.raw).not_to be_nil } + + it "removes trace" do + subject + + expect(trace.raw).to be_nil + end + end + + context 'when it is an archived trace' do + let(:build) { create(:ci_build, :trace_artifact) } + + it "has trace at first" do + expect(trace.raw).not_to be_nil + end + + it "removes trace" do + subject + + build.reload + expect(trace.raw).to be_nil + end + end + end end diff --git a/spec/workers/ci/archive_traces_cron_worker_spec.rb b/spec/workers/ci/archive_traces_cron_worker_spec.rb index 9af51b7d4d8..23f5dda298a 100644 --- a/spec/workers/ci/archive_traces_cron_worker_spec.rb +++ b/spec/workers/ci/archive_traces_cron_worker_spec.rb @@ -25,24 +25,36 @@ describe Ci::ArchiveTracesCronWorker do end end - context 'when a job was succeeded' do + context 'when a job succeeded' do let!(:build) { create(:ci_build, :success, :trace_live) } it_behaves_like 'archives trace' - context 'when archive raised an exception' do - let!(:build) { create(:ci_build, :success, :trace_artifact, :trace_live) } + context 'when a trace had already been archived' do + let!(:build) { create(:ci_build, :success, :trace_live, :trace_artifact) } let!(:build2) { create(:ci_build, :success, :trace_live) } - it 'archives valid targets' do - expect(Rails.logger).to receive(:error).with("Failed to archive stale live trace. id: #{build.id} message: Already archived") - + it 'continues to archive live traces' do subject build2.reload expect(build2.job_artifacts_trace).to be_exist end end + + context 'when an unexpected exception happened during archiving' do + let!(:build) { create(:ci_build, :success, :trace_live) } + + before do + allow_any_instance_of(Gitlab::Ci::Trace).to receive(:archive!).and_raise('Unexpected error') + end + + it 'puts a log' do + expect(Rails.logger).to receive(:error).with("Failed to archive stale live trace. id: #{build.id} message: Unexpected error") + + subject + end + end end context 'when a job was cancelled' do |