diff options
46 files changed, 991 insertions, 42 deletions
@@ -448,9 +448,9 @@ end # Gitaly GRPC protocol definitions gem 'gitaly', '~> 1.65.0' -gem 'grpc', '~> 1.19.0' +gem 'grpc', '~> 1.24.0' -gem 'google-protobuf', '~> 3.7.1' +gem 'google-protobuf', '~> 3.8.0' gem 'toml-rb', '~> 1.0.0', require: false diff --git a/Gemfile.lock b/Gemfile.lock index 18160932c56..902a8281101 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -400,7 +400,7 @@ GEM mime-types (~> 3.0) representable (~> 3.0) retriable (>= 2.0, < 4.0) - google-protobuf (3.7.1) + google-protobuf (3.8.0) googleapis-common-protos-types (1.0.4) google-protobuf (~> 3.0) googleauth (0.6.6) @@ -440,9 +440,9 @@ GEM graphql (~> 1.6) html-pipeline (~> 2.8) sass (~> 3.4) - grpc (1.19.0) - google-protobuf (~> 3.1) - googleapis-common-protos-types (~> 1.0.0) + grpc (1.24.0) + google-protobuf (~> 3.8) + googleapis-common-protos-types (~> 1.0) gssapi (1.2.0) ffi (>= 1.0.1) haml (5.0.4) @@ -1181,7 +1181,7 @@ DEPENDENCIES gitlab_omniauth-ldap (~> 2.1.1) gon (~> 6.2) google-api-client (~> 0.23) - google-protobuf (~> 3.7.1) + google-protobuf (~> 3.8.0) gpgme (~> 2.0.18) grape (~> 1.1.0) grape-entity (~> 0.7.1) @@ -1190,7 +1190,7 @@ DEPENDENCIES graphiql-rails (~> 1.4.10) graphql (~> 1.9.11) graphql-docs (~> 1.6.0) - grpc (~> 1.19.0) + grpc (~> 1.24.0) gssapi haml_lint (~> 0.31.0) hamlit (~> 2.8.8) diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index ff199e05e99..e6032323c13 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -13,7 +13,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo skip_before_action :merge_request, only: [:index, :bulk_update] before_action :whitelist_query_limiting, only: [:assign_related_issues, :update] before_action :authorize_update_issuable!, only: [:close, :edit, :update, :remove_wip, :sort] - before_action :authorize_test_reports!, only: [:test_reports] + before_action :authorize_read_actual_head_pipeline!, only: [:test_reports, :exposed_artifacts] before_action :set_issuables_index, only: [:index] before_action :authenticate_user!, only: [:assign_related_issues] before_action :check_user_can_push_to_source_branch!, only: [:rebase] @@ -115,6 +115,14 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo reports_response(@merge_request.compare_test_reports) end + def exposed_artifacts + if @merge_request.has_exposed_artifacts? + reports_response(@merge_request.find_exposed_artifacts) + else + head :no_content + end + end + def edit define_edit_vars end @@ -357,8 +365,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo end end - def authorize_test_reports! - # MergeRequest#actual_head_pipeline is the pipeline accessed in MergeRequest#compare_reports. + def authorize_read_actual_head_pipeline! return render_404 unless can?(current_user, :read_build, merge_request.actual_head_pipeline) end end diff --git a/app/helpers/builds_helper.rb b/app/helpers/builds_helper.rb index a5fe6bb8f07..2def3488184 100644 --- a/app/helpers/builds_helper.rb +++ b/app/helpers/builds_helper.rb @@ -4,12 +4,12 @@ module BuildsHelper def build_summary(build, skip: false) if build.has_trace? if skip - link_to _("View job trace"), pipeline_job_url(build.pipeline, build) + link_to _("View job log"), pipeline_job_url(build.pipeline, build) else build.trace.html(last_lines: 10).html_safe end else - _("No job trace") + _("No job log") end end diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index c48ab28ce73..4089fcf7097 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -118,6 +118,11 @@ module Ci scope :eager_load_job_artifacts, -> { includes(:job_artifacts) } + scope :with_exposed_artifacts, -> do + joins(:metadata).merge(Ci::BuildMetadata.with_exposed_artifacts) + .includes(:metadata, :job_artifacts_metadata) + end + scope :with_artifacts_not_expired, ->() { with_artifacts_archive.where('artifacts_expire_at IS NULL OR artifacts_expire_at > ?', Time.now) } scope :with_expired_artifacts, ->() { with_artifacts_archive.where('artifacts_expire_at < ?', Time.now) } scope :last_month, ->() { where('created_at > ?', Date.today - 1.month) } @@ -595,6 +600,14 @@ module Ci update_column(:trace, nil) end + def artifacts_expose_as + options.dig(:artifacts, :expose_as) + end + + def artifacts_paths + options.dig(:artifacts, :paths) + end + def needs_touch? Time.now - updated_at > 15.minutes.to_i end diff --git a/app/models/ci/build_metadata.rb b/app/models/ci/build_metadata.rb index 3097e40dd3b..0df5ebfe843 100644 --- a/app/models/ci/build_metadata.rb +++ b/app/models/ci/build_metadata.rb @@ -27,6 +27,7 @@ module Ci scope :scoped_build, -> { where('ci_builds_metadata.build_id = ci_builds.id') } scope :with_interruptible, -> { where(interruptible: true) } + scope :with_exposed_artifacts, -> { where(has_exposed_artifacts: true) } enum timeout_source: { unknown_timeout_source: 1, diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 3bf19399cec..913253e4e92 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -783,6 +783,10 @@ module Ci end end + def has_exposed_artifacts? + complete? && builds.latest.with_exposed_artifacts.exists? + end + def branch_updated? strong_memoize(:branch_updated) do push_details.branch_updated? diff --git a/app/models/concerns/ci/metadatable.rb b/app/models/concerns/ci/metadatable.rb index a0ca8a34c6d..17d431bacf2 100644 --- a/app/models/concerns/ci/metadatable.rb +++ b/app/models/concerns/ci/metadatable.rb @@ -16,6 +16,7 @@ module Ci delegate :timeout, to: :metadata, prefix: true, allow_nil: true delegate :interruptible, to: :metadata, prefix: false, allow_nil: true + delegate :has_exposed_artifacts?, to: :metadata, prefix: false, allow_nil: true before_create :ensure_metadata end @@ -45,6 +46,9 @@ module Ci def options=(value) write_metadata_attribute(:options, :config_options, value) + + # Store presence of exposed artifacts in build metadata to make it easier to query + ensure_metadata.has_exposed_artifacts = value&.dig(:artifacts, :expose_as).present? end def yaml_variables=(value) diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 7cdaa3e3ca7..6ef84c5f59b 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -1255,6 +1255,27 @@ class MergeRequest < ApplicationRecord compare_reports(Ci::CompareTestReportsService) end + def has_exposed_artifacts? + return false unless Feature.enabled?(:ci_expose_arbitrary_artifacts_in_mr, default_enabled: true) + + actual_head_pipeline&.has_exposed_artifacts? + end + + # TODO: this method and compare_test_reports use the same + # result type, which is handled by the controller's #reports_response. + # we should minimize mistakes by isolating the common parts. + # issue: https://gitlab.com/gitlab-org/gitlab/issues/34224 + def find_exposed_artifacts + unless has_exposed_artifacts? + return { status: :error, status_reason: 'This merge request does not have exposed artifacts' } + end + + compare_reports(Ci::GenerateExposedArtifactsReportService) + end + + # TODO: consider renaming this as with exposed artifacts we generate reports, + # not always compare + # issue: https://gitlab.com/gitlab-org/gitlab/issues/34224 def compare_reports(service_class, current_user = nil) with_reactive_cache(service_class.name, current_user&.id) do |data| unless service_class.new(project, current_user) @@ -1269,6 +1290,8 @@ class MergeRequest < ApplicationRecord def calculate_reactive_cache(identifier, current_user_id = nil, *args) service_class = identifier.constantize + # TODO: the type check should change to something that includes exposed artifacts service + # issue: https://gitlab.com/gitlab-org/gitlab/issues/34224 raise NameError, service_class unless service_class < Ci::CompareReportsBaseService current_user = User.find_by(id: current_user_id) diff --git a/app/serializers/merge_request_poll_widget_entity.rb b/app/serializers/merge_request_poll_widget_entity.rb index 854349e8507..2a61187a856 100644 --- a/app/serializers/merge_request_poll_widget_entity.rb +++ b/app/serializers/merge_request_poll_widget_entity.rb @@ -65,6 +65,12 @@ class MergeRequestPollWidgetEntity < IssuableEntity end end + expose :exposed_artifacts_path do |merge_request| + if merge_request.has_exposed_artifacts? + exposed_artifacts_project_merge_request_path(merge_request.project, merge_request, format: :json) + end + end + expose :create_issue_to_resolve_discussions_path do |merge_request| presenter(merge_request).create_issue_to_resolve_discussions_path end diff --git a/app/services/ci/compare_reports_base_service.rb b/app/services/ci/compare_reports_base_service.rb index 5b76e1824e4..83ba70e8437 100644 --- a/app/services/ci/compare_reports_base_service.rb +++ b/app/services/ci/compare_reports_base_service.rb @@ -1,6 +1,11 @@ # frozen_string_literal: true module Ci + # TODO: when using this class with exposed artifacts we see that there are + # 2 responsibilities: + # 1. reactive caching interface (same in all cases) + # 2. data generator (report comparison in most of the case but not always) + # issue: https://gitlab.com/gitlab-org/gitlab/issues/34224 class CompareReportsBaseService < ::BaseService def execute(base_pipeline, head_pipeline) comparer = comparer_class.new(get_report(base_pipeline), get_report(head_pipeline)) diff --git a/app/services/ci/find_exposed_artifacts_service.rb b/app/services/ci/find_exposed_artifacts_service.rb new file mode 100644 index 00000000000..5c75af294bf --- /dev/null +++ b/app/services/ci/find_exposed_artifacts_service.rb @@ -0,0 +1,70 @@ +# frozen_string_literal: true + +module Ci + # This class loops through all builds with exposed artifacts and returns + # basic information about exposed artifacts for given jobs for the frontend + # to display them as custom links in the merge request. + # + # This service must be used with care. + # Looking for exposed artifacts is very slow and should be done asynchronously. + class FindExposedArtifactsService < ::BaseService + include Gitlab::Routing + + MAX_EXPOSED_ARTIFACTS = 10 + + def for_pipeline(pipeline, limit: MAX_EXPOSED_ARTIFACTS) + results = [] + + pipeline.builds.latest.with_exposed_artifacts.find_each do |job| + if job_exposed_artifacts = for_job(job) + results << job_exposed_artifacts + end + + break if results.size >= limit + end + + results + end + + def for_job(job) + return unless job.has_exposed_artifacts? + + metadata_entries = first_2_metadata_entries_for_artifacts_paths(job) + return if metadata_entries.empty? + + { + text: job.artifacts_expose_as, + url: path_for_entries(metadata_entries, job), + job_path: project_job_path(project, job), + job_name: job.name + } + end + + private + + # we don't need to fetch all artifacts entries for a job because + # it could contain many. We only need to know whether it has 1 or more + # artifacts, so fetching the first 2 would be sufficient. + def first_2_metadata_entries_for_artifacts_paths(job) + job.artifacts_paths + .lazy + .map { |path| job.artifacts_metadata_entry(path, recursive: true) } + .select { |entry| entry.exists? } + .first(2) + end + + def path_for_entries(entries, job) + return if entries.empty? + + if single_artifact?(entries) + file_project_job_artifacts_path(project, job, entries.first.path) + else + browse_project_job_artifacts_path(project, job) + end + end + + def single_artifact?(entries) + entries.size == 1 && entries.first.file? + end + end +end diff --git a/app/services/ci/generate_exposed_artifacts_report_service.rb b/app/services/ci/generate_exposed_artifacts_report_service.rb new file mode 100644 index 00000000000..b9bf580bcbc --- /dev/null +++ b/app/services/ci/generate_exposed_artifacts_report_service.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +module Ci + # TODO: a couple of points with this approach: + # + reuses existing architecture and reactive caching + # - it's not a report comparison and some comparing features must be turned off. + # see CompareReportsBaseService for more notes. + # issue: https://gitlab.com/gitlab-org/gitlab/issues/34224 + class GenerateExposedArtifactsReportService < CompareReportsBaseService + def execute(base_pipeline, head_pipeline) + data = FindExposedArtifactsService.new(project, current_user).for_pipeline(head_pipeline) + { + status: :parsed, + key: key(base_pipeline, head_pipeline), + data: data + } + rescue => e + Gitlab::Sentry.track_acceptable_exception(e, extra: { project_id: project.id }) + { + status: :error, + key: key(base_pipeline, head_pipeline), + status_reason: _('An error occurred while fetching exposed artifacts.') + } + end + + def latest?(base_pipeline, head_pipeline, data) + data&.fetch(:key, nil) == key(base_pipeline, head_pipeline) + end + end +end diff --git a/app/views/projects/_export.html.haml b/app/views/projects/_export.html.haml index e4129a91daf..2e00632892b 100644 --- a/app/views/projects/_export.html.haml +++ b/app/views/projects/_export.html.haml @@ -14,7 +14,7 @@ %li= desc %p= _('The following items will NOT be exported:') %ul - %li= _('Job traces and artifacts') + %li= _('Job logs and artifacts') %li= _('Container registry images') %li= _('CI variables') %li= _('Webhooks') diff --git a/app/views/projects/services/_form.html.haml b/app/views/projects/services/_form.html.haml index 959a2423e02..582f3d6fce4 100644 --- a/app/views/projects/services/_form.html.haml +++ b/app/views/projects/services/_form.html.haml @@ -6,7 +6,6 @@ - hide_class = 'd-none' if @service.activated? != value %span.js-service-active-status{ class: hide_class, data: { value: value.to_s } } = boolean_to_icon value - %p= #{@service.description}. - if @service.respond_to?(:detailed_description) %p= @service.detailed_description diff --git a/app/views/projects/settings/ci_cd/_form.html.haml b/app/views/projects/settings/ci_cd/_form.html.haml index 66ed1cadf6a..ea815be23c1 100644 --- a/app/views/projects/settings/ci_cd/_form.html.haml +++ b/app/views/projects/settings/ci_cd/_form.html.haml @@ -98,7 +98,7 @@ %span.input-group-append .input-group-text / %p.form-text.text-muted - = _("A regular expression that will be used to find the test coverage output in the job trace. Leave blank to disable") + = _("A regular expression that will be used to find the test coverage output in the job log. Leave blank to disable") = link_to icon('question-circle'), help_page_path('user/project/pipelines/settings', anchor: 'test-coverage-parsing'), target: '_blank' .bs-callout.bs-callout-info %p= _("Below are examples of regex for existing tools:") diff --git a/app/views/projects/show.html.haml b/app/views/projects/show.html.haml index b58af545439..c5653c3dd5a 100644 --- a/app/views/projects/show.html.haml +++ b/app/views/projects/show.html.haml @@ -6,7 +6,7 @@ = render partial: 'flash_messages', locals: { project: @project } -- if !@project.empty_repo? && can?(current_user, :download_code, @project) +- if !@project.empty_repo? && can?(current_user, :download_code, @project) && !vue_file_list_enabled? - signatures_path = project_signatures_path(@project, @project.default_branch) .js-signature-container{ data: { 'signatures-path': signatures_path } } diff --git a/app/views/projects/tree/show.html.haml b/app/views/projects/tree/show.html.haml index 39b29a20df6..65f5bc31d2e 100644 --- a/app/views/projects/tree/show.html.haml +++ b/app/views/projects/tree/show.html.haml @@ -6,7 +6,8 @@ = content_for :meta_tags do = auto_discovery_link_tag(:atom, project_commits_url(@project, @ref, rss_url_options), title: "#{@project.name}:#{@ref} commits") -.js-signature-container{ data: { 'signatures-path': signatures_path } } +- unless vue_file_list_enabled? + .js-signature-container{ data: { 'signatures-path': signatures_path } } = render 'projects/last_push' = render 'projects/files', commit: @last_commit, project: @project, ref: @ref, content_url: project_tree_path(@project, @id) diff --git a/changelogs/unreleased/29121-rename-trace.yml b/changelogs/unreleased/29121-rename-trace.yml new file mode 100644 index 00000000000..14c724e8356 --- /dev/null +++ b/changelogs/unreleased/29121-rename-trace.yml @@ -0,0 +1,5 @@ +--- +title: Replace wording trace with log +merge_request: +author: +type: changed diff --git a/changelogs/unreleased/expose-artifacts-to-merge-request-widget.yml b/changelogs/unreleased/expose-artifacts-to-merge-request-widget.yml new file mode 100644 index 00000000000..24113325feb --- /dev/null +++ b/changelogs/unreleased/expose-artifacts-to-merge-request-widget.yml @@ -0,0 +1,5 @@ +--- +title: Expose arbitrary job artifacts in Merge Request widget +merge_request: 18385 +author: +type: added diff --git a/changelogs/unreleased/sh-upgrade-grpc.yml b/changelogs/unreleased/sh-upgrade-grpc.yml new file mode 100644 index 00000000000..d0c3034eb93 --- /dev/null +++ b/changelogs/unreleased/sh-upgrade-grpc.yml @@ -0,0 +1,5 @@ +--- +title: Update gRPC to v1.24.0 +merge_request: 18837 +author: +type: other diff --git a/config/routes/project.rb b/config/routes/project.rb index 7d51cfd6dee..056289a72db 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -274,6 +274,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do get :discussions, format: :json post :rebase get :test_reports + get :exposed_artifacts scope constraints: { format: nil }, action: :show do get :commits, defaults: { tab: 'commits' } diff --git a/db/migrate/20191009110124_add_has_exposed_artifacts_to_ci_builds_metadata.rb b/db/migrate/20191009110124_add_has_exposed_artifacts_to_ci_builds_metadata.rb new file mode 100644 index 00000000000..86c3c540e5e --- /dev/null +++ b/db/migrate/20191009110124_add_has_exposed_artifacts_to_ci_builds_metadata.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class AddHasExposedArtifactsToCiBuildsMetadata < ActiveRecord::Migration[5.2] + DOWNTIME = false + + def up + add_column :ci_builds_metadata, :has_exposed_artifacts, :boolean + end + + def down + remove_column :ci_builds_metadata, :has_exposed_artifacts + end +end diff --git a/db/migrate/20191009110757_add_index_to_ci_builds_metadata_has_exposed_artifacts.rb b/db/migrate/20191009110757_add_index_to_ci_builds_metadata_has_exposed_artifacts.rb new file mode 100644 index 00000000000..6b8c452a62a --- /dev/null +++ b/db/migrate/20191009110757_add_index_to_ci_builds_metadata_has_exposed_artifacts.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddIndexToCiBuildsMetadataHasExposedArtifacts < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_index :ci_builds_metadata, [:build_id], where: "has_exposed_artifacts IS TRUE", name: 'index_ci_builds_metadata_on_build_id_and_has_exposed_artifacts' + end + + def down + remove_concurrent_index_by_name :ci_builds_metadata, 'index_ci_builds_metadata_on_build_id_and_has_exposed_artifacts' + end +end diff --git a/db/schema.rb b/db/schema.rb index f3a2b4608f5..37e540a9b3a 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -691,7 +691,9 @@ ActiveRecord::Schema.define(version: 2019_10_16_220135) do t.boolean "interruptible" t.jsonb "config_options" t.jsonb "config_variables" + t.boolean "has_exposed_artifacts" t.index ["build_id"], name: "index_ci_builds_metadata_on_build_id", unique: true + t.index ["build_id"], name: "index_ci_builds_metadata_on_build_id_and_has_exposed_artifacts", where: "(has_exposed_artifacts IS TRUE)" t.index ["build_id"], name: "index_ci_builds_metadata_on_build_id_and_interruptible", where: "(interruptible = true)" t.index ["project_id"], name: "index_ci_builds_metadata_on_project_id" end diff --git a/lib/gitlab/ci/config/entry/artifacts.rb b/lib/gitlab/ci/config/entry/artifacts.rb index 41613369ca2..9d8d7675234 100644 --- a/lib/gitlab/ci/config/entry/artifacts.rb +++ b/lib/gitlab/ci/config/entry/artifacts.rb @@ -12,7 +12,9 @@ module Gitlab include ::Gitlab::Config::Entry::Validatable include ::Gitlab::Config::Entry::Attributable - ALLOWED_KEYS = %i[name untracked paths reports when expire_in].freeze + ALLOWED_KEYS = %i[name untracked paths reports when expire_in expose_as].freeze + EXPOSE_AS_REGEX = /\A\w[-\w ]*\z/.freeze + EXPOSE_AS_ERROR_MESSAGE = "can contain only letters, digits, '-', '_' and spaces" attributes ALLOWED_KEYS @@ -21,11 +23,18 @@ module Gitlab validations do validates :config, type: Hash validates :config, allowed_keys: ALLOWED_KEYS + validates :paths, presence: true, if: :expose_as_present? with_options allow_nil: true do validates :name, type: String validates :untracked, boolean: true validates :paths, array_of_strings: true + validates :paths, array_of_strings: { + with: /\A[^*]*\z/, + message: "can't contain '*' when used with 'expose_as'" + }, if: :expose_as_present? + validates :expose_as, type: String, length: { maximum: 100 }, if: :expose_as_present? + validates :expose_as, format: { with: EXPOSE_AS_REGEX, message: EXPOSE_AS_ERROR_MESSAGE }, if: :expose_as_present? validates :reports, type: Hash validates :when, inclusion: { in: %w[on_success on_failure always], @@ -41,6 +50,12 @@ module Gitlab @config[:reports] = reports_value if @config.key?(:reports) @config end + + def expose_as_present? + return false unless Feature.enabled?(:ci_expose_arbitrary_artifacts_in_mr, default_enabled: true) + + !@config[:expose_as].nil? + end end end end diff --git a/lib/gitlab/config/entry/validators.rb b/lib/gitlab/config/entry/validators.rb index 374f929878e..8a04cca60d7 100644 --- a/lib/gitlab/config/entry/validators.rb +++ b/lib/gitlab/config/entry/validators.rb @@ -61,8 +61,15 @@ module Gitlab include LegacyValidationHelpers def validate_each(record, attribute, value) - unless validate_array_of_strings(value) - record.errors.add(attribute, 'should be an array of strings') + valid = validate_array_of_strings(value) + + record.errors.add(attribute, 'should be an array of strings') unless valid + + if valid && options[:with] + unless value.all? { |v| v =~ options[:with] } + message = options[:message] || 'contains elements that do not match the format' + record.errors.add(attribute, message) + end end end end diff --git a/lib/tasks/gitlab/shell.rake b/lib/tasks/gitlab/shell.rake index abd47f018f1..a592015963d 100644 --- a/lib/tasks/gitlab/shell.rake +++ b/lib/tasks/gitlab/shell.rake @@ -43,7 +43,7 @@ namespace :gitlab do [ %w(bin/install) + repository_storage_paths_args, - %w(bin/compile) + %w(make build) ].each do |cmd| unless Kernel.system(*cmd) raise "command failed: #{cmd.join(' ')}" diff --git a/locale/gitlab.pot b/locale/gitlab.pot index e3860825b73..766b8c71dc5 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -686,7 +686,7 @@ msgstr "" msgid "A ready-to-go template for use with iOS Swift apps." msgstr "" -msgid "A regular expression that will be used to find the test coverage output in the job trace. Leave blank to disable" +msgid "A regular expression that will be used to find the test coverage output in the job log. Leave blank to disable" msgstr "" msgid "A secure token that identifies an external storage request." @@ -1515,6 +1515,9 @@ msgstr "" msgid "An error occurred while fetching environments." msgstr "" +msgid "An error occurred while fetching exposed artifacts." +msgstr "" + msgid "An error occurred while fetching folder content." msgstr "" @@ -9339,7 +9342,7 @@ msgstr "" msgid "Job is stuck. Check runners." msgstr "" -msgid "Job traces and artifacts" +msgid "Job logs and artifacts" msgstr "" msgid "Job was retried" @@ -10992,7 +10995,7 @@ msgstr "" msgid "No issues for the selected time period." msgstr "" -msgid "No job trace" +msgid "No job log" msgstr "" msgid "No jobs to show" @@ -18390,7 +18393,7 @@ msgstr "" msgid "View job" msgstr "" -msgid "View job trace" +msgid "View job log" msgstr "" msgid "View jobs" diff --git a/qa/qa/page/component/select2.rb b/qa/qa/page/component/select2.rb index d05c44d22b2..8fe6a4a75b3 100644 --- a/qa/qa/page/component/select2.rb +++ b/qa/qa/page/component/select2.rb @@ -20,12 +20,20 @@ module QA def search_and_select(item_text) find('.select2-input').set(item_text) + + wait_for_search_to_complete + select_item(item_text) end def expand_select_list find('span.select2-arrow').click end + + def wait_for_search_to_complete + has_css?('.select2-active') + has_no_css?('.select2-active', wait: 30) + end end end end diff --git a/qa/qa/page/file/shared/commit_button.rb b/qa/qa/page/file/shared/commit_button.rb index d8e751dd7b6..559b4c6ceea 100644 --- a/qa/qa/page/file/shared/commit_button.rb +++ b/qa/qa/page/file/shared/commit_button.rb @@ -13,6 +13,10 @@ module QA def commit_changes click_element(:commit_button) + + wait(reload: false, max: 60) do + finished_loading? + end end end end diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index ea702792557..827b34b8850 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -4,6 +4,7 @@ require 'spec_helper' describe Projects::MergeRequestsController do include ProjectForksHelper + include Gitlab::Routing let(:project) { create(:project, :repository) } let(:user) { project.owner } @@ -206,7 +207,7 @@ describe Projects::MergeRequestsController do it 'redirects to last_page if page number is larger than number of pages' do get_merge_requests(last_page + 1) - expect(response).to redirect_to(namespace_project_merge_requests_path(page: last_page, state: controller.params[:state], scope: controller.params[:scope])) + expect(response).to redirect_to(project_merge_requests_path(project, page: last_page, state: controller.params[:state], scope: controller.params[:scope])) end it 'redirects to specified page' do @@ -227,7 +228,7 @@ describe Projects::MergeRequestsController do host: external_host } - expect(response).to redirect_to(namespace_project_merge_requests_path(page: last_page, state: controller.params[:state], scope: controller.params[:scope])) + expect(response).to redirect_to(project_merge_requests_path(project, page: last_page, state: controller.params[:state], scope: controller.params[:scope])) end end @@ -770,6 +771,189 @@ describe Projects::MergeRequestsController do end end + describe 'GET exposed_artifacts' do + let(:merge_request) do + create(:merge_request, + :with_merge_request_pipeline, + target_project: project, + source_project: project) + end + + let(:pipeline) do + create(:ci_pipeline, + :success, + project: merge_request.source_project, + ref: merge_request.source_branch, + sha: merge_request.diff_head_sha) + end + + let!(:job) { create(:ci_build, pipeline: pipeline, options: job_options) } + let!(:job_metadata) { create(:ci_job_artifact, :metadata, job: job) } + + before do + allow_any_instance_of(MergeRequest) + .to receive(:find_exposed_artifacts) + .and_return(report) + + allow_any_instance_of(MergeRequest) + .to receive(:actual_head_pipeline) + .and_return(pipeline) + end + + subject do + get :exposed_artifacts, params: { + namespace_id: project.namespace.to_param, + project_id: project, + id: merge_request.iid + }, + format: :json + end + + describe 'permissions on a public project with private CI/CD' do + let(:project) { create :project, :repository, :public, :builds_private } + let(:report) { { status: :parsed, data: [] } } + let(:job_options) { {} } + + context 'while signed out' do + before do + sign_out(user) + end + + it 'responds with a 404' do + subject + + expect(response).to have_gitlab_http_status(404) + expect(response.body).to be_blank + end + end + + context 'while signed in as an unrelated user' do + before do + sign_in(create(:user)) + end + + it 'responds with a 404' do + subject + + expect(response).to have_gitlab_http_status(404) + expect(response.body).to be_blank + end + end + end + + context 'when pipeline has jobs with exposed artifacts' do + let(:job_options) do + { + artifacts: { + paths: ['ci_artifacts.txt'], + expose_as: 'Exposed artifact' + } + } + end + + context 'when fetching exposed artifacts is in progress' do + let(:report) { { status: :parsing } } + + it 'sends polling interval' do + expect(Gitlab::PollingInterval).to receive(:set_header) + + subject + end + + it 'returns 204 HTTP status' do + subject + + expect(response).to have_gitlab_http_status(:no_content) + end + end + + context 'when fetching exposed artifacts is completed' do + let(:data) do + Ci::GenerateExposedArtifactsReportService.new(project, user) + .execute(nil, pipeline) + end + + let(:report) { { status: :parsed, data: data } } + + it 'returns exposed artifacts' do + subject + + expect(response).to have_gitlab_http_status(200) + expect(json_response['status']).to eq('parsed') + expect(json_response['data']).to eq([{ + 'job_name' => 'test', + 'job_path' => project_job_path(project, job), + 'url' => file_project_job_artifacts_path(project, job, 'ci_artifacts.txt'), + 'text' => 'Exposed artifact' + }]) + end + end + + context 'when something went wrong on our system' do + let(:report) { {} } + + it 'does not send polling interval' do + expect(Gitlab::PollingInterval).not_to receive(:set_header) + + subject + end + + it 'returns 500 HTTP status' do + subject + + expect(response).to have_gitlab_http_status(:internal_server_error) + expect(json_response).to eq({ 'status_reason' => 'Unknown error' }) + end + end + + context 'when feature flag :ci_expose_arbitrary_artifacts_in_mr is disabled' do + let(:job_options) do + { + artifacts: { + paths: ['ci_artifacts.txt'], + expose_as: 'Exposed artifact' + } + } + end + let(:report) { double } + + before do + stub_feature_flags(ci_expose_arbitrary_artifacts_in_mr: false) + end + + it 'does not send polling interval' do + expect(Gitlab::PollingInterval).not_to receive(:set_header) + + subject + end + + it 'returns 204 HTTP status' do + subject + + expect(response).to have_gitlab_http_status(:no_content) + end + end + end + + context 'when pipeline does not have jobs with exposed artifacts' do + let(:report) { double } + let(:job_options) do + { + artifacts: { + paths: ['ci_artifacts.txt'] + } + } + end + + it 'returns no content' do + subject + + expect(response).to have_gitlab_http_status(204) + expect(response.body).to be_empty + end + end + end + describe 'GET test_reports' do let(:merge_request) do create(:merge_request, diff --git a/spec/factories/ci/pipelines.rb b/spec/factories/ci/pipelines.rb index fefd89728e6..39ab574cc76 100644 --- a/spec/factories/ci/pipelines.rb +++ b/spec/factories/ci/pipelines.rb @@ -95,6 +95,17 @@ FactoryBot.define do end end + trait :with_exposed_artifacts do + status { :success } + + after(:build) do |pipeline, evaluator| + pipeline.builds << build(:ci_build, :artifacts, + pipeline: pipeline, + project: pipeline.project, + options: { artifacts: { expose_as: 'the artifact', paths: ['ci_artifacts.txt'] } }) + end + end + trait :with_job do after(:build) do |pipeline, evaluator| pipeline.builds << build(:ci_build, pipeline: pipeline, project: pipeline.project) diff --git a/spec/factories/merge_requests.rb b/spec/factories/merge_requests.rb index d16e0c10671..13612214e72 100644 --- a/spec/factories/merge_requests.rb +++ b/spec/factories/merge_requests.rb @@ -120,6 +120,18 @@ FactoryBot.define do end end + trait :with_exposed_artifacts do + after(:build) do |merge_request| + merge_request.head_pipeline = build( + :ci_pipeline, + :success, + :with_exposed_artifacts, + project: merge_request.source_project, + ref: merge_request.source_branch, + sha: merge_request.diff_head_sha) + end + end + trait :with_legacy_detached_merge_request_pipeline do after(:create) do |merge_request| merge_request.pipelines_for_merge_request << create(:ci_pipeline, diff --git a/spec/features/merge_request/user_sees_merge_widget_spec.rb b/spec/features/merge_request/user_sees_merge_widget_spec.rb index 6b6226ad1c5..9fadd46ed44 100644 --- a/spec/features/merge_request/user_sees_merge_widget_spec.rb +++ b/spec/features/merge_request/user_sees_merge_widget_spec.rb @@ -5,6 +5,7 @@ require 'spec_helper' describe 'Merge request > User sees merge widget', :js do include ProjectForksHelper include TestReportsHelper + include ReactiveCachingHelpers let(:project) { create(:project, :repository) } let(:project_only_mwps) { create(:project, :repository, only_allow_merge_if_pipeline_succeeds: true) } @@ -435,6 +436,54 @@ describe 'Merge request > User sees merge widget', :js do end end + context 'exposed artifacts' do + subject { visit project_merge_request_path(project, merge_request) } + + context 'when merge request has exposed artifacts' do + let(:merge_request) { create(:merge_request, :with_exposed_artifacts, source_project: project) } + let(:job) { merge_request.head_pipeline.builds.last } + let!(:artifacts_metadata) { create(:ci_job_artifact, :metadata, job: job) } + + context 'when result has not been parsed yet' do + it 'shows parsing status' do + subject + + expect(page).to have_content('Loading artifacts') + end + end + + context 'when result has been parsed' do + before do + allow_any_instance_of(MergeRequest).to receive(:find_exposed_artifacts).and_return( + status: :parsed, data: [ + { + text: "the artifact", + url: "/namespace1/project1/-/jobs/1/artifacts/file/ci_artifacts.txt", + job_path: "/namespace1/project1/-/jobs/1", + job_name: "test" + } + ]) + end + + it 'shows the parsed results' do + subject + + expect(page).to have_content('View exposed artifact') + end + end + end + + context 'when merge request does not have exposed artifacts' do + let(:merge_request) { create(:merge_request, source_project: project) } + + it 'does not show parsing status' do + subject + + expect(page).not_to have_content('Loading artifacts') + end + end + end + context 'when merge request has test reports' do let!(:head_pipeline) do create(:ci_pipeline, diff --git a/spec/features/projects/pipelines/pipeline_spec.rb b/spec/features/projects/pipelines/pipeline_spec.rb index 04adb1ec6af..66807eb1c17 100644 --- a/spec/features/projects/pipelines/pipeline_spec.rb +++ b/spec/features/projects/pipelines/pipeline_spec.rb @@ -778,10 +778,10 @@ describe 'Pipeline', :js do expect(page).to have_content(failed_build.stage) end - it 'does not show trace' do + it 'does not show log' do subject - expect(page).to have_content('No job trace') + expect(page).to have_content('No job log') end end diff --git a/spec/features/signed_commits_spec.rb b/spec/features/signed_commits_spec.rb index 70e6978a7b6..2615e8400a4 100644 --- a/spec/features/signed_commits_spec.rb +++ b/spec/features/signed_commits_spec.rb @@ -152,4 +152,34 @@ describe 'GPG signed commits' do end end end + + context 'view signed commit on the tree view', :js do + shared_examples 'a commit with a signature' do + before do + visit project_tree_path(project, 'signed-commits') + end + + it 'displays commit signature' do + expect(page).to have_button 'Unverified' + + click_on 'Unverified' + + within '.popover' do + expect(page).to have_content 'This commit was signed with an unverified signature' + end + end + end + + context 'with vue tree view enabled' do + it_behaves_like 'a commit with a signature' + end + + context 'with vue tree view disabled' do + before do + stub_feature_flags(vue_file_list: false) + end + + it_behaves_like 'a commit with a signature' + end + end end diff --git a/spec/lib/gitlab/ci/config/entry/artifacts_spec.rb b/spec/lib/gitlab/ci/config/entry/artifacts_spec.rb index a7f457e0f5e..513a9b8f2b4 100644 --- a/spec/lib/gitlab/ci/config/entry/artifacts_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/artifacts_spec.rb @@ -28,6 +28,14 @@ describe Gitlab::Ci::Config::Entry::Artifacts do expect(entry.value).to eq config end end + + context "when value includes 'expose_as' keyword" do + let(:config) { { paths: %w[results.txt], expose_as: "Test results" } } + + it 'returns general artifact and report-type artifacts configuration' do + expect(entry.value).to eq config + end + end end context 'when entry value is not correct' do @@ -58,6 +66,84 @@ describe Gitlab::Ci::Config::Entry::Artifacts do .to include 'artifacts reports should be a hash' end end + + context "when 'expose_as' is not a string" do + let(:config) { { paths: %w[results.txt], expose_as: 1 } } + + it 'reports error' do + expect(entry.errors) + .to include 'artifacts expose as should be a string' + end + end + + context "when 'expose_as' is too long" do + let(:config) { { paths: %w[results.txt], expose_as: 'A' * 101 } } + + it 'reports error' do + expect(entry.errors) + .to include 'artifacts expose as is too long (maximum is 100 characters)' + end + end + + context "when 'expose_as' is an empty string" do + let(:config) { { paths: %w[results.txt], expose_as: '' } } + + it 'reports error' do + expect(entry.errors) + .to include 'artifacts expose as ' + Gitlab::Ci::Config::Entry::Artifacts::EXPOSE_AS_ERROR_MESSAGE + end + end + + context "when 'expose_as' contains invalid characters" do + let(:config) do + { paths: %w[results.txt], expose_as: '<script>alert("xss");</script>' } + end + + it 'reports error' do + expect(entry.errors) + .to include 'artifacts expose as ' + Gitlab::Ci::Config::Entry::Artifacts::EXPOSE_AS_ERROR_MESSAGE + end + end + + context "when 'expose_as' is used without 'paths'" do + let(:config) { { expose_as: 'Test results' } } + + it 'reports error' do + expect(entry.errors) + .to include "artifacts paths can't be blank" + end + end + + context "when 'paths' includes '*' and 'expose_as' is defined" do + let(:config) { { expose_as: 'Test results', paths: ['test.txt', 'test*.txt'] } } + + it 'reports error' do + expect(entry.errors) + .to include "artifacts paths can't contain '*' when used with 'expose_as'" + end + end + end + + context 'when feature flag :ci_expose_arbitrary_artifacts_in_mr is disabled' do + before do + stub_feature_flags(ci_expose_arbitrary_artifacts_in_mr: false) + end + + context 'when syntax is correct' do + let(:config) { { expose_as: 'Test results', paths: ['test.txt'] } } + + it 'is valid' do + expect(entry.errors).to be_empty + end + end + + context 'when syntax for :expose_as is incorrect' do + let(:config) { { paths: %w[results.txt], expose_as: '' } } + + it 'is valid' do + expect(entry.errors).to be_empty + end + end end end end diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb index cb5ebde16d7..c7a90d2a254 100644 --- a/spec/lib/gitlab/ci/yaml_processor_spec.rb +++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb @@ -970,6 +970,7 @@ module Gitlab rspec: { artifacts: { paths: ["logs/", "binaries/"], + expose_as: "Exposed artifacts", untracked: true, name: "custom_name", expire_in: "7d" @@ -993,6 +994,7 @@ module Gitlab artifacts: { name: "custom_name", paths: ["logs/", "binaries/"], + expose_as: "Exposed artifacts", untracked: true, expire_in: "7d" } diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 058305bc04e..50b104c4095 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -206,6 +206,35 @@ describe Ci::Build do end end + describe '.with_exposed_artifacts' do + subject { described_class.with_exposed_artifacts } + + let!(:job1) { create(:ci_build) } + let!(:job2) { create(:ci_build, options: options) } + let!(:job3) { create(:ci_build) } + + context 'when some jobs have exposed artifacs and some not' do + let(:options) { { artifacts: { expose_as: 'test', paths: ['test'] } } } + + before do + job1.ensure_metadata.update!(has_exposed_artifacts: nil) + job3.ensure_metadata.update!(has_exposed_artifacts: false) + end + + it 'selects only the jobs with exposed artifacts' do + is_expected.to eq([job2]) + end + end + + context 'when job does not expose artifacts' do + let(:options) { nil } + + it 'returns an empty array' do + is_expected.to be_empty + end + end + end + describe '.with_reports' do subject { described_class.with_reports(Ci::JobArtifact.test_reports) } @@ -1844,6 +1873,14 @@ describe Ci::Build do expect(build.metadata.read_attribute(:config_options)).to be_nil end end + + context 'when options include artifacts:expose_as' do + let(:build) { create(:ci_build, options: { artifacts: { expose_as: 'test' } }) } + + it 'saves the presence of expose_as into build metadata' do + expect(build.metadata).to have_exposed_artifacts + end + end end describe '#other_manual_actions' do diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index ad79bee8801..b8d09f03fe6 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -1674,6 +1674,63 @@ describe MergeRequest do end end + describe '#find_exposed_artifacts' do + let(:project) { create(:project, :repository) } + let(:merge_request) { create(:merge_request, :with_test_reports, source_project: project) } + let(:pipeline) { merge_request.head_pipeline } + + subject { merge_request.find_exposed_artifacts } + + context 'when head pipeline has exposed artifacts' do + let!(:job) do + create(:ci_build, options: { artifacts: { expose_as: 'artifact', paths: ['ci_artifacts.txt'] } }, pipeline: pipeline) + end + + let!(:artifacts_metadata) { create(:ci_job_artifact, :metadata, job: job) } + + context 'when reactive cache worker is parsing results asynchronously' do + it 'returns status' do + expect(subject[:status]).to eq(:parsing) + end + end + + context 'when reactive cache worker is inline' do + before do + synchronous_reactive_cache(merge_request) + end + + it 'returns status and data' do + expect(subject[:status]).to eq(:parsed) + end + + context 'when an error occurrs' do + before do + expect_next_instance_of(Ci::FindExposedArtifactsService) do |service| + expect(service).to receive(:for_pipeline) + .and_raise(StandardError.new) + end + end + + it 'returns an error message' do + expect(subject[:status]).to eq(:error) + end + end + + context 'when cached results is not latest' do + before do + allow_next_instance_of(Ci::GenerateExposedArtifactsReportService) do |service| + allow(service).to receive(:latest?).and_return(false) + end + end + + it 'raises and InvalidateReactiveCache error' do + expect { subject }.to raise_error(ReactiveCaching::InvalidateReactiveCache) + end + end + end + end + end + describe '#compare_test_reports' do subject { merge_request.compare_test_reports } diff --git a/spec/serializers/merge_request_widget_entity_spec.rb b/spec/serializers/merge_request_widget_entity_spec.rb index 4872b23d26b..35940ac062e 100644 --- a/spec/serializers/merge_request_widget_entity_spec.rb +++ b/spec/serializers/merge_request_widget_entity_spec.rb @@ -358,4 +358,26 @@ describe MergeRequestWidgetEntity do end end end + + describe 'exposed_artifacts_path' do + context 'when merge request has exposed artifacts' do + before do + expect(resource).to receive(:has_exposed_artifacts?).and_return(true) + end + + it 'set the path to poll data' do + expect(subject[:exposed_artifacts_path]).to be_present + end + end + + context 'when merge request has no exposed artifacts' do + before do + expect(resource).to receive(:has_exposed_artifacts?).and_return(false) + end + + it 'set the path to poll data' do + expect(subject[:exposed_artifacts_path]).to be_nil + end + end + end end diff --git a/spec/services/ci/find_exposed_artifacts_service_spec.rb b/spec/services/ci/find_exposed_artifacts_service_spec.rb new file mode 100644 index 00000000000..f6309822fe0 --- /dev/null +++ b/spec/services/ci/find_exposed_artifacts_service_spec.rb @@ -0,0 +1,147 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Ci::FindExposedArtifactsService do + include Gitlab::Routing + + let(:metadata) do + Gitlab::Ci::Build::Artifacts::Metadata + .new(metadata_file_stream, path, { recursive: true }) + end + + let(:metadata_file_stream) do + File.open(Rails.root + 'spec/fixtures/ci_build_artifacts_metadata.gz') + end + + let_it_be(:project) { create(:project) } + let(:user) { nil } + + after do + metadata_file_stream&.close + end + + def create_job_with_artifacts(options) + create(:ci_build, pipeline: pipeline, options: options).tap do |job| + create(:ci_job_artifact, :metadata, job: job) + end + end + + describe '#for_pipeline' do + shared_examples 'finds a single match' do + it 'returns the artifact with exact location' do + expect(subject).to eq([{ + text: 'Exposed artifact', + url: file_project_job_artifacts_path(project, job, 'other_artifacts_0.1.2/doc_sample.txt'), + job_name: job.name, + job_path: project_job_path(project, job) + }]) + end + end + + shared_examples 'finds multiple matches' do + it 'returns the path to the artifacts browser' do + expect(subject).to eq([{ + text: 'Exposed artifact', + url: browse_project_job_artifacts_path(project, job), + job_name: job.name, + job_path: project_job_path(project, job) + }]) + end + end + + let_it_be(:pipeline) { create(:ci_pipeline, project: project) } + + subject { described_class.new(project, user).for_pipeline(pipeline) } + + context 'with jobs having at most 1 matching exposed artifact' do + let!(:job) do + create_job_with_artifacts(artifacts: { + expose_as: 'Exposed artifact', + paths: ['other_artifacts_0.1.2/doc_sample.txt', 'something-else.html'] + }) + end + + it_behaves_like 'finds a single match' + end + + context 'with jobs having more than 1 matching exposed artifacts' do + let!(:job) do + create_job_with_artifacts(artifacts: { + expose_as: 'Exposed artifact', + paths: [ + 'ci_artifacts.txt', + 'other_artifacts_0.1.2/doc_sample.txt', + 'something-else.html' + ] + }) + end + + it_behaves_like 'finds multiple matches' + end + + context 'with jobs having more than 1 matching exposed artifacts inside a directory' do + let!(:job) do + create_job_with_artifacts(artifacts: { + expose_as: 'Exposed artifact', + paths: ['tests_encoding/'] + }) + end + + it_behaves_like 'finds multiple matches' + end + + context 'with jobs having paths with glob expression' do + let!(:job) do + create_job_with_artifacts(artifacts: { + expose_as: 'Exposed artifact', + paths: ['other_artifacts_0.1.2/doc_sample.txt', 'tests_encoding/*.*'] + }) + end + + it_behaves_like 'finds a single match' # because those with * are ignored + end + + context 'limiting results' do + let!(:job1) do + create_job_with_artifacts(artifacts: { + expose_as: 'artifact 1', + paths: ['ci_artifacts.txt'] + }) + end + + let!(:job2) do + create_job_with_artifacts(artifacts: { + expose_as: 'artifact 2', + paths: ['tests_encoding/'] + }) + end + + let!(:job3) do + create_job_with_artifacts(artifacts: { + expose_as: 'should not be exposed', + paths: ['other_artifacts_0.1.2/doc_sample.txt'] + }) + end + + subject { described_class.new(project, user).for_pipeline(pipeline, limit: 2) } + + it 'returns first 2 results' do + expect(subject).to eq([ + { + text: 'artifact 1', + url: file_project_job_artifacts_path(project, job1, 'ci_artifacts.txt'), + job_name: job1.name, + job_path: project_job_path(project, job1) + }, + { + text: 'artifact 2', + url: browse_project_job_artifacts_path(project, job2), + job_name: job2.name, + job_path: project_job_path(project, job2) + } + ]) + end + end + end +end diff --git a/spec/tasks/gitlab/shell_rake_spec.rb b/spec/tasks/gitlab/shell_rake_spec.rb index abad16be580..08b3fea0c80 100644 --- a/spec/tasks/gitlab/shell_rake_spec.rb +++ b/spec/tasks/gitlab/shell_rake_spec.rb @@ -17,7 +17,7 @@ describe 'gitlab:shell rake tasks' do expect_any_instance_of(Gitlab::TaskHelpers).to receive(:checkout_or_clone_version) allow(Kernel).to receive(:system).with('bin/install', *storages).and_return(true) - allow(Kernel).to receive(:system).with('bin/compile').and_return(true) + allow(Kernel).to receive(:system).with('make', 'build').and_return(true) run_rake_task('gitlab:shell:install') end diff --git a/spec/views/projects/show.html.haml_spec.rb b/spec/views/projects/show.html.haml_spec.rb new file mode 100644 index 00000000000..820772b592f --- /dev/null +++ b/spec/views/projects/show.html.haml_spec.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'projects/show' do + include Devise::Test::ControllerHelpers + + let(:user) { create(:admin) } + let(:project) { create(:project, :repository) } + + before do + presented_project = project.present(current_user: user) + + allow(presented_project).to receive(:default_view).and_return('customize_workflow') + allow(controller).to receive(:current_user).and_return(user) + + assign(:project, presented_project) + end + + context 'commit signatures' do + context 'with vue tree view disabled' do + before do + stub_feature_flags(vue_file_list: false) + end + + it 'rendered via js-signature-container' do + render + + expect(rendered).to have_css('.js-signature-container') + end + end + + context 'with vue tree view enabled' do + it 'are not rendered via js-signature-container' do + render + + expect(rendered).not_to have_css('.js-signature-container') + end + end + end +end diff --git a/spec/views/projects/tree/show.html.haml_spec.rb b/spec/views/projects/tree/show.html.haml_spec.rb index 960cf42a793..4307d1b49c9 100644 --- a/spec/views/projects/tree/show.html.haml_spec.rb +++ b/spec/views/projects/tree/show.html.haml_spec.rb @@ -7,6 +7,10 @@ describe 'projects/tree/show' do let(:project) { create(:project, :repository) } let(:repository) { project.repository } + let(:ref) { 'master' } + let(:commit) { repository.commit(ref) } + let(:path) { '' } + let(:tree) { repository.tree(commit.id, path) } before do stub_feature_flags(vue_file_list: false) @@ -19,26 +23,45 @@ describe 'projects/tree/show' do allow(view).to receive(:can_collaborate_with_project?).and_return(true) allow(view).to receive_message_chain('user_access.can_push_to_branch?').and_return(true) allow(view).to receive(:current_application_settings).and_return(Gitlab::CurrentSettings.current_application_settings) + allow(view).to receive(:current_user).and_return(project.creator) + + assign(:id, File.join(ref, path)) + assign(:ref, ref) + assign(:path, path) + assign(:last_commit, commit) + assign(:tree, tree) end context 'for branch names ending on .json' do let(:ref) { 'ends-with.json' } - let(:commit) { repository.commit(ref) } - let(:path) { '' } - let(:tree) { repository.tree(commit.id, path) } - - before do - assign(:id, File.join(ref, path)) - assign(:ref, ref) - assign(:path, path) - assign(:last_commit, commit) - assign(:tree, tree) - end it 'displays correctly' do render + expect(rendered).to have_css('.js-project-refs-dropdown .dropdown-toggle-text', text: ref) expect(rendered).to have_css('.readme-holder') end end + + context 'commit signatures' do + context 'with vue tree view disabled' do + it 'rendered via js-signature-container' do + render + + expect(rendered).to have_css('.js-signature-container') + end + end + + context 'with vue tree view enabled' do + before do + stub_feature_flags(vue_file_list: true) + end + + it 'are not rendered via js-signature-container' do + render + + expect(rendered).not_to have_css('.js-signature-container') + end + end + end end |