diff options
32 files changed, 570 insertions, 348 deletions
diff --git a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.vue b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.vue index ebedd4842c9..7c5f35579b8 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.vue @@ -115,7 +115,10 @@ export default { data-qa-selector="merge_request_pipeline_info_content" > {{ pipeline.details.name }} - <gl-link :href="pipeline.path" class="pipeline-id font-weight-normal pipeline-number" + <gl-link + :href="pipeline.path" + class="pipeline-id font-weight-normal pipeline-number" + data-qa-selector="pipeline_link" >#{{ pipeline.id }}</gl-link > {{ pipeline.details.status.label }} diff --git a/app/services/git/base_hooks_service.rb b/app/services/git/base_hooks_service.rb index 35a4d2015fa..0d320e96b2e 100644 --- a/app/services/git/base_hooks_service.rb +++ b/app/services/git/base_hooks_service.rb @@ -15,8 +15,6 @@ module Git # Not a hook, but it needs access to the list of changed commits enqueue_invalidate_cache - update_remote_mirrors - success end @@ -121,13 +119,6 @@ module Git {} end - def update_remote_mirrors - return unless project.has_remote_mirror? - - project.mark_stuck_remote_mirrors_as_failed! - project.update_remote_mirrors - end - def log_pipeline_errors(exception) data = { class: self.class.name, diff --git a/app/services/git/branch_push_service.rb b/app/services/git/branch_push_service.rb index 49c54e42b7c..7adc3320e06 100644 --- a/app/services/git/branch_push_service.rb +++ b/app/services/git/branch_push_service.rb @@ -57,13 +57,6 @@ module Git Ci::StopEnvironmentsService.new(project, current_user).execute(branch_name) end - def update_remote_mirrors - return unless project.has_remote_mirror? - - project.mark_stuck_remote_mirrors_as_failed! - project.update_remote_mirrors - end - def execute_related_hooks BranchHooksService.new(project, current_user, params).execute end diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb index c01094bd689..2cc8771bfe8 100644 --- a/app/services/system_note_service.rb +++ b/app/services/system_note_service.rb @@ -16,20 +16,9 @@ module SystemNoteService # existing_commits - Array of Commits added in a previous push # oldrev - Optional String SHA of a previous Commit # - # See new_commit_summary and existing_commit_summary. - # # Returns the created Note object def add_commits(noteable, project, author, new_commits, existing_commits = [], oldrev = nil) - total_count = new_commits.length + existing_commits.length - commits_text = "#{total_count} commit".pluralize(total_count) - - text_parts = ["added #{commits_text}"] - text_parts << commits_list(noteable, new_commits, existing_commits, oldrev) - text_parts << "[Compare with previous version](#{diff_comparison_path(noteable, project, oldrev)})" - - body = text_parts.join("\n\n") - - create_note(NoteSummary.new(noteable, project, author, body, action: 'commit', commit_count: total_count)) + ::SystemNotes::CommitService.new(noteable: noteable, project: project, author: author).add_commits(new_commits, existing_commits, oldrev) end # Called when a commit was tagged @@ -41,10 +30,7 @@ module SystemNoteService # # Returns the created Note object def tag_commit(noteable, project, author, tag_name) - link = url_helpers.project_tag_path(project, id: tag_name) - body = "tagged commit #{noteable.sha} to [`#{tag_name}`](#{link})" - - create_note(NoteSummary.new(noteable, project, author, body, action: 'tag')) + ::SystemNotes::CommitService.new(noteable: noteable, project: project, author: author).tag_commit(tag_name) end # Called when the assignee of a Noteable is changed or removed @@ -497,17 +483,6 @@ module SystemNoteService notes_for_mentioner(mentioner, noteable, notes).exists? end - # Build an Array of lines detailing each commit added in a merge request - # - # new_commits - Array of new Commit objects - # - # Returns an Array of Strings - def new_commit_summary(new_commits) - new_commits.collect do |commit| - content_tag('li', "#{commit.short_id} - #{commit.title}") - end - end - # Called when the status of a Task has changed # # noteable - Noteable object. @@ -637,71 +612,10 @@ module SystemNoteService "#{cross_reference_note_prefix}#{gfm_reference}" end - # Builds a list of existing and new commits according to existing_commits and - # new_commits methods. - # Returns a String wrapped in `ul` and `li` tags. - def commits_list(noteable, new_commits, existing_commits, oldrev) - existing_commit_summary = existing_commit_summary(noteable, existing_commits, oldrev) - new_commit_summary = new_commit_summary(new_commits).join - - content_tag('ul', "#{existing_commit_summary}#{new_commit_summary}".html_safe) - end - - # Build a single line summarizing existing commits being added in a merge - # request - # - # noteable - MergeRequest object - # existing_commits - Array of existing Commit objects - # oldrev - Optional String SHA of a previous Commit - # - # Examples: - # - # "* ea0f8418...2f4426b7 - 24 commits from branch `master`" - # - # "* ea0f8418..4188f0ea - 15 commits from branch `fork:master`" - # - # "* ea0f8418 - 1 commit from branch `feature`" - # - # Returns a newline-terminated String - def existing_commit_summary(noteable, existing_commits, oldrev = nil) - return '' if existing_commits.empty? - - count = existing_commits.size - - commit_ids = if count == 1 - existing_commits.first.short_id - else - if oldrev && !Gitlab::Git.blank_ref?(oldrev) - "#{Commit.truncate_sha(oldrev)}...#{existing_commits.last.short_id}" - else - "#{existing_commits.first.short_id}..#{existing_commits.last.short_id}" - end - end - - commits_text = "#{count} commit".pluralize(count) - - branch = noteable.target_branch - branch = "#{noteable.target_project_namespace}:#{branch}" if noteable.for_fork? - - branch_name = content_tag('code', branch) - content_tag('li', "#{commit_ids} - #{commits_text} from branch #{branch_name}".html_safe) - end - def url_helpers @url_helpers ||= Gitlab::Routing.url_helpers end - def diff_comparison_path(merge_request, project, oldrev) - diff_id = merge_request.merge_request_diff.id - - url_helpers.diffs_project_merge_request_path( - project, - merge_request, - diff_id: diff_id, - start_sha: oldrev - ) - end - def content_tag(*args) ActionController::Base.helpers.content_tag(*args) end diff --git a/app/services/system_notes/base_service.rb b/app/services/system_notes/base_service.rb new file mode 100644 index 00000000000..7341a25b133 --- /dev/null +++ b/app/services/system_notes/base_service.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +module SystemNotes + class BaseService + attr_reader :noteable, :project, :author + + def initialize(noteable: nil, author: nil, project: nil) + @noteable = noteable + @project = project + @author = author + end + + protected + + def create_note(note_summary) + note = Note.create(note_summary.note.merge(system: true)) + note.system_note_metadata = SystemNoteMetadata.new(note_summary.metadata) if note_summary.metadata? + + note + end + + def content_tag(*args) + ActionController::Base.helpers.content_tag(*args) + end + + def url_helpers + @url_helpers ||= Gitlab::Routing.url_helpers + end + end +end diff --git a/app/services/system_notes/commit_service.rb b/app/services/system_notes/commit_service.rb new file mode 100644 index 00000000000..11119956e0f --- /dev/null +++ b/app/services/system_notes/commit_service.rb @@ -0,0 +1,112 @@ +# frozen_string_literal: true + +module SystemNotes + class CommitService < ::SystemNotes::BaseService + # Called when commits are added to a Merge Request + # + # new_commits - Array of Commits added since last push + # existing_commits - Array of Commits added in a previous push + # oldrev - Optional String SHA of a previous Commit + # + # See new_commit_summary and existing_commit_summary. + # + # Returns the created Note object + def add_commits(new_commits, existing_commits = [], oldrev = nil) + total_count = new_commits.length + existing_commits.length + commits_text = "#{total_count} commit".pluralize(total_count) + + text_parts = ["added #{commits_text}"] + text_parts << commits_list(noteable, new_commits, existing_commits, oldrev) + text_parts << "[Compare with previous version](#{diff_comparison_path(noteable, project, oldrev)})" + + body = text_parts.join("\n\n") + + create_note(NoteSummary.new(noteable, project, author, body, action: 'commit', commit_count: total_count)) + end + + # Called when a commit was tagged + # + # tag_name - The created tag name + # + # Returns the created Note object + def tag_commit(tag_name) + link = url_helpers.project_tag_path(project, id: tag_name) + body = "tagged commit #{noteable.sha} to [`#{tag_name}`](#{link})" + + create_note(NoteSummary.new(noteable, project, author, body, action: 'tag')) + end + + # Build an Array of lines detailing each commit added in a merge request + # + # new_commits - Array of new Commit objects + # + # Returns an Array of Strings + def new_commit_summary(new_commits) + new_commits.collect do |commit| + content_tag('li', "#{commit.short_id} - #{commit.title}") + end + end + + private + + # Builds a list of existing and new commits according to existing_commits and + # new_commits methods. + # Returns a String wrapped in `ul` and `li` tags. + def commits_list(noteable, new_commits, existing_commits, oldrev) + existing_commit_summary = existing_commit_summary(noteable, existing_commits, oldrev) + new_commit_summary = new_commit_summary(new_commits).join + + content_tag('ul', "#{existing_commit_summary}#{new_commit_summary}".html_safe) + end + + # Build a single line summarizing existing commits being added in a merge + # request + # + # existing_commits - Array of existing Commit objects + # oldrev - Optional String SHA of a previous Commit + # + # Examples: + # + # "* ea0f8418...2f4426b7 - 24 commits from branch `master`" + # + # "* ea0f8418..4188f0ea - 15 commits from branch `fork:master`" + # + # "* ea0f8418 - 1 commit from branch `feature`" + # + # Returns a newline-terminated String + def existing_commit_summary(noteable, existing_commits, oldrev = nil) + return '' if existing_commits.empty? + + count = existing_commits.size + + commit_ids = if count == 1 + existing_commits.first.short_id + else + if oldrev && !Gitlab::Git.blank_ref?(oldrev) + "#{Commit.truncate_sha(oldrev)}...#{existing_commits.last.short_id}" + else + "#{existing_commits.first.short_id}..#{existing_commits.last.short_id}" + end + end + + commits_text = "#{count} commit".pluralize(count) + + branch = noteable.target_branch + branch = "#{noteable.target_project_namespace}:#{branch}" if noteable.for_fork? + + branch_name = content_tag('code', branch) + content_tag('li', "#{commit_ids} - #{commits_text} from branch #{branch_name}".html_safe) + end + + def diff_comparison_path(merge_request, project, oldrev) + diff_id = merge_request.merge_request_diff.id + + url_helpers.diffs_project_merge_request_path( + project, + merge_request, + diff_id: diff_id, + start_sha: oldrev + ) + end + end +end diff --git a/app/workers/post_receive.rb b/app/workers/post_receive.rb index 63a32083b5d..cae3cb45c45 100644 --- a/app/workers/post_receive.rb +++ b/app/workers/post_receive.rb @@ -70,6 +70,7 @@ class PostReceive refs << ref end + update_remote_mirrors(post_received) after_project_changes_hooks(post_received, user, refs.to_a, changes) end @@ -93,6 +94,16 @@ class PostReceive ) end + def update_remote_mirrors(post_received) + return unless post_received.includes_branches? || post_received.includes_tags? + + project = post_received.project + return unless project.has_remote_mirror? + + project.mark_stuck_remote_mirrors_as_failed! + project.update_remote_mirrors + end + def after_project_changes_hooks(post_received, user, refs, changes) hook_data = Gitlab::DataBuilder::Repository.update(post_received.project, user, changes, refs) SystemHooksService.new.execute_hooks(hook_data, :repository_update_hooks) diff --git a/changelogs/unreleased/bvl-only-one-pushmirror-per-push.yml b/changelogs/unreleased/bvl-only-one-pushmirror-per-push.yml new file mode 100644 index 00000000000..29090739d8b --- /dev/null +++ b/changelogs/unreleased/bvl-only-one-pushmirror-per-push.yml @@ -0,0 +1,5 @@ +--- +title: Only schedule updating push-mirrors once per push +merge_request: 17902 +author: +type: performance diff --git a/changelogs/unreleased/sh-enable-google-api-retries.yml b/changelogs/unreleased/sh-enable-google-api-retries.yml new file mode 100644 index 00000000000..5c6b10faa91 --- /dev/null +++ b/changelogs/unreleased/sh-enable-google-api-retries.yml @@ -0,0 +1,5 @@ +--- +title: Enable Google API retries for uploads +merge_request: 18040 +author: +type: fixed diff --git a/config/initializers/google_api_client.rb b/config/initializers/google_api_client.rb index 71338d2d9e1..443bb29fb52 100644 --- a/config/initializers/google_api_client.rb +++ b/config/initializers/google_api_client.rb @@ -10,6 +10,12 @@ # require 'google/apis/container_v1beta1' +require 'google/apis/options' + +# As stated in https://github.com/googleapis/google-api-ruby-client#errors--retries, +# enabling retries is strongly encouraged but disabled by default. Large uploads +# that may hit timeouts will mainly benefit from this. +Google::Apis::RequestOptions.default.retries = 3 if Gitlab::Utils.to_boolean(ENV.fetch('ENABLE_GOOGLE_API_RETRIES', true)) Google::Apis::ContainerV1beta1::AddonsConfig::Representation.tap do |representation| representation.hash :cloud_run_config, as: 'cloudRunConfig' diff --git a/doc/administration/high_availability/monitoring_node.md b/doc/administration/high_availability/monitoring_node.md index 0b04e48f74a..d293fc350fa 100644 --- a/doc/administration/high_availability/monitoring_node.md +++ b/doc/administration/high_availability/monitoring_node.md @@ -34,6 +34,9 @@ Omnibus: prometheus['listen_address'] = '0.0.0.0:9090' prometheus['monitor_kubernetes'] = false + # Enable Login form + grafana['disable_login_form'] = false + # Enable Grafana grafana['enable'] = true grafana['admin_password'] = 'toomanysecrets' @@ -63,6 +66,7 @@ Omnibus: sidekiq['enable'] = false unicorn['enable'] = false node_exporter['enable'] = false + gitlab_exporter['enable'] = false ``` 1. Run `sudo gitlab-ctl reconfigure` to compile the configuration. diff --git a/doc/user/permissions.md b/doc/user/permissions.md index a46181bb74a..4381012abf5 100644 --- a/doc/user/permissions.md +++ b/doc/user/permissions.md @@ -48,7 +48,7 @@ The following table depicts the various user permission levels in a project. | View Insights charts **(ULTIMATE)** | ✓ | ✓ | ✓ | ✓ | ✓ | | View approved/blacklisted licenses **(ULTIMATE)** | ✓ | ✓ | ✓ | ✓ | ✓ | | View License Compliance reports **(ULTIMATE)** | ✓ (*1*) | ✓ | ✓ | ✓ | ✓ | -| View Security reports **(ULTIMATE)** | ✓ (*1*) | ✓ | ✓ | ✓ | ✓ | +| View Security reports **(ULTIMATE)** | ✓ (*3*) | ✓ | ✓ | ✓ | ✓ | | View Dependency list **(ULTIMATE)** | ✓ (*1*) | ✓ | ✓ | ✓ | ✓ | | View licenses in Dependency list **(ULTIMATE)** | ✓ (*1*) | ✓ | ✓ | ✓ | ✓ | | View [Design Management](project/issues/design_management.md) pages **(PREMIUM)** | ✓ (*1*) | ✓ | ✓ | ✓ | ✓ | diff --git a/lib/gitlab/import_export/fast_hash_serializer.rb b/lib/gitlab/import_export/fast_hash_serializer.rb index a6ab4f3a3d9..e2d5c5ae055 100644 --- a/lib/gitlab/import_export/fast_hash_serializer.rb +++ b/lib/gitlab/import_export/fast_hash_serializer.rb @@ -26,6 +26,51 @@ module Gitlab class FastHashSerializer attr_reader :subject, :tree + # Usage of this class results in delayed + # serialization of relation. The serialization + # will be triggered when the `JSON.generate` + # is exected. + # + # This class uses memory-optimised, lazily + # initialised, fast to recycle relation + # serialization. + # + # The `JSON.generate` does use `#to_json`, + # that returns raw JSON content that is written + # directly to file. + class JSONBatchRelation + include Gitlab::Utils::StrongMemoize + + def initialize(relation, options, preloads) + @relation = relation + @options = options + @preloads = preloads + end + + def raw_json + strong_memoize(:raw_json) do + result = +'' + + batch = @relation + batch = batch.preload(@preloads) if @preloads + batch.each do |item| + result.concat(",") unless result.empty? + result.concat(item.to_json(@options)) + end + + result + end + end + + def to_json(options = {}) + raw_json + end + + def as_json(*) + raise NotImplementedError + end + end + BATCH_SIZE = 100 def initialize(subject, tree, batch_size: BATCH_SIZE) @@ -34,8 +79,11 @@ module Gitlab @tree = tree end - # Serializes the subject into a Hash for the given option tree - # (e.g. Project#as_json) + # With the usage of `JSONBatchRelation`, it returns partially + # serialized hash which is not easily accessible. + # It means you can only manipulate and replace top-level objects. + # All future mutations of the hash (such as `fix_project_tree`) + # should be aware of that. def execute simple_serialize.merge(serialize_includes) end @@ -85,13 +133,20 @@ module Gitlab return record.as_json(options) end - # has-many relation data = [] - record.in_batches(of: @batch_size) do |batch| # rubocop:disable Cop/InBatches - batch = batch.preload(preloads[key]) if preloads&.key?(key) - data += batch.as_json(options) + # rubocop:disable Cop/InBatches + # If we put `rubocop:disable` inline after `do |batch|`, + # `Cop/LineBreakAroundConditionalBlock` will fail + record.in_batches(of: @batch_size) do |batch| + if Feature.enabled?(:export_fast_serialize_with_raw_json, default_enabled: true) + data.append(JSONBatchRelation.new(batch, options, preloads[key]).tap(&:raw_json)) + else + batch = batch.preload(preloads[key]) if preloads&.key?(key) + data += batch.as_json(options) + end end + # rubocop:enable Cop/InBatches data end diff --git a/lib/gitlab/import_export/project_tree_saver.rb b/lib/gitlab/import_export/project_tree_saver.rb index f75f69b2c75..63c71105efe 100644 --- a/lib/gitlab/import_export/project_tree_saver.rb +++ b/lib/gitlab/import_export/project_tree_saver.rb @@ -20,7 +20,8 @@ module Gitlab project_tree = serialize_project_tree fix_project_tree(project_tree) - File.write(full_path, project_tree.to_json) + project_tree_json = JSON.generate(project_tree) + File.write(full_path, project_tree_json) true rescue => e @@ -30,6 +31,8 @@ module Gitlab private + # Aware that the resulting hash needs to be pure-hash and + # does not include any AR objects anymore, only objects that run `.to_json` def fix_project_tree(project_tree) if @params[:description].present? project_tree['description'] = @params[:description] @@ -357,6 +357,7 @@ module QA # Classes describing components that are used by several pages. # module Component + autoload :CiBadgeLink, 'qa/page/component/ci_badge_link' autoload :ClonePanel, 'qa/page/component/clone_panel' autoload :LazyLoader, 'qa/page/component/lazy_loader' autoload :LegacyClonePanel, 'qa/page/component/legacy_clone_panel' diff --git a/qa/qa/page/base.rb b/qa/qa/page/base.rb index a6ae0767cb4..71df90f2f42 100644 --- a/qa/qa/page/base.rb +++ b/qa/qa/page/base.rb @@ -95,7 +95,7 @@ module QA # replace with (..., page = self.class) def click_element(name, page = nil, text: nil) - find_element(name, text: nil).click + find_element(name, text: text).click page.validate_elements_present! if page end diff --git a/qa/qa/page/component/ci_badge_link.rb b/qa/qa/page/component/ci_badge_link.rb new file mode 100644 index 00000000000..aad8dc1d3df --- /dev/null +++ b/qa/qa/page/component/ci_badge_link.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +module QA + module Page + module Component + module CiBadgeLink + COMPLETED_STATUSES = %w[passed failed canceled blocked skipped manual].freeze # excludes created, pending, running + PASSED_STATUS = 'passed'.freeze + + def self.included(base) + base.view 'app/assets/javascripts/vue_shared/components/ci_badge_link.vue' do + element :status_badge + end + end + + def status_badge + find_element(:status_badge).text + end + + def successful?(timeout: 60) + raise "Timed out waiting for the status to be a valid completed state" unless completed?(timeout: timeout) + + status_badge == PASSED_STATUS + end + + private + + def completed?(timeout: 60) + wait(reload: false, max: timeout) do + COMPLETED_STATUSES.include?(status_badge) + end + end + end + end + end +end diff --git a/qa/qa/page/merge_request/show.rb b/qa/qa/page/merge_request/show.rb index 6b4a8bacf24..14b8c420b16 100644 --- a/qa/qa/page/merge_request/show.rb +++ b/qa/qa/page/merge_request/show.rb @@ -14,6 +14,7 @@ module QA view 'app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.vue' do element :merge_request_pipeline_info_content + element :pipeline_link end view 'app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue' do @@ -59,6 +60,18 @@ module QA element :edit_button end + def click_discussions_tab + click_element :notes_tab + end + + def click_diffs_tab + click_element :diffs_tab + end + + def click_pipeline_link + click_element :pipeline_link + end + def fast_forward_possible? has_no_text?('Fast-forward merge is not possible') end @@ -156,14 +169,6 @@ module QA click_element :squash_checkbox end - def click_discussions_tab - click_element :notes_tab - end - - def click_diffs_tab - click_element :diffs_tab - end - def add_comment_to_diff(text) wait(interval: 5) do has_text?("No newline at end of file") diff --git a/qa/qa/page/project/job/show.rb b/qa/qa/page/project/job/show.rb index 751bdb32506..cf847710024 100644 --- a/qa/qa/page/project/job/show.rb +++ b/qa/qa/page/project/job/show.rb @@ -3,17 +3,12 @@ module QA::Page module Project::Job class Show < QA::Page::Base - COMPLETED_STATUSES = %w[passed failed canceled blocked skipped manual].freeze # excludes created, pending, running - PASSED_STATUS = 'passed'.freeze + include Component::CiBadgeLink view 'app/assets/javascripts/jobs/components/job_log.vue' do element :build_trace end - view 'app/assets/javascripts/vue_shared/components/ci_badge_link.vue' do - element :status_badge - end - view 'app/assets/javascripts/jobs/components/stages_dropdown.vue' do element :pipeline_path end @@ -45,16 +40,6 @@ module QA::Page has_element?(:build_trace, wait: 1) end end - - def completed?(timeout: 60) - wait(reload: false, max: timeout) do - COMPLETED_STATUSES.include?(status_badge) - end - end - - def status_badge - find_element(:status_badge).text - end end end end diff --git a/qa/qa/page/project/pipeline/show.rb b/qa/qa/page/project/pipeline/show.rb index 3dca47a57e9..ce81bace01e 100644 --- a/qa/qa/page/project/pipeline/show.rb +++ b/qa/qa/page/project/pipeline/show.rb @@ -3,6 +3,8 @@ module QA::Page module Project::Pipeline class Show < QA::Page::Base + include Component::CiBadgeLink + view 'app/assets/javascripts/vue_shared/components/header_ci_component.vue' do element :pipeline_header, /header class.*ci-header-container.*/ # rubocop:disable QA/ElementWithPattern end @@ -38,6 +40,14 @@ module QA::Page end end + def has_job?(job_name) + has_element?(:job_link, text: job_name) + end + + def has_no_job?(job_name) + has_no_element?(:job_link, text: job_name) + end + def has_tag?(tag_name) within_element(:pipeline_badges) do has_selector?('.badge', text: tag_name) @@ -45,7 +55,11 @@ module QA::Page end def click_job(job_name) - find_element(:job_link, text: job_name).click + click_element(:job_link, text: job_name) + end + + def click_linked_job(project_name) + click_element(:linked_pipeline_button, text: /#{project_name}/) end def click_on_first_job diff --git a/qa/qa/resource/project.rb b/qa/qa/resource/project.rb index a0389390c83..caaa766e982 100644 --- a/qa/qa/resource/project.rb +++ b/qa/qa/resource/project.rb @@ -88,6 +88,10 @@ module QA "#{api_get_path}/members" end + def api_runners_path + "#{api_get_path}/runners" + end + def api_post_path '/projects' end @@ -108,6 +112,11 @@ module QA post_body end + def runners + response = get Runtime::API::Request.new(api_client, api_runners_path).url + parse_body(response) + end + def share_with_group(invitee, access_level = Resource::Members::AccessLevel::DEVELOPER) post Runtime::API::Request.new(api_client, "/projects/#{id}/share").url, { group_id: invitee.id, group_access: access_level } end diff --git a/qa/qa/resource/runner.rb b/qa/qa/resource/runner.rb index 9c2e138bade..3f0eed7458a 100644 --- a/qa/qa/resource/runner.rb +++ b/qa/qa/resource/runner.rb @@ -6,8 +6,9 @@ module QA module Resource class Runner < Base attr_writer :name, :tags, :image - attr_accessor :config + attr_accessor :config, :token + attribute :id attribute :project do Project.fabricate_via_api! do |resource| resource.name = 'project-with-ci-cd' @@ -30,7 +31,7 @@ module QA def fabricate_via_api! Service::Runner.new(name).tap do |runner| runner.pull - runner.token = project.runners_token + runner.token = @token ||= project.runners_token runner.address = Runtime::Scenario.gitlab_address runner.tags = tags runner.image = image @@ -40,6 +41,18 @@ module QA end end + def remove_via_api! + @id = project.runners.find { |runner| runner[:description] == name }[:id] + + super + + Service::Runner.new(name).remove! + end + + def api_delete_path + "/runners/#{id}" + end + def api_get_path end diff --git a/qa/qa/resource/sandbox.rb b/qa/qa/resource/sandbox.rb index 47bd86440a0..6ee3dcf350f 100644 --- a/qa/qa/resource/sandbox.rb +++ b/qa/qa/resource/sandbox.rb @@ -10,6 +10,7 @@ module QA attr_accessor :path attribute :id + attribute :runners_token def initialize @path = Runtime::Namespace.sandbox_name diff --git a/spec/lib/gitlab/import_export/fast_hash_serializer_spec.rb b/spec/lib/gitlab/import_export/fast_hash_serializer_spec.rb index d23b27c9d8e..934e676d020 100644 --- a/spec/lib/gitlab/import_export/fast_hash_serializer_spec.rb +++ b/spec/lib/gitlab/import_export/fast_hash_serializer_spec.rb @@ -1,7 +1,12 @@ require 'spec_helper' describe Gitlab::ImportExport::FastHashSerializer do - subject { described_class.new(project, tree).execute } + # FastHashSerializer#execute generates the hash which is not easily accessible + # and includes `JSONBatchRelation` items which are serialized at this point. + # Wrapping the result into JSON generating/parsing is for making + # the testing more convenient. Doing this, we can check that + # all items are properly serialized while traversing the simple hash. + subject { JSON.parse(JSON.generate(described_class.new(project, tree).execute)) } let!(:project) { setup_project } let(:user) { create(:user) } diff --git a/spec/services/git/base_hooks_service_spec.rb b/spec/services/git/base_hooks_service_spec.rb index 874df9a68cd..6dc0353299b 100644 --- a/spec/services/git/base_hooks_service_spec.rb +++ b/spec/services/git/base_hooks_service_spec.rb @@ -85,78 +85,4 @@ describe Git::BaseHooksService do end end end - - describe 'with remote mirrors' do - class TestService < described_class - def commits - [] - end - end - - let(:project) { create(:project, :repository, :remote_mirror) } - - subject { TestService.new(project, user, oldrev: oldrev, newrev: newrev, ref: ref) } - - before do - expect(subject).to receive(:execute_project_hooks) - end - - context 'when remote mirror feature is enabled' do - it 'fails stuck remote mirrors' do - allow(project).to receive(:update_remote_mirrors).and_return(project.remote_mirrors) - expect(project).to receive(:mark_stuck_remote_mirrors_as_failed!) - - subject.execute - end - - it 'updates remote mirrors' do - expect(project).to receive(:update_remote_mirrors) - - subject.execute - end - end - - context 'when remote mirror feature is disabled' do - before do - stub_application_setting(mirror_available: false) - end - - context 'with remote mirrors global setting overridden' do - before do - project.remote_mirror_available_overridden = true - end - - it 'fails stuck remote mirrors' do - allow(project).to receive(:update_remote_mirrors).and_return(project.remote_mirrors) - expect(project).to receive(:mark_stuck_remote_mirrors_as_failed!) - - subject.execute - end - - it 'updates remote mirrors' do - expect(project).to receive(:update_remote_mirrors) - - subject.execute - end - end - - context 'without remote mirrors global setting overridden' do - before do - project.remote_mirror_available_overridden = false - end - - it 'does not fails stuck remote mirrors' do - expect(project).not_to receive(:mark_stuck_remote_mirrors_as_failed!) - - subject.execute - end - - it 'does not updates remote mirrors' do - expect(project).not_to receive(:update_remote_mirrors) - - subject.execute - end - end - end - end end diff --git a/spec/services/git/branch_hooks_service_spec.rb b/spec/services/git/branch_hooks_service_spec.rb index 2bf7dc32436..206b87fb889 100644 --- a/spec/services/git/branch_hooks_service_spec.rb +++ b/spec/services/git/branch_hooks_service_spec.rb @@ -19,12 +19,6 @@ describe Git::BranchHooksService do described_class.new(project, user, oldrev: oldrev, newrev: newrev, ref: ref) end - it 'update remote mirrors' do - expect(service).to receive(:update_remote_mirrors).and_call_original - - service.execute - end - describe "Git Push Data" do subject(:push_data) { service.send(:push_data) } diff --git a/spec/services/git/tag_hooks_service_spec.rb b/spec/services/git/tag_hooks_service_spec.rb index e362577d289..1b8e0675f1b 100644 --- a/spec/services/git/tag_hooks_service_spec.rb +++ b/spec/services/git/tag_hooks_service_spec.rb @@ -18,12 +18,6 @@ describe Git::TagHooksService, :service do described_class.new(project, user, oldrev: oldrev, newrev: newrev, ref: ref) end - it 'update remote mirrors' do - expect(service).to receive(:update_remote_mirrors).and_call_original - - service.execute - end - describe 'System hooks' do it 'Executes system hooks' do push_data = service.send(:push_data) diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index 910fe3b50b7..ad87f9e2f23 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -14,127 +14,29 @@ describe SystemNoteService do let(:noteable) { create(:issue, project: project) } let(:issue) { noteable } - shared_examples_for 'a note with overridable created_at' do - let(:noteable) { create(:issue, project: project, system_note_timestamp: Time.at(42)) } - - it 'the note has the correct time' do - expect(subject.created_at).to eq Time.at(42) - end - end - - shared_examples_for 'a system note' do - let(:expected_noteable) { noteable } - let(:commit_count) { nil } - - it 'has the correct attributes', :aggregate_failures do - expect(subject).to be_valid - expect(subject).to be_system - - expect(subject.noteable).to eq expected_noteable - expect(subject.project).to eq project - expect(subject.author).to eq author - - expect(subject.system_note_metadata.action).to eq(action) - expect(subject.system_note_metadata.commit_count).to eq(commit_count) - end - end - describe '.add_commits' do - subject { described_class.add_commits(noteable, project, author, new_commits, old_commits, oldrev) } - - let(:noteable) { create(:merge_request, source_project: project, target_project: project) } - let(:new_commits) { noteable.commits } - let(:old_commits) { [] } - let(:oldrev) { nil } - - it_behaves_like 'a system note' do - let(:commit_count) { new_commits.size } - let(:action) { 'commit' } - end + let(:new_commits) { double } + let(:old_commits) { double } + let(:oldrev) { double } - describe 'note body' do - let(:note_lines) { subject.note.split("\n").reject(&:blank?) } - - describe 'comparison diff link line' do - it 'adds the comparison text' do - expect(note_lines[2]).to match "[Compare with previous version]" - end + it 'calls CommitService' do + expect_next_instance_of(::SystemNotes::CommitService) do |service| + expect(service).to receive(:add_commits).with(new_commits, old_commits, oldrev) end - context 'without existing commits' do - it 'adds a message header' do - expect(note_lines[0]).to eq "added #{new_commits.size} commits" - end - - it 'adds a message for each commit' do - decoded_note_content = HTMLEntities.new.decode(subject.note) - - new_commits.each do |commit| - expect(decoded_note_content).to include("<li>#{commit.short_id} - #{commit.title}</li>") - end - end - end - - describe 'summary line for existing commits' do - let(:summary_line) { note_lines[1] } - - context 'with one existing commit' do - let(:old_commits) { [noteable.commits.last] } - - it 'includes the existing commit' do - expect(summary_line).to start_with("<ul><li>#{old_commits.first.short_id} - 1 commit from branch <code>feature</code>") - end - end - - context 'with multiple existing commits' do - let(:old_commits) { noteable.commits[3..-1] } - - context 'with oldrev' do - let(:oldrev) { noteable.commits[2].id } - - it 'includes a commit range and count' do - expect(summary_line) - .to start_with("<ul><li>#{Commit.truncate_sha(oldrev)}...#{old_commits.last.short_id} - 26 commits from branch <code>feature</code>") - end - end - - context 'without oldrev' do - it 'includes a commit range and count' do - expect(summary_line) - .to start_with("<ul><li>#{old_commits[0].short_id}..#{old_commits[-1].short_id} - 26 commits from branch <code>feature</code>") - end - end - - context 'on a fork' do - before do - expect(noteable).to receive(:for_fork?).and_return(true) - end - - it 'includes the project namespace' do - expect(summary_line).to include("<code>#{noteable.target_project_namespace}:feature</code>") - end - end - end - end + described_class.add_commits(noteable, project, author, new_commits, old_commits, oldrev) end end describe '.tag_commit' do - let(:noteable) do - project.commit - end - let(:tag_name) { 'v1.2.3' } - - subject { described_class.tag_commit(noteable, project, author, tag_name) } - - it_behaves_like 'a system note' do - let(:action) { 'tag' } - end + let(:tag_name) { double } - it 'sets the note text' do - link = "/#{project.full_path}/-/tags/#{tag_name}" + it 'calls CommitService' do + expect_next_instance_of(::SystemNotes::CommitService) do |service| + expect(service).to receive(:tag_commit).with(tag_name) + end - expect(subject.note).to eq "tagged commit #{noteable.sha} to [`#{tag_name}`](#{link})" + described_class.tag_commit(noteable, project, author, tag_name) end end @@ -804,15 +706,6 @@ describe SystemNoteService do end end - describe '.new_commit_summary' do - it 'escapes HTML titles' do - commit = double(title: '<pre>This is a test</pre>', short_id: '12345678') - escaped = '<pre>This is a test</pre>' - - expect(described_class.new_commit_summary([commit])).to all(match(/- #{escaped}/)) - end - end - describe 'Jira integration' do include JiraServiceHelper diff --git a/spec/services/system_notes/base_service_spec.rb b/spec/services/system_notes/base_service_spec.rb new file mode 100644 index 00000000000..96788b05829 --- /dev/null +++ b/spec/services/system_notes/base_service_spec.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe SystemNotes::BaseService do + let(:noteable) { double } + let(:project) { double } + let(:author) { double } + + let(:base_service) { described_class.new(noteable: noteable, project: project, author: author) } + + describe '#noteable' do + subject { base_service.noteable } + + it { is_expected.to eq(noteable) } + + it 'returns nil if no arguments are given' do + instance = described_class.new + expect(instance.noteable).to be_nil + end + end + + describe '#project' do + subject { base_service.project } + + it { is_expected.to eq(project) } + + it 'returns nil if no arguments are given' do + instance = described_class.new + expect(instance.project).to be_nil + end + end + + describe '#author' do + subject { base_service.author } + + it { is_expected.to eq(author) } + + it 'returns nil if no arguments are given' do + instance = described_class.new + expect(instance.author).to be_nil + end + end +end diff --git a/spec/services/system_notes/commit_service_spec.rb b/spec/services/system_notes/commit_service_spec.rb new file mode 100644 index 00000000000..4d4403be59a --- /dev/null +++ b/spec/services/system_notes/commit_service_spec.rb @@ -0,0 +1,117 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe SystemNotes::CommitService do + set(:group) { create(:group) } + set(:project) { create(:project, :repository, group: group) } + set(:author) { create(:user) } + + let(:commit_service) { described_class.new(noteable: noteable, project: project, author: author) } + + describe '#add_commits' do + subject { commit_service.add_commits(new_commits, old_commits, oldrev) } + + let(:noteable) { create(:merge_request, source_project: project, target_project: project) } + let(:new_commits) { noteable.commits } + let(:old_commits) { [] } + let(:oldrev) { nil } + + it_behaves_like 'a system note' do + let(:commit_count) { new_commits.size } + let(:action) { 'commit' } + end + + describe 'note body' do + let(:note_lines) { subject.note.split("\n").reject(&:blank?) } + + describe 'comparison diff link line' do + it 'adds the comparison text' do + expect(note_lines[2]).to match "[Compare with previous version]" + end + end + + context 'without existing commits' do + it 'adds a message header' do + expect(note_lines[0]).to eq "added #{new_commits.size} commits" + end + + it 'adds a message for each commit' do + decoded_note_content = HTMLEntities.new.decode(subject.note) + + new_commits.each do |commit| + expect(decoded_note_content).to include("<li>#{commit.short_id} - #{commit.title}</li>") + end + end + end + + describe 'summary line for existing commits' do + let(:summary_line) { note_lines[1] } + + context 'with one existing commit' do + let(:old_commits) { [noteable.commits.last] } + + it 'includes the existing commit' do + expect(summary_line).to start_with("<ul><li>#{old_commits.first.short_id} - 1 commit from branch <code>feature</code>") + end + end + + context 'with multiple existing commits' do + let(:old_commits) { noteable.commits[3..-1] } + + context 'with oldrev' do + let(:oldrev) { noteable.commits[2].id } + + it 'includes a commit range and count' do + expect(summary_line) + .to start_with("<ul><li>#{Commit.truncate_sha(oldrev)}...#{old_commits.last.short_id} - 26 commits from branch <code>feature</code>") + end + end + + context 'without oldrev' do + it 'includes a commit range and count' do + expect(summary_line) + .to start_with("<ul><li>#{old_commits[0].short_id}..#{old_commits[-1].short_id} - 26 commits from branch <code>feature</code>") + end + end + + context 'on a fork' do + before do + expect(noteable).to receive(:for_fork?).and_return(true) + end + + it 'includes the project namespace' do + expect(summary_line).to include("<code>#{noteable.target_project_namespace}:feature</code>") + end + end + end + end + end + end + + describe '#tag_commit' do + let(:noteable) { project.commit } + let(:tag_name) { 'v1.2.3' } + + subject { commit_service.tag_commit(tag_name) } + + it_behaves_like 'a system note' do + let(:action) { 'tag' } + end + + it 'sets the note text' do + link = "/#{project.full_path}/-/tags/#{tag_name}" + + expect(subject.note).to eq "tagged commit #{noteable.sha} to [`#{tag_name}`](#{link})" + end + end + + describe '#new_commit_summary' do + it 'escapes HTML titles' do + commit = double(title: '<pre>This is a test</pre>', short_id: '12345678') + escaped = '<pre>This is a test</pre>' + + expect(described_class.new.new_commit_summary([commit])).to all(match(/- #{escaped}/)) + end + end +end diff --git a/spec/support/shared_examples/common_system_notes_examples.rb b/spec/support/shared_examples/common_system_notes_examples.rb index 75f93a32d78..83d0e818eec 100644 --- a/spec/support/shared_examples/common_system_notes_examples.rb +++ b/spec/support/shared_examples/common_system_notes_examples.rb @@ -27,3 +27,28 @@ shared_examples 'WIP notes creation' do |wip_action| expect(Note.second.note).to match('changed title') end end + +shared_examples_for 'a note with overridable created_at' do + let(:noteable) { create(:issue, project: project, system_note_timestamp: Time.at(42)) } + + it 'the note has the correct time' do + expect(subject.created_at).to eq Time.at(42) + end +end + +shared_examples_for 'a system note' do + let(:expected_noteable) { noteable } + let(:commit_count) { nil } + + it 'has the correct attributes', :aggregate_failures do + expect(subject).to be_valid + expect(subject).to be_system + + expect(subject.noteable).to eq expected_noteable + expect(subject.project).to eq project + expect(subject.author).to eq author + + expect(subject.system_note_metadata.action).to eq(action) + expect(subject.system_note_metadata.commit_count).to eq(commit_count) + end +end diff --git a/spec/workers/post_receive_spec.rb b/spec/workers/post_receive_spec.rb index fb89eae9fa9..d034e962cee 100644 --- a/spec/workers/post_receive_spec.rb +++ b/spec/workers/post_receive_spec.rb @@ -61,6 +61,16 @@ describe PostReceive do end end + shared_examples 'not updating remote mirrors' do + it 'does not schedule an update' do + expect(project).not_to receive(:has_remote_mirror?) + expect(project).not_to receive(:mark_stuck_remote_mirrors_as_failed!) + expect(project).not_to receive(:update_remote_mirrors) + + perform + end + end + context 'empty changes' do it "does not call any PushService but runs after project hooks" do expect(Git::BranchPushService).not_to receive(:new) @@ -69,6 +79,8 @@ describe PostReceive do perform(changes: "") end + + it_behaves_like 'not updating remote mirrors' end context 'unidentified user' do @@ -88,6 +100,16 @@ describe PostReceive do allow(Gitlab::GlRepository).to receive(:parse).and_return([project, Gitlab::GlRepository::PROJECT]) end + shared_examples 'updating remote mirrors' do + it 'schedules an update if the project had mirrors' do + expect(project).to receive(:has_remote_mirror?).and_return(true) + expect(project).to receive(:mark_stuck_remote_mirrors_as_failed!) + expect(project).to receive(:update_remote_mirrors) + + perform + end + end + context "branches" do let(:changes) do <<~EOF @@ -126,6 +148,8 @@ describe PostReceive do perform end + it_behaves_like 'updating remote mirrors' + context 'with a default branch' do let(:changes) do <<~EOF @@ -191,6 +215,8 @@ describe PostReceive do perform end + + it_behaves_like 'updating remote mirrors' end context "merge-requests" do @@ -202,6 +228,8 @@ describe PostReceive do perform end + + it_behaves_like 'not updating remote mirrors' end context "gitlab-ci.yml" do |