diff options
67 files changed, 893 insertions, 128 deletions
diff --git a/Gemfile.lock b/Gemfile.lock index 3dd1adb5f63..1bd88b65124 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -272,7 +272,7 @@ GEM ruby-progressbar (~> 1.4) gemojione (3.3.0) json - get_process_mem (0.2.0) + get_process_mem (0.2.3) gettext (3.2.9) locale (>= 2.0.5) text (>= 1.3.0) @@ -812,7 +812,7 @@ GEM ruby-progressbar (1.10.0) ruby-saml (1.7.2) nokogiri (>= 1.5.10) - ruby_parser (3.11.0) + ruby_parser (3.13.1) sexp_processor (~> 4.9) rubyntlm (0.6.2) rubypants (0.2.0) @@ -852,7 +852,7 @@ GEM sentry-raven (2.9.0) faraday (>= 0.7.6, < 1.0) settingslogic (2.0.9) - sexp_processor (4.11.0) + sexp_processor (4.12.0) sham_rack (1.3.6) rack shoulda-matchers (3.1.2) diff --git a/app/assets/javascripts/main.js b/app/assets/javascripts/main.js index b503c746801..213d5c6521a 100644 --- a/app/assets/javascripts/main.js +++ b/app/assets/javascripts/main.js @@ -147,7 +147,7 @@ function deferredInitialisation() { const canaryBadge = document.querySelector('.js-canary-badge'); const canaryLink = document.querySelector('.js-canary-link'); if (canaryBadge) { - canaryBadge.classList.remove('hidden'); + canaryBadge.classList.add('hidden'); } if (canaryLink) { canaryLink.classList.add('hidden'); diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index 060b09f015c..5d28635232b 100644 --- a/app/controllers/uploads_controller.rb +++ b/app/controllers/uploads_controller.rb @@ -45,7 +45,7 @@ class UploadsController < ApplicationController when Appearance true else - permission = "read_#{model.class.to_s.underscore}".to_sym + permission = "read_#{model.class.underscore}".to_sym can?(current_user, permission, model) end diff --git a/app/helpers/events_helper.rb b/app/helpers/events_helper.rb index 1371e9993b4..e990e425cb6 100644 --- a/app/helpers/events_helper.rb +++ b/app/helpers/events_helper.rb @@ -68,7 +68,7 @@ module EventsHelper end def event_preposition(event) - if event.push? || event.commented? || event.target + if event.push_action? || event.commented_action? || event.target "at" elsif event.milestone? "in" @@ -80,11 +80,11 @@ module EventsHelper words << event.author_name words << event_action_name(event) - if event.push? + if event.push_action? words << event.ref_type words << event.ref_name words << "at" - elsif event.commented? + elsif event.commented_action? words << event.note_target_reference words << "at" elsif event.milestone? @@ -121,9 +121,9 @@ module EventsHelper if event.note_target event_note_target_url(event) end - elsif event.push? + elsif event.push_action? push_event_feed_url(event) - elsif event.created_project? + elsif event.created_project_action? project_url(event.project) end end @@ -147,7 +147,7 @@ module EventsHelper def event_feed_summary(event) if event.issue? render "events/event_issue", issue: event.issue - elsif event.push? + elsif event.push_action? render "events/event_push", event: event elsif event.merge_request? render "events/event_merge_request", merge_request: event.merge_request diff --git a/app/models/application_record.rb b/app/models/application_record.rb index d1d01368972..0979d03f6e6 100644 --- a/app/models/application_record.rb +++ b/app/models/application_record.rb @@ -41,4 +41,8 @@ class ApplicationRecord < ActiveRecord::Base find_or_create_by(*args) end end + + def self.underscore + Gitlab::SafeRequestStore.fetch("model:#{self}:underscore") { self.to_s.underscore } + end end diff --git a/app/models/ci/job_artifact.rb b/app/models/ci/job_artifact.rb index f9cf398556d..3beb76ffc2b 100644 --- a/app/models/ci/job_artifact.rb +++ b/app/models/ci/job_artifact.rb @@ -99,7 +99,7 @@ module Ci raw: 1, zip: 2, gzip: 3 - } + }, _suffix: true # `file_location` indicates where actual files are stored. # Ideally, actual files should be stored in the same directory, and use the same diff --git a/app/models/event.rb b/app/models/event.rb index 593acf5edfe..738080eb584 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -68,7 +68,7 @@ class Event < ApplicationRecord # Callbacks after_create :reset_project_activity - after_create :set_last_repository_updated_at, if: :push? + after_create :set_last_repository_updated_at, if: :push_action? after_create :track_user_interacted_projects # Scopes @@ -138,11 +138,11 @@ class Event < ApplicationRecord # rubocop:disable Metrics/CyclomaticComplexity # rubocop:disable Metrics/PerceivedComplexity def visible_to_user?(user = nil) - if push? || commit_note? + if push_action? || commit_note? Ability.allowed?(user, :download_code, project) elsif membership_changed? Ability.allowed?(user, :read_project, project) - elsif created_project? + elsif created_project_action? Ability.allowed?(user, :read_project, project) elsif issue? || issue_note? Ability.allowed?(user, :read_issue, note? ? note_target : target) @@ -173,56 +173,56 @@ class Event < ApplicationRecord target.try(:title) end - def created? + def created_action? action == CREATED end - def push? + def push_action? false end - def merged? + def merged_action? action == MERGED end - def closed? + def closed_action? action == CLOSED end - def reopened? + def reopened_action? action == REOPENED end - def joined? + def joined_action? action == JOINED end - def left? + def left_action? action == LEFT end - def expired? + def expired_action? action == EXPIRED end - def destroyed? + def destroyed_action? action == DESTROYED end - def commented? + def commented_action? action == COMMENTED end def membership_changed? - joined? || left? || expired? + joined_action? || left_action? || expired_action? end - def created_project? - created? && !target && target_type.nil? + def created_project_action? + created_action? && !target && target_type.nil? end def created_target? - created? && target + created_action? && target end def milestone? @@ -258,23 +258,23 @@ class Event < ApplicationRecord end def action_name - if push? + if push_action? push_action_name - elsif closed? + elsif closed_action? "closed" - elsif merged? + elsif merged_action? "accepted" - elsif joined? + elsif joined_action? 'joined' - elsif left? + elsif left_action? 'left' - elsif expired? + elsif expired_action? 'removed due to membership expiration from' - elsif destroyed? + elsif destroyed_action? 'destroyed' - elsif commented? + elsif commented_action? "commented on" - elsif created_project? + elsif created_project_action? created_project_action_name else "opened" @@ -337,7 +337,7 @@ class Event < ApplicationRecord end def body? - if push? + if push_action? push_with_commits? elsif note? true diff --git a/app/models/members/group_member.rb b/app/models/members/group_member.rb index b266c61f002..4cba69069bb 100644 --- a/app/models/members/group_member.rb +++ b/app/models/members/group_member.rb @@ -12,7 +12,7 @@ class GroupMember < Member validates :source_type, format: { with: /\ANamespace\z/ } default_scope { where(source_type: SOURCE_TYPE) } - scope :in_groups, ->(groups) { where(source_id: groups.select(:id)) } + scope :of_groups, ->(groups) { where(source_id: groups.select(:id)) } scope :count_users_by_group_id, -> { joins(:user).group(:source_id).count } diff --git a/app/models/project.rb b/app/models/project.rb index 61d245478ca..ab4da61dcf8 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -56,7 +56,6 @@ class Project < ApplicationRecord VALID_MIRROR_PROTOCOLS = %w(http https ssh git).freeze ignore_column :import_status, :import_jid, :import_error - ignore_column :ci_id cache_markdown_field :description, pipeline: :description @@ -337,8 +336,8 @@ class Project < ApplicationRecord validates :star_count, numericality: { greater_than_or_equal_to: 0 } validate :check_personal_projects_limit, on: :create validate :check_repository_path_availability, on: :update, if: ->(project) { project.renamed? } - validate :visibility_level_allowed_by_group, if: -> { changes.has_key?(:visibility_level) } - validate :visibility_level_allowed_as_fork, if: -> { changes.has_key?(:visibility_level) } + validate :visibility_level_allowed_by_group, if: :should_validate_visibility_level? + validate :visibility_level_allowed_as_fork, if: :should_validate_visibility_level? validate :check_wiki_path_conflict validate :validate_pages_https_only, if: -> { changes.has_key?(:pages_https_only) } validates :repository_storage, @@ -892,6 +891,10 @@ class Project < ApplicationRecord self.errors.add(:limit_reached, error % { limit: limit }) end + def should_validate_visibility_level? + new_record? || changes.has_key?(:visibility_level) + end + def visibility_level_allowed_by_group return if visibility_level_allowed_by_group? diff --git a/app/models/push_event.rb b/app/models/push_event.rb index 9c0267c3140..4698df39730 100644 --- a/app/models/push_event.rb +++ b/app/models/push_event.rb @@ -69,7 +69,7 @@ class PushEvent < Event PUSHED end - def push? + def push_action? true end diff --git a/app/services/concerns/users/participable_service.rb b/app/services/concerns/users/participable_service.rb index a3cc6014fd3..1c828234f1b 100644 --- a/app/services/concerns/users/participable_service.rb +++ b/app/services/concerns/users/participable_service.rb @@ -29,7 +29,7 @@ module Users def groups group_counts = GroupMember - .in_groups(current_user.authorized_groups) + .of_groups(current_user.authorized_groups) .non_request .count_users_by_group_id diff --git a/app/services/members/destroy_service.rb b/app/services/members/destroy_service.rb index f9717a9426b..c8d5e563cd8 100644 --- a/app/services/members/destroy_service.rb +++ b/app/services/members/destroy_service.rb @@ -45,7 +45,7 @@ module Members def delete_subgroup_members(member) groups = member.group.descendants - GroupMember.in_groups(groups).with_user(member.user).each do |group_member| + GroupMember.of_groups(groups).with_user(member.user).each do |group_member| self.class.new(current_user).execute(group_member, skip_authorization: @skip_auth, skip_subresources: true) end end diff --git a/app/uploaders/attachment_uploader.rb b/app/uploaders/attachment_uploader.rb index 0a166335b4e..b488bba00e9 100644 --- a/app/uploaders/attachment_uploader.rb +++ b/app/uploaders/attachment_uploader.rb @@ -9,6 +9,6 @@ class AttachmentUploader < GitlabUploader private def dynamic_segment - File.join(model.class.to_s.underscore, mounted_as.to_s, model.id.to_s) + File.join(model.class.underscore, mounted_as.to_s, model.id.to_s) end end diff --git a/app/uploaders/avatar_uploader.rb b/app/uploaders/avatar_uploader.rb index c0165759203..9af59b0aceb 100644 --- a/app/uploaders/avatar_uploader.rb +++ b/app/uploaders/avatar_uploader.rb @@ -25,6 +25,6 @@ class AvatarUploader < GitlabUploader private def dynamic_segment - File.join(model.class.to_s.underscore, mounted_as.to_s, model.id.to_s) + File.join(model.class.underscore, mounted_as.to_s, model.id.to_s) end end diff --git a/app/uploaders/personal_file_uploader.rb b/app/uploaders/personal_file_uploader.rb index 272837aa6ce..f4697d898af 100644 --- a/app/uploaders/personal_file_uploader.rb +++ b/app/uploaders/personal_file_uploader.rb @@ -20,7 +20,7 @@ class PersonalFileUploader < FileUploader def self.model_path_segment(model) return 'temp/' unless model - File.join(model.class.to_s.underscore, model.id.to_s) + File.join(model.class.underscore, model.id.to_s) end def object_store diff --git a/app/views/events/_event.html.haml b/app/views/events/_event.html.haml index 2fcb1d1fd2b..222175c818a 100644 --- a/app/views/events/_event.html.haml +++ b/app/views/events/_event.html.haml @@ -3,11 +3,11 @@ .event-item-timestamp #{time_ago_with_tooltip(event.created_at)} - - if event.created_project? + - if event.created_project_action? = render "events/event/created_project", event: event - - elsif event.push? + - elsif event.push_action? = render "events/event/push", event: event - - elsif event.commented? + - elsif event.commented_action? = render "events/event/note", event: event - else = render "events/event/common", event: event diff --git a/app/views/projects/branches/_branch.html.haml b/app/views/projects/branches/_branch.html.haml index 91c51d5e091..0e15f581ddc 100644 --- a/app/views/projects/branches/_branch.html.haml +++ b/app/views/projects/branches/_branch.html.haml @@ -10,7 +10,7 @@ .branch-info .branch-title = sprite_icon('fork', size: 12) - = link_to project_tree_path(@project, branch.name), class: 'item-title str-truncated-100 ref-name prepend-left-8' do + = link_to project_tree_path(@project, branch.name), class: 'item-title str-truncated-100 ref-name prepend-left-8 qa-branch-name' do = branch.name - if branch.name == @repository.root_ref %span.badge.badge-primary.prepend-left-5 default diff --git a/app/views/shared/members/_member.html.haml b/app/views/shared/members/_member.html.haml index 2e5747121b6..2db1f67a793 100644 --- a/app/views/shared/members/_member.html.haml +++ b/app/views/shared/members/_member.html.haml @@ -53,7 +53,7 @@ = time_ago_with_tooltip(member.created_at) - if show_roles - current_resource = @project || @group - .controls.member-controls.row + .controls.member-controls - if show_controls && member.source == current_resource - if member.can_resend_invite? diff --git a/app/views/users/calendar_activities.html.haml b/app/views/users/calendar_activities.html.haml index 01acbf8eadd..3191eaa1e2c 100644 --- a/app/views/users/calendar_activities.html.haml +++ b/app/views/users/calendar_activities.html.haml @@ -9,7 +9,7 @@ %i.fa.fa-clock-o = event.created_at.to_time.in_time_zone.strftime('%-I:%M%P') - if event.visible_to_user?(current_user) - - if event.push? + - if event.push_action? #{event.action_name} #{event.ref_type} %strong - commits_path = project_commits_path(event.project, event.ref_name) diff --git a/changelogs/unreleased/61550-next-badge.yml b/changelogs/unreleased/61550-next-badge.yml new file mode 100644 index 00000000000..122e394a68c --- /dev/null +++ b/changelogs/unreleased/61550-next-badge.yml @@ -0,0 +1,5 @@ +--- +title: Fixes next badge being always visible +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/61606-support-string-piwik-website-ids.yml b/changelogs/unreleased/61606-support-string-piwik-website-ids.yml new file mode 100644 index 00000000000..5c525294132 --- /dev/null +++ b/changelogs/unreleased/61606-support-string-piwik-website-ids.yml @@ -0,0 +1,5 @@ +--- +title: "Supports Matomo/Piwik string website ID (\"Protect Track ID\" plugin)" +merge_request: 28214 +author: DUVERGIER Claude +type: fixed
\ No newline at end of file diff --git a/changelogs/unreleased/ce-11542-remove-non-semantic-use-of-row-in-member-listing-controls.yml b/changelogs/unreleased/ce-11542-remove-non-semantic-use-of-row-in-member-listing-controls.yml new file mode 100644 index 00000000000..c2dcd309abd --- /dev/null +++ b/changelogs/unreleased/ce-11542-remove-non-semantic-use-of-row-in-member-listing-controls.yml @@ -0,0 +1,5 @@ +--- +title: Remove non-semantic use of `.row` in member listing controls +merge_request: 28204 +author: +type: fixed diff --git a/changelogs/unreleased/fix-project-visibility-level-validation.yml b/changelogs/unreleased/fix-project-visibility-level-validation.yml new file mode 100644 index 00000000000..9581a475842 --- /dev/null +++ b/changelogs/unreleased/fix-project-visibility-level-validation.yml @@ -0,0 +1,5 @@ +--- +title: Fix project visibility level validation +merge_request: 28305 +author: Peter Marko +type: fixed diff --git a/changelogs/unreleased/include-ee-fixtures.yml b/changelogs/unreleased/include-ee-fixtures.yml new file mode 100644 index 00000000000..ba500d92de3 --- /dev/null +++ b/changelogs/unreleased/include-ee-fixtures.yml @@ -0,0 +1,5 @@ +--- +title: Add EE fixtures to SeedFu list +merge_request: 28241 +author: +type: other diff --git a/changelogs/unreleased/make-autocomplete-faster-with-lots-of-results.yml b/changelogs/unreleased/make-autocomplete-faster-with-lots-of-results.yml new file mode 100644 index 00000000000..daeefd3ffd7 --- /dev/null +++ b/changelogs/unreleased/make-autocomplete-faster-with-lots-of-results.yml @@ -0,0 +1,5 @@ +--- +title: Improve performance of users autocomplete when there are lots of results +merge_request: +author: +type: performance diff --git a/changelogs/unreleased/sh-update-process-mem.yml b/changelogs/unreleased/sh-update-process-mem.yml new file mode 100644 index 00000000000..51b22fb0f00 --- /dev/null +++ b/changelogs/unreleased/sh-update-process-mem.yml @@ -0,0 +1,5 @@ +--- +title: Update get_process_mem to 0.2.3 +merge_request: 28248 +author: +type: other diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index bff809b7661..23377b43f78 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -245,7 +245,7 @@ production: &base host: example.com port: 80 # Set to 443 if you serve the pages with HTTPS https: false # Set to true if you serve the pages with HTTPS - artifacts_server: true + artifacts_server: true # Set to false if you want to disable online view of HTML artifacts # external_http: ["1.1.1.1:80", "[2001::1]:80"] # If defined, enables custom domain support in GitLab Pages # external_https: ["1.1.1.1:443", "[2001::1]:443"] # If defined, enables custom domain and certificate support in GitLab Pages admin: diff --git a/config/initializers/01_secret_token.rb b/config/initializers/01_secret_token.rb index 4328ca509ba..e24b5cbd510 100644 --- a/config/initializers/01_secret_token.rb +++ b/config/initializers/01_secret_token.rb @@ -1,3 +1,14 @@ +# WARNING: If you add a new secret to this file, make sure you also +# update Omnibus GitLab or updates will fail. Omnibus is responsible for +# writing the `secrets.yml` file. If Omnibus doesn't know about a +# secret, Rails will attempt to write to the file, but this will fail +# because Rails doesn't have write access. +# +# As an example: +# * https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/27581 +# * https://gitlab.com/gitlab-org/omnibus-gitlab/merge_requests/3267 +# +# # This file needs to be loaded BEFORE any initializers that attempt to # prepend modules that require access to secrets (e.g. EE's 0_as_concern.rb). # diff --git a/config/initializers/seed_fu.rb b/config/initializers/seed_fu.rb new file mode 100644 index 00000000000..2e48e41a311 --- /dev/null +++ b/config/initializers/seed_fu.rb @@ -0,0 +1,4 @@ +# frozen_string_literal: true +if Gitlab.ee? + SeedFu.fixture_paths += %W[ee/db/fixtures ee/db/fixtures/#{Rails.env}] +end diff --git a/doc/administration/geo/disaster_recovery/background_verification.md b/doc/administration/geo/disaster_recovery/background_verification.md index 7d2fd51f834..c7299b6e196 100644 --- a/doc/administration/geo/disaster_recovery/background_verification.md +++ b/doc/administration/geo/disaster_recovery/background_verification.md @@ -157,6 +157,42 @@ For wikis: sudo -u git -H bundle exec rake geo:verification:wiki:reset RAILS_ENV=production ``` +## Reconcile differences with checksum mismatches + +If the **primary** and **secondary** nodes have a checksum verification mismatch, the cause may not be apparent. To find the cause of a checksum mismatch: + +1. Navigate to the **Admin Area > Projects** dashboard on the **primary** node, find the + project that you want to check the checksum differences and click on the + **Edit** button: +  + +1. On the project admin page get the **Gitaly storage name**, and **Gitaly relative path**: +  + +1. Navigate to the project's repository directory on both **primary** and **secondary** nodes. For an installation from source, the path is usually `/home/git/repositories`. For Omnibus installs, the path is usually `/var/opt/gitlab/git-data/repositories`. Note that if `git_data_dirs` is customized, check the directory layout on your server to be sure. + + ```sh + cd /var/opt/gitlab/git-data/repositories + ``` + +1. Run the following command on the **primary** node, redirecting the output to a file: + + ```sh + git show-ref --head | grep -E "HEAD|(refs/(heads|tags|keep-around|merge-requests|environments|notes)/)" > primary-node-refs + ``` + +1. Run the following command on the **secondary** node, redirecting the output to a file: + + ```sh + git show-ref --head | grep -E "HEAD|(refs/(heads|tags|keep-around|merge-requests|environments|notes)/)" > secondary-node-refs + ``` + +1. Copy the files from the previous steps on the same system, and do a diff between the contents: + + ```sh + diff primary-node-refs secondary-node-refs + ``` + ## Current limitations Until [issue #5064][ee-5064] is completed, background verification doesn't cover diff --git a/doc/administration/geo/disaster_recovery/img/checksum-differences-admin-project-page.png b/doc/administration/geo/disaster_recovery/img/checksum-differences-admin-project-page.png Binary files differnew file mode 100644 index 00000000000..fd51523104b --- /dev/null +++ b/doc/administration/geo/disaster_recovery/img/checksum-differences-admin-project-page.png diff --git a/doc/administration/geo/disaster_recovery/img/checksum-differences-admin-projects.png b/doc/administration/geo/disaster_recovery/img/checksum-differences-admin-projects.png Binary files differnew file mode 100644 index 00000000000..b2a6da69d3d --- /dev/null +++ b/doc/administration/geo/disaster_recovery/img/checksum-differences-admin-projects.png diff --git a/doc/api/repository_files.md b/doc/api/repository_files.md index 6fcc06ea8cd..87c7f371de1 100644 --- a/doc/api/repository_files.md +++ b/doc/api/repository_files.md @@ -181,7 +181,7 @@ Currently gitlab-shell has a boolean return code, preventing GitLab from specify ## Delete existing file in repository -This allows you to delete a single file. For deleting multiple files with a singleh request see the [commits API](commits.html#create-a-commit-with-multiple-files-and-actions). +This allows you to delete a single file. For deleting multiple files with a single request, see the [commits API](commits.html#create-a-commit-with-multiple-files-and-actions). ``` DELETE /projects/:id/repository/files/:file_path diff --git a/doc/development/architecture.md b/doc/development/architecture.md index 88c16a8db22..1e59b59b312 100644 --- a/doc/development/architecture.md +++ b/doc/development/architecture.md @@ -322,7 +322,7 @@ graph TB | Jaeger: GitLab instance | View traces generated by the GitLab instance | [❌](https://gitlab.com/gitlab-org/omnibus-gitlab/issues/4104) | [❌](https://gitlab.com/charts/gitlab/issues/1320) | [❌](https://gitlab.com/charts/gitlab/issues/1320) | [❌](https://gitlab.com/gitlab-org/omnibus-gitlab/issues/4104) | CE & EE | | Sentry: deployed apps | Error tracking for deployed apps | [⤓](https://docs.gitlab.com/ee/user/project/operations/error_tracking.html) | [⤓](https://docs.gitlab.com/ee/user/project/operations/error_tracking.html) | [⤓](https://docs.gitlab.com/ee/user/project/operations/error_tracking.html) | [⤓](https://docs.gitlab.com/ee/user/project/operations/error_tracking.html) | CE & EE | | Jaeger: deployed apps | Distributed tracing for deployed apps | [⤓](https://docs.gitlab.com/ee/user/project/operations/tracing.html) | [⤓](https://docs.gitlab.com/ee/user/project/operations/tracing.html) | [⤓](https://docs.gitlab.com/ee/user/project/operations/tracing.html) | [⤓](https://docs.gitlab.com/ee/user/project/operations/tracing.html) | EE Only | -| Kubernetes cluster apps | Deploy [Helm](https://docs.helm.sh/), [Ingress](https://kubernetes.io/docs/concepts/services-networking/ingress/), [Cert-Manager](https://docs.cert-manager.io/en/latest/), [Prometheus](https://prometheus.io/docs/introduction/overview/), a [Runner](https://docs.gitlab.com/runner/), [JupyterHub](http://jupyter.org/), [Knative](https://cloud.google.com/knative) to a cluster | [⤓](https://docs.gitlab.com/ee/user/project/clusters/#installing-applications) | [⤓](https://docs.gitlab.com/ee/user/project/clusters/#installing-applications) | [⤓](https://docs.gitlab.com/ee/user/project/clusters/#installing-applications) | [⤓](https://docs.gitlab.com/ee/user/project/clusters/#installing-applications) | CE & EE | +| Kubernetes cluster apps | Deploy [Helm](https://docs.helm.sh/), [Ingress](https://kubernetes.io/docs/concepts/services-networking/ingress/), [Cert-Manager](https://docs.cert-manager.io/en/latest/), [Prometheus](https://prometheus.io/docs/introduction/overview/), a [Runner](https://docs.gitlab.com/runner/), [JupyterHub](http://jupyter.org/), [Knative](https://cloud.google.com/knative) to a cluster | [✅](https://docs.gitlab.com/ee/user/project/clusters/#installing-applications) | [✅](https://docs.gitlab.com/ee/user/project/clusters/#installing-applications) | [✅](https://docs.gitlab.com/ee/user/project/clusters/#installing-applications) | [✅](https://docs.gitlab.com/ee/user/project/clusters/#installing-applications) | CE & EE | A typical install of GitLab will be on GNU/Linux. It uses Nginx or Apache as a web front end to proxypass the Unicorn web server. By default, communication between Unicorn and the front end is via a Unix domain socket but forwarding requests via TCP is also supported. The web front end accesses `/home/git/gitlab/public` bypassing the Unicorn server to serve static pages, uploads (e.g. avatar images or attachments), and precompiled assets. GitLab serves web pages and a [GitLab API](https://gitlab.com/gitlab-org/gitlab-ce/tree/master/doc/api) using the Unicorn web server. It uses Sidekiq as a job queue which, in turn, uses redis as a non-persistent database backend for job information, meta data, and incoming jobs. diff --git a/doc/development/fe_guide/graphql.md b/doc/development/fe_guide/graphql.md index 5f6123b5f9b..8e06aa5d173 100644 --- a/doc/development/fe_guide/graphql.md +++ b/doc/development/fe_guide/graphql.md @@ -43,9 +43,9 @@ new Vue({ Read more about [Vue Apollo][vue-apollo] in the [Vue Apollo documentation][vue-apollo-docs]. -### Local state with `apollo-link-state` +### Local state with Apollo -It is possible to use our Apollo setup with [apollo-link-state][apollo-link-state] by passing +It is possible to manage an application state with Apollo by passing in a resolvers object when creating the default client. The default state can be set by writing to the cache after setting up the default client. @@ -76,6 +76,8 @@ const apolloProvider = new VueApollo({ }); ``` +Read more about local state management with Apollo in the [Vue Apollo documentation](https://vue-apollo.netlify.com/guide/local-state.html#local-state). + ### Testing With [Vue test utils][vue-test-utils] it is easy to quickly test components that @@ -92,6 +94,8 @@ it('tests apollo component', () => { }); ``` +Another possible way is testing queries with mocked GraphQL schema. Read more about this way in [Vue Apollo testing documentation](https://vue-apollo.netlify.com/guide/testing.html#tests-with-mocked-graqhql-schema) + ## Usage outside of Vue It is also possible to use GraphQL outside of Vue by directly importing diff --git a/doc/development/testing_guide/frontend_testing.md b/doc/development/testing_guide/frontend_testing.md index f58a8dcbcdc..9bd99e80357 100644 --- a/doc/development/testing_guide/frontend_testing.md +++ b/doc/development/testing_guide/frontend_testing.md @@ -187,6 +187,7 @@ export default function doSomething() { visitUrl('/foo/bar'); } ``` + ```js // my_module_spec.js import doSomething from '~/my_module'; @@ -213,7 +214,187 @@ Further documentation on the babel rewire pluign API can be found on #### Waiting in tests -If you cannot avoid using [`setTimeout`](https://developer.mozilla.org/en-US/docs/Web/API/WindowOrWorkerGlobalScope/setTimeout) in tests, please use the [Jasmine mock clock](https://jasmine.github.io/api/2.9/Clock.html). +Sometimes a test needs to wait for something to happen in the application before it continues. +Avoid using [`setTimeout`](https://developer.mozilla.org/en-US/docs/Web/API/WindowOrWorkerGlobalScope/setTimeout) +because it makes the reason for waiting unclear and if passed a time larger than zero it will slow down our test suite. +Instead use one of the following approaches. + +##### Promises and Ajax calls + +Register handler functions to wait for the `Promise` to be resolved. + +```javascript +const askTheServer = () => { + return axios + .get('/endpoint') + .then(response => { + // do something + }) + .catch(error => { + // do something else + }); +}; +``` + +**in Jest:** + +```javascript +it('waits for an Ajax call', () => { + return askTheServer().then(() => { + expect(something).toBe('done'); + }); +}); +``` + +**in Karma:** + +```javascript +it('waits for an Ajax call', done => { + askTheServer() + .then(() => { + expect(something).toBe('done'); + }) + .then(done) + .catch(done.fail); +}); +``` + +If you are not able to register handlers to the `Promise`—for example because it is executed in a synchronous Vue life +cycle hook—you can flush all pending `Promise`s: + +**in Jest:** + +```javascript +it('waits for an Ajax call', () => { + synchronousFunction(); + jest.runAllTicks(); + + expect(something).toBe('done'); +}); +``` + +**in Karma:** + +You are out of luck. The following only works sometimes and may lead to flaky failures: + +```javascript +it('waits for an Ajax call', done => { + synchronousFunction(); + + // create a new Promise and hope that it resolves after the rest + Promise.resolve() + .then(() => { + expect(something).toBe('done'); + }) + .then(done) + .catch(done.fail); +}); +``` + +##### Vue rendering + +To wait until a Vue component is re-rendered, use either of the equivalent +[`Vue.nextTick()`](https://vuejs.org/v2/api/#Vue-nextTick) or `vm.$nextTick()`. + +**in Jest:** + +```javascript +it('renders something', () => { + wrapper.setProps({ value: 'new value' }); + + return wrapper.vm.$nextTick().then(() => { + expect(wrapper.text()).toBe('new value'); + }); +}); +``` + +**in Karma:** + +```javascript +it('renders something', done => { + wrapper.setProps({ value: 'new value' }); + + wrapper.vm + .$nextTick() + .then(() => { + expect(wrapper.text()).toBe('new value'); + }) + .then(done) + .catch(done.fail); +}); +``` + +##### `setTimeout()` / `setInterval()` in application + +If the application itself is waiting for some time, mock await the waiting. In Jest this is already +[done by default](https://gitlab.com/gitlab-org/gitlab-ce/blob/a2128edfee799e49a8732bfa235e2c5e14949c68/jest.config.js#L47) +(see also [Jest Timer Mocks](https://jestjs.io/docs/en/timer-mocks)). In Karma you can use the +[Jasmine mock clock](https://jasmine.github.io/api/2.9/Clock.html). + +```javascript +const doSomethingLater = () => { + setTimeout(() => { + // do something + }, 4000); +}; +``` + +**in Jest:** + +```javascript +it('does something', () => { + doSomethingLater(); + jest.runAllTimers(); + + expect(something).toBe('done'); +}); +``` + +**in Karma:** + +```javascript +it('does something', () => { + jasmine.clock().install(); + + doSomethingLater(); + jasmine.clock().tick(4000); + + expect(something).toBe('done'); + jasmine.clock().uninstall(); +}); +``` + +##### Events + +If the application triggers an event that you need to wait for in your test, register an event handler which contains +the assertions: + +```javascript +it('waits for an event', done => { + eventHub.$once('someEvent', eventHandler); + + someFunction(); + + function eventHandler() { + expect(something).toBe('done'); + done(); + } +}); +``` + +In Jest you can also use a `Promise` for this: + +```javascript +it('waits for an event', () => { + const eventTriggered = new Promise(resolve => eventHub.$once('someEvent', resolve)); + + someFunction(); + + return eventTriggered.then(() => { + expect(something).toBe('done'); + }); +}); +``` #### Migrating flaky Karma tests to Jest diff --git a/doc/install/installation.md b/doc/install/installation.md index f16bc04af34..1fff3e92280 100644 --- a/doc/install/installation.md +++ b/doc/install/installation.md @@ -448,6 +448,18 @@ sudo -u git cp config/database.yml.postgresql config/database.yml # MySQL only: sudo -u git cp config/database.yml.mysql config/database.yml +# PostgreSQL only: +# Remove host, username, and password lines from config/database.yml. +# Once modified, the `production` settings will be as follows: +# +# production: +# adapter: postgresql +# encoding: unicode +# database: gitlabhq_production +# pool: 10 +# +sudo -u git -H editor config/database.yml + # MySQL and remote PostgreSQL only: # Update username/password in config/database.yml. # You only need to adapt the production settings (first part). @@ -565,6 +577,18 @@ sudo -u git -H editor config.toml For more information about configuring Gitaly see [doc/administration/gitaly](../administration/gitaly). +### Start Gitaly + +Gitaly must be running for the next section. + +```sh +gitlab_path=/home/git/gitlab +gitaly_path=/home/git/gitaly + +sudo -u git -H $gitlab_path/bin/daemon_with_pidfile $gitlab_path/tmp/pids/gitaly.pid \ + $gitaly_path/gitaly $gitaly_path/config.toml >> $gitlab_path/log/gitaly.log 2>&1 & +``` + ### Initialize Database and Activate Advanced Features ```sh @@ -640,6 +664,12 @@ sudo -u git -H yarn install --production --pure-lockfile sudo -u git -H bundle exec rake gitlab:assets:compile RAILS_ENV=production NODE_ENV=production ``` +If `rake` fails with `JavaScript heap out of memory` error, try to run it with `NODE_OPTIONS` set as follows. + +```sh +sudo -u git -H bundle exec rake gitlab:assets:compile RAILS_ENV=production NODE_ENV=production NODE_OPTIONS="--max_old_space_size=4096" +``` + ### Start Your GitLab Instance ```sh diff --git a/doc/integration/salesforce.md b/doc/integration/salesforce.md index 18d42486fd6..8a99641a256 100644 --- a/doc/integration/salesforce.md +++ b/doc/integration/salesforce.md @@ -1,15 +1,15 @@ # SalesForce OmniAuth Provider -You can integrate your GitLab instance with [SalesForce](https://www.salesforce.com/) to enable users to login to your GitLab instance with their SalesForce account. +You can integrate your GitLab instance with [SalesForce](https://www.salesforce.com/) to enable users to login to your GitLab instance with their SalesForce account. ## Create SalesForce Application To enable SalesForce OmniAuth provider, you must use SalesForce's credentials for your GitLab instance. -To get the credentials (a pair of Client ID and Client Secret), you must register an application on UltraAuth. +To get the credentials (a pair of Client ID and Client Secret), you must register an application on SalesForces. 1. Sign in to [SalesForce](https://www.salesforce.com/). -1. Navigate to **Platform Tools/Apps** and click on **New Connected App**. +1. Navigate to **Platform Tools/Apps/App Manager** and click on **New Connected App**. 1. Fill in the application details into the following fields: - **Connected App Name** and **API Name**: Set to any value but consider something like `<Organization>'s GitLab`, `<Your Name>'s GitLab`, or something else that is descriptive. @@ -64,7 +64,7 @@ To get the credentials (a pair of Client ID and Client Secret), you must registe } ``` 1. Change `SALESFORCE_CLIENT_ID` to the Consumer Key from the SalesForce connected application page. -1. Change `SALESFORCE_CLIENT_SECRET` to the Client Secret from the SalesForce connected application page. +1. Change `SALESFORCE_CLIENT_SECRET` to the Consumer Secret from the SalesForce connected application page.  1. Save the configuration file. diff --git a/doc/user/admin_area/settings/continuous_integration.md b/doc/user/admin_area/settings/continuous_integration.md index 834a1e2423a..42fc4efa4ce 100644 --- a/doc/user/admin_area/settings/continuous_integration.md +++ b/doc/user/admin_area/settings/continuous_integration.md @@ -98,8 +98,7 @@ the group. NOTE: **Note:** Only available on GitLab.com. -If you have a Group with a [paid plan](https://about.gitlab.com/pricing/#gitlab-com) on GitLab.com, -then you can purchase additional CI minutes so your pipelines will not be blocked after you have +You can purchase additional CI minutes so your pipelines will not be blocked after you have used all your CI minutes from your main quota. In order to purchase additional minutes, you should follow these steps: diff --git a/doc/user/img/mermaid_diagram_render_gfm.png b/doc/user/img/mermaid_diagram_render_gfm.png Binary files differdeleted file mode 100644 index 9d192a30a85..00000000000 --- a/doc/user/img/mermaid_diagram_render_gfm.png +++ /dev/null diff --git a/doc/user/markdown.md b/doc/user/markdown.md index c1c2c036bae..c4f9fe45211 100644 --- a/doc/user/markdown.md +++ b/doc/user/markdown.md @@ -480,7 +480,7 @@ GitLab 10.3. > If this is not rendered correctly, see https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/user/markdown.md#mermaid -It is possible to generate diagrams and flowcharts from text using [Mermaid][mermaid]. +It is possible to generate diagrams and flowcharts from text using [Mermaid](https://mermaidjs.github.io/). In order to generate a diagram or flowchart, you should write your text inside the `mermaid` block. @@ -496,9 +496,15 @@ Example: Becomes: -<img src="./img/mermaid_diagram_render_gfm.png" width="200px" height="400px"> +```mermaid +graph TD; + A-->B; + A-->C; + B-->D; + C-->D; +``` -For details see the [Mermaid official page][mermaid]. +For details see the [Mermaid official page](https://mermaidjs.github.io/). ### Front matter @@ -1072,7 +1078,6 @@ A link starting with a `/` is relative to the wiki root. [^2]: This is my awesome footnote. [markdown.md]: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/user/markdown.md -[mermaid]: https://mermaidjs.github.io/ "Mermaid website" [rouge]: http://rouge.jneen.net/ "Rouge website" [redcarpet]: https://github.com/vmg/redcarpet "Redcarpet website" [katex]: https://github.com/Khan/KaTeX "KaTeX website" diff --git a/doc/user/project/integrations/webhooks.md b/doc/user/project/integrations/webhooks.md index 8e1603f9ec9..10eb3a4f3b7 100644 --- a/doc/user/project/integrations/webhooks.md +++ b/doc/user/project/integrations/webhooks.md @@ -321,8 +321,14 @@ X-Gitlab-Event: Issue Hook "group_id": 41 }], "changes": { - "updated_by_id": [null, 1], - "updated_at": ["2017-09-15 16:50:55 UTC", "2017-09-15 16:52:00 UTC"], + "updated_by_id": { + "previous": null, + "current": 1 + }, + "updated_at": { + "previous": "2017-09-15 16:50:55 UTC", + "current": "2017-09-15 16:52:00 UTC" + }, "labels": { "previous": [{ "id": 206, @@ -851,8 +857,14 @@ X-Gitlab-Event: Merge Request Hook "group_id": 41 }], "changes": { - "updated_by_id": [null, 1], - "updated_at": ["2017-09-15 16:50:55 UTC", "2017-09-15 16:52:00 UTC"], + "updated_by_id": { + "previous": null, + "current": 1 + }, + "updated_at": { + "previous": "2017-09-15 16:50:55 UTC", + "current":"2017-09-15 16:52:00 UTC" + }, "labels": { "previous": [{ "id": 206, diff --git a/doc/user/project/new_ci_build_permissions_model.md b/doc/user/project/new_ci_build_permissions_model.md index 6c3fa5eb463..d36312c9b8d 100644 --- a/doc/user/project/new_ci_build_permissions_model.md +++ b/doc/user/project/new_ci_build_permissions_model.md @@ -44,14 +44,14 @@ It is important to note that we have a few types of users: - **Administrators**: CI jobs created by Administrators will not have access to all GitLab projects, but only to projects and container images of projects - that the administrator is a member of.That means that if a project is either + that the administrator is a member of. That means that if a project is either public or internal users have access anyway, but if a project is private, the Administrator will have to be a member of it in order to have access to it via another project's job. - **External users**: CI jobs created by [external users](../permissions.md#external-users-permissions) will have access only to projects to which user has at least reporter access. This - rules out accessing all internal projects by default, + rules out accessing all internal projects by default. This allows us to make the CI and permission system more trustworthy. Let's consider the following scenario: diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 90ed24a2ded..296688ba25b 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -878,7 +878,7 @@ module API expose :push_event_payload, as: :push_data, using: PushEventPayload, - if: -> (event, _) { event.push? } + if: -> (event, _) { event.push_action? } expose :author_username do |event, options| event.author&.username diff --git a/lib/tasks/gitlab/features.rake b/lib/tasks/gitlab/features.rake index d115961108e..d88bcca0819 100644 --- a/lib/tasks/gitlab/features.rake +++ b/lib/tasks/gitlab/features.rake @@ -17,7 +17,7 @@ namespace :gitlab do if status Feature.enable(flag) else - Feature.disable(flag) + Feature.get(flag).remove end end end diff --git a/package.json b/package.json index eb557101662..f19baac0229 100644 --- a/package.json +++ b/package.json @@ -1,13 +1,17 @@ { "private": true, "scripts": { + "check-dependencies": "yarn check --integrity", "clean": "rm -rf public/assets tmp/cache/*-loader", "dev-server": "NODE_OPTIONS=\"--max-old-space-size=3584\" nodemon -w 'config/webpack.config.js' --exec 'webpack-dev-server --config config/webpack.config.js'", "eslint": "eslint --max-warnings 0 --report-unused-disable-directives --ext .js,.vue .", "eslint-fix": "eslint --max-warnings 0 --report-unused-disable-directives --ext .js,.vue --fix .", "eslint-report": "eslint --max-warnings 0 --ext .js,.vue --format html --output-file ./eslint-report.html --no-inline-config .", + "prejest": "yarn check-dependencies", + "jest": "jest", "jest-debug": "node --inspect-brk node_modules/.bin/jest --runInBand", "jsdoc": "jsdoc -c config/jsdocs.config.js", + "prekarma": "yarn check-dependencies", "karma": "BABEL_ENV=${BABEL_ENV:=karma} karma start --single-run true config/karma.config.js", "karma-coverage": "BABEL_ENV=coverage karma start --single-run true config/karma.config.js", "karma-start": "BABEL_ENV=karma karma start config/karma.config.js", diff --git a/qa/README.md b/qa/README.md index 8efdd8514f1..002ad4c65f5 100644 --- a/qa/README.md +++ b/qa/README.md @@ -49,8 +49,10 @@ will need to [modify your GDK setup](https://gitlab.com/gitlab-org/gitlab-qa/blo ### Writing tests -1. [Using page objects](qa/page/README.md) -2. [Style guide](STYLE_GUIDE.md) +- [Writing tests from scratch tutorial](docs/WRITING_TESTS_FROM_SCRATCH.md) + - [Best practices](docs/BEST_PRACTICES.md) + - [Using page objects](qa/page/README.md) + - [Guidelines](docs/GUIDELINES.md) ### Running specific tests diff --git a/qa/docs/BEST_PRACTICES.md b/qa/docs/BEST_PRACTICES.md new file mode 100644 index 00000000000..a872b915926 --- /dev/null +++ b/qa/docs/BEST_PRACTICES.md @@ -0,0 +1,38 @@ +# Best practices when writing end-to-end tests + +The majority of the end-to-end tests require some state to be built in the application for the tests to happen. + +A good example is a user being logged in as a pre-condition for testing the feature. + +But if the login feature is already covered with end-to-end tests through the GUI, there is no reason to perform such an expensive task to test the functionality of creating a project, or importing a repo, even if this features depend on a user being logged in. Let's see an example to make things clear. + +Let's say that, on average, the process to perform a successfull login through the GUI takes 2 seconds. + +Now, realize that almost all tests need the user to be logged in, and that we need every test to run in isolation, meaning that tests cannot interfere with each other. This would mean that for every test the user needs to log in, and "waste 2 seconds". + +Now, multiply the number of tests per 2 seconds, and as your test suite grows, the time to run it grows with it, and this is not sustainable. + +An alternative to perform a login in a cheaper way would be having an endpoint (available only for testing) where we could pass the user's credentials as encrypted values as query strings, and then we would be redirected to the logged in home page if the credentials are valid. Let's say that, on average, this process takes only 200 miliseconds. + +You see the point right? + +Performing a login through the GUI for every test would cost a lot in terms of tests' execution. + +And there is another reason. + +Let's say that you don't follow the above suggestion, and depend on the GUI for the creation of every application state in order to test a specific feature. In this case we could be talking about the **Issues** feature, that depends on a project to exist, and the user to be logged in. + +What would happen if there was a bug in the project creation page, where the 'Create' button is disabled, not allowing for the creation of a project through the GUI, but the API logic is still working? + +In this case, instead of having only the project creation test failing, we would have many tests that depend on a project to be failing too. + +But, if we were following the best practices, only one test would be failing, and tests for other features that depend on a project to exist would continue to pass, since they could be creating the project behind the scenes interacting directly with the public APIs, ensuring a more reliable metric of test failure rate. + +Finally, interacting with the application only by its GUI generates a higher rate of test flakiness, and we want to avoid that at max. + +**The takeaways here are:** + +- Building state through the GUI is time consuming and it's not sustainable as the test suite grows. +- When depending only on the GUI to create the application's state and tests fail due to front-end issues, we can't rely on the test failures rate, and we generates a higher rate of test flakiness. + +Now that we are aware of all of it, [let's go create some tests](./WRITING_TESTS_FROM_SCRATCH.md). diff --git a/qa/STYLE_GUIDE.md b/qa/docs/GUIDELINES.md index 900f7456e1a..9db52cd07e6 100644 --- a/qa/STYLE_GUIDE.md +++ b/qa/docs/GUIDELINES.md @@ -43,4 +43,4 @@ end Notice that in the above example, before clicking the `:operations_environments_link`, another element is hovered over. -> We can create these methods as helpers to abstract multi-step navigation.
\ No newline at end of file +> We can create these methods as helpers to abstract multi-step navigation. diff --git a/qa/docs/WRITING_TESTS_FROM_SCRATCH.md b/qa/docs/WRITING_TESTS_FROM_SCRATCH.md new file mode 100644 index 00000000000..a6daffc964e --- /dev/null +++ b/qa/docs/WRITING_TESTS_FROM_SCRATCH.md @@ -0,0 +1,380 @@ +# Writing end-to-end tests step-by-step + +In this tutorial, you will find different examples, and the steps involved, in the creation of end-to-end (_e2e_) tests for GitLab CE and GitLab EE, using GitLab QA. + +> When referring to end-to-end tests in this document, this means testing a specific feature end-to-end, such as a user logging in, the creation of a project, the management of labels, breaking down epics into sub-epics and issues, etc. + +## Important information before we start writing tests + +It's important to understand that end-to-end tests of isolated features, such as the ones described in the above note, doesn't mean that everything needs to happen through the GUI. + +If you don't exactly understand what we mean by **not everything needs to happen through the GUI,** please make sure you've read the [best practices](./BEST_PRACTICES.md) before moving on. + +## This document covers the following items: + +0. Identifying if end-to-end tests are really needed +1. Identifying the [DevOps stage](https://about.gitlab.com/stages-devops-lifecycle/) of the feature that you are going to cover with end-to-end tests +2. Creating the skeleton of the test file (`*_spec.rb`) +3. The [MVC](https://about.gitlab.com/handbook/values/#minimum-viable-change-mvc) of the test cases logic +4. Extracting duplicated code into methods +5. Tests' pre-conditions (`before :all` and `before`) using resources and [Page Objects](./qa/page/README.md) +6. Optimizing the test suite +7. Using and implementing resources +8. Moving elements definitions and its methods to [Page Objects](./qa/page/README.md) + - Adding testability to the application + +### 0. Are end-to-end tests needed? + +At GitLab we respect the [test pyramid](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/testing_guide/testing_levels.md), and so, we recommend to check the code coverage of a specific feature before writing end-to-end tests. + +Sometimes you may notice that there is already a good coverage in other test levels, and we can stay confident that if we break a feature, we will still have quick feedback about it, even without having end-to-end tests. + +If after this analysis you still think that end-to-end tests are needed, keep reading. + +### 1. Identifying the DevOps stage + +The GitLab QA end-to-end tests are organized by the different [stages in the DevOps lifecycle](https://gitlab.com/gitlab-org/gitlab-ce/tree/master/qa/qa/specs/features/browser_ui), and so, if you are creating tests for issue creation, for instance, you would locate the spec files under the `qa/qa/specs/features/browser_ui/2_plan/` directory since issue creation is part of the Plan stage. + + In another case of a test for listing merged merge requests (MRs), the test should go under the `qa/qa/specs/features/browser_ui/3_create/` directory since merge request is a feature from the Create stage. + +> There may be sub-directories inside the stages directories, for different features. For example: `.../browser_ui/2_plan/ee_epics/` and `.../browser_ui/2_plan/issues/`. + +Now, let's say we want to create tests for the [scoped labels](https://about.gitlab.com/2019/04/22/gitlab-11-10-released/#scoped-labels) feature, available on GitLab EE Premium (this feature is part of the Plan stage.) + +> Because these tests are for a feature available only on GitLab EE, we need to create them in the [EE repository](https://gitlab.com/gitlab-org/gitlab-ee). + +Since [there is no specific directory for this feature](https://gitlab.com/gitlab-org/gitlab-ee/tree/master/qa/qa/specs/features/browser_ui/2_plan), we should create a sub-directory for it. + +Under `.../browser_ui/2_plan/`, let's create a sub-directory called `ee_scoped_labels/`. + +> Notice that since this feature is only available for GitLab EE we prefix the sub-directory with `ee_`. + +### 2. Test skeleton + +Inside the newly created sub-directory, let's create a file describing the test suite (e.g. `editing_scoped_labels_spec.rb`.) + +#### The `context` and `describe` blocks + +Specs have an outer `context` that indicates the DevOps stage. The next level is the `describe` block, that briefly states the subject of the test suite. See the following example: + +```ruby +module QA + context 'Plan' do + describe 'Editing scoped labels properties on issues' do + end + end +end +``` + +#### The `it` blocks + +Every test suite is composed by at least one `it` block, and a good way to start writing end-to-end tests is by typing test cases descriptions as `it` blocks. Take a look at the following example: + +```ruby +module QA + context 'Plan' do + describe 'Editing scoped labels properties on issues' do + it 'replaces an existing label if it has the same key' do + end + + it 'keeps both scoped labels when adding a label with a different key' do + end + end + end +end +``` + +### 3. Test cases MVC + +For the [MVC](https://about.gitlab.com/handbook/values/#minimum-viable-change-mvc) of our test cases, let's say that we already have the application in the state needed for the tests, and then let's focus on the logic of the test cases only. + +To evolve the test cases drafted on step 2, let's imagine that the user is already logged in a GitLab EE instance, they already have at least a Premium license in use, there is already a project created, there is already an issue opened in the project, the issue already has a scoped label (e.g. `foo::bar`), there are other scoped labels (for the same scope and for a different scope, e.g. `foo::baz` and `bar::bah`), and finally, the user is already on the issue's page. Let's also suppose that for every test case the application is in a clean state, meaning that one test case won't affect another. + +> Note: there are different approaches to create an application state for end-to-end tests. Some of them are very time consuming and subject to failures, such as when using the GUI for all the pre-conditions of the tests. On the other hand, other approaches are more efficient, such as using the public APIs. The latter is more efficient since it doesn't depend on the GUI. We won't focus on this part yet, but it's good to keep it in mind. + +Let's now focus on the first test case. + +```ruby +it 'keeps the latest scoped label when adding a label with the same key of an existing one, but with a different value' do + # This implementation is only for tutorial purposes. We normally encapsulate elements in Page Objects. + page.find('.block.labels .edit-link').click + page.find('.dropdown-menu-labels .dropdown-input-field').send_keys ['foo::baz', :enter] + page.find('#content-body').click + page.refresh + + scoped_label = page.find('.qa-labels-block .scoped-label-wrapper') + + expect(scoped_label).to have_content('foo::baz') + expect(scoped_label).not_to have_content('foo::bar') + expect(page).to have_content('added foo::baz label and removed foo::bar') +end +``` + +> Notice that the test itself is simple. The most challenging part is the creation of the application state, which will be covered later. + +> The exemplified test cases' MVC is not enough for the change to be submitted in an MR, but they help on building up the test logic. The reason is that we do not want to use locators directly in the tests, and tests **must** use [Page Objects](./qa/page/README.md) before they can be merged. + +Below are the steps that the test covers: + +1. The test finds the 'Edit' link for the labels and clicks on it +2. Then it fills in the 'Assign labels' input field with the value 'foo::baz' and press enter +3. Then it clicks in the content body to apply the label and refreshes the page +4. Finally the expectation that the previous scoped label was removed and that the new one was added happens + +Let's now see how the second test case would look like. + +```ruby +it 'keeps both scoped labels when adding a label with a different key' do + # This implementation is only for tutorial purposes. We normally encapsulate elements in Page Objects. + page.find('.block.labels .edit-link').click + page.find('.dropdown-menu-labels .dropdown-input-field').send_keys ['bar::bah', :enter] + page.find('#content-body').click + page.refresh + + scoped_labels = page.all('.qa-labels-block .scoped-label-wrapper') + + expect(scoped_labels.first).to have_content('bar::bah') + expect(scoped_labels.last).to have_content('foo::ba') + expect(page).to have_content('added bar::bah') + expect(page).to have_content('added foo::ba') +end +``` + +> Note that elements are always located using CSS selectors, and a good practice is to add test specific attribute:value for elements (this is called adding testability to the application and we will talk more about it later.) + +Below are the steps that the test covers: + +1. The test finds the 'Edit' link for the labels and clicks on it +2. Then it fills in the 'Assign labels' input field with the value 'bar::bah' and press enter +3. Then it clicks in the content body to apply the label and refreshes the page +4. Finally the expectation that the both scoped labels are present happens + +> Similar to the previous test, this one is also very straight forward, but there is some code duplication. Let's address it. + +### 4. Extracting duplicated code + +If we refactor the tests created on step 3 we could come up with something like this: + +```ruby +it 'keeps the latest scoped label when adding a label with the same key of an existing one, but with a different value' do + select_label_and_refresh 'foo::baz' + + expect(page).to have_content('added foo::baz') + expect(page).to have_content('and removed foo::bar') + + scoped_label = page.find('.qa-labels-block .scoped-label-wrapper') + + expect(scoped_label).to have_content('foo::baz') + expect(scoped_label).not_to have_content('foo::bar') +end + +it 'keeps both scoped label when adding a label with a different key' do + select_label_and_refresh 'bar::bah' + + expect(page).to have_content('added bar::bah') + expect(page).to have_content('added foo::ba') + + scoped_labels = page.all('.qa-labels-block .scoped-label-wrapper') + + expect(scoped_labels.first).to have_content('bar::bah') + expect(scoped_labels.last).to have_content('foo::ba') +end + +def select_label_and_refresh(label) + page.find('.block.labels .edit-link').click + page.find('.dropdown-menu-labels .dropdown-input-field').send_keys [label, :enter] + page.find('#content-body').click + page.refresh +end +``` + +By creating a reusable `select_label_and_refresh` method we remove the code duplication, and later we can move this method to a Page Object class that will be created for easier maintenance purposes. + +> Notice that the reusable method is created in the bottom of the file. The reason for that is that reading the code should be similar to reading a newspaper, where high-level information is at the top, like the title and summary of the news, while low level, or more specific information, is at the bottom. + +### 5. Tests' pre-conditions using resources and Page Objects + +In this section, we will address the previously mentioned subject of creating the application state for the tests, using the `before :all` and `before` blocks, together with resources and Page Objects. + +#### `before :all` + +A pre-condition for the entire test suite is defined in the `before :all` block. + +For our test suite example, some things that could happen before the entire test suite starts are: + +- The user logging in; +- A premium license already being set up; +- A project being created with an issue and labels already setup. + +> In case of a test suite with only one `it` block it's ok to use only the `before` block (see below) with all the test's pre-conditions. + +#### `before` + +A pre-condition for each test case is defined in the `before` block. + +For our test cases samples, what we need is that for every test the issue page is opened, and there is only one scoped label applied to it. + +#### Implementation + +In the following code we will focus on the test suite and the test cases' pre-conditions only: + +```ruby +module QA + context 'Plan' do + describe 'Editing scoped labels properties on issues' do + before :all do + project = Resource::Project.fabricate_via_api! do |resource| + resource.name = 'scoped-labels-project' + end + + @foo_bar_scoped_label = 'foo::bar' + + @issue = Resource::Issue.fabricate_via_api! do |issue| + issue.project = project + issue.title = 'Issue to test the scoped labels' + issue.labels = @foo_bar_scoped_label + end + + @labels = ['foo::baz', 'bar::bah'] + @labels.each do |label| + Resource::Label.fabricate_via_api! do |l| + l.project = project.id + l.title = label + end + end + + Runtime::Browser.visit(:gitlab, Page::Main::Login) + Page::Main::Login.perform(&:sign_in_using_credentials) + end + + before do + Page::Project::Issue::Show.perform do |issue_page| + @issue.visit! + end + end + + it 'keeps the latest scoped label when adding a label with the same key of an existing one, but with a different value' do + ... + end + + it 'keeps both scoped labels when adding a label with a different key' do + ... + end + + def select_label_and_refresh(label) + ... + end + end + end +end +``` + +In the `before :all` block we create all the application state needed for the tests to run. We do that by fabricating resources via APIs (`project`, `@issue`, and `@labels`), by using the `Runtime::Browser.visit` method to go to the login page, and by performing a `sign_in_using_credentials` from the `Login` Page Object. + +> When creating the resources, notice that when calling the `fabricate_via_api` method, we pass some attribute:values, like `name` for the `project` resoruce, `project`, `title`, and `labels` for the the issue resource, and `project`, and `title` for `label` resources. + +> What's important to understand here is that by creating the application state mostly using the public APIs we save a lot of time in the test suite setup stage. + +> Soon we will cover the use of the already existing resources' methods and the creation of your own `fabricate_via_api` methods for resources where this is still not available, but first, let's optimize our implementation. + +### 6. Optimization + +As already mentioned in the [best practices](./BEST_PRACTICES.md) document, end-to-end tests are very costly in terms of execution time, and it's our responsibility as software engineers to ensure that we optimize them as max as possible. + +> Differently than unit tests, that exercise every little piece of the application in isolation, usually having only one assertion per test, and being very fast to run, end-to-end tests can have more actions and assertions in a single test to help on speeding up the test's feedback since they are much slower when comparing to unit tests. + +Some improvements that we could make in our test suite to optimize its time to run are: + +1. Having a single test case (an `it` block) that exercise both scenarios to avoid "wasting" time in the tests' pre-conditions, instead of having two different test cases. +2. Moving all the pre-conditions to the `before` block since there will be only one `it` block. +3. Making the selection of labels more performant by allowing for the selection of more than one label in the same reusable method. + +Let's look at a suggestion that addresses the above points, one by one: + +```ruby +module QA + context 'Plan' do + describe 'Editing scoped labels properties on issues' do + before do + project = Resource::Project.fabricate_via_api! do |resource| + resource.name = 'scoped-labels-project' + end + + @foo_bar_scoped_label = 'foo::bar' + + @issue = Resource::Issue.fabricate_via_api! do |issue| + issue.project = project + issue.title = 'Issue to test the scoped labels' + issue.labels = @foo_bar_scoped_label + end + + @labels = ['foo::baz', 'bar::bah'] + @labels.each do |label| + Resource::Label.fabricate_via_api! do |l| + l.project = project.id + l.title = label + end + end + + Runtime::Browser.visit(:gitlab, Page::Main::Login) + Page::Main::Login.perform(&:sign_in_using_credentials) + Page::Project::Issue::Show.perform do |issue_page| + @issue.visit! + end + end + + it 'correctly applies the scoped labels depending if they are from the same or a different scope' do + select_labels_and_refresh @labels + + scoped_labels = page.all('.qa-labels-block .scoped-label-wrapper') + + expect(page).to have_content("added #{@foo_bar_scoped_label}") + expect(page).to have_content("added #{@labels[1]} #{@labels[0]} labels and removed #{@foo_bar_scoped_label}") + expect(scoped_labels.count).to eq(2) + expect(scoped_labels.first).to have_content(@labels[1]) + expect(scoped_labels.last).to have_content(@labels[0]) + end + + def select_labels_and_refresh(labels) + find('.block.labels .edit-link').click + labels.each do |label| + find('.dropdown-menu-labels .dropdown-input-field').send_keys [label, :enter] + end + find('#content-body').click + refresh + end + end + end + end +``` + +As you can see, now all the pre-conditions from the `before :all` block were moved to the `before` block, addressing point 2. + +To address point 1, we changed the test implementation from two `it` blocks into a single one that exercises both scenarios. Now the new test description is: `'correctly applies the scoped labels depending if they are from the same or a different scope'`. It's a long description, but it describes well what the test does. + +> Notice that the implementation of the new and unique `it` block had to change a little bit. Below we describe in details what it does. + +1. At the same time, it selects two scoped labels, one from the same scope of the one already applied in the issue during the setup phase (in the `before` block), and another one from a different scope. +2. It runs the assertions that the labels where correctly added and removed; that only two labels are applied; and that those are the correct ones, and that they are shown in the right order. + +Finally, the `select_label_and_refresh` method is changed to `select_labels_and_refresh`, which accepts an array of labels instead of a single label, and it iterates on them for faster label selection (this is what is used in step 1 explained above.) + +### 7. Resources + +TBD. + +### 8. Page Objects + +> Page Objects are auto-loaded in the `qa/qa.rb` file and available in all the test files (`*_spec.rb`). + +Page Objects are used in end-to-end tests for maintenance reasons, where page's elements and methods are defined to be reused in any test. + +Take a look at [this document that specifically details the usage of Page Objects](./qa/page/README.md). + +Now, let's go back to our examples. + +... + +#### Adding testability + +TBD.
\ No newline at end of file diff --git a/qa/qa/page/base.rb b/qa/qa/page/base.rb index 9fabf83e2ce..c395e5f6011 100644 --- a/qa/qa/page/base.rb +++ b/qa/qa/page/base.rb @@ -113,8 +113,8 @@ module QA has_css?(element_selector_css(name), wait: wait, text: text) end - def has_no_element?(name, wait: Capybara.default_max_wait_time) - has_no_css?(element_selector_css(name), wait: wait) + def has_no_element?(name, text: nil, wait: Capybara.default_max_wait_time) + has_no_css?(element_selector_css(name), wait: wait, text: text) end def has_text?(text) @@ -129,8 +129,8 @@ module QA has_no_css?('.fa-spinner', wait: Capybara.default_max_wait_time) end - def within_element(name) - page.within(element_selector_css(name)) do + def within_element(name, text: nil) + page.within(element_selector_css(name), text: text) do yield end end diff --git a/qa/qa/page/project/branches/show.rb b/qa/qa/page/project/branches/show.rb index 922a6ddb086..762eacdab15 100644 --- a/qa/qa/page/project/branches/show.rb +++ b/qa/qa/page/project/branches/show.rb @@ -7,6 +7,7 @@ module QA class Show < Page::Base view 'app/views/projects/branches/_branch.html.haml' do element :remove_btn + element :branch_name end view 'app/views/projects/branches/_panel.html.haml' do element :all_branches @@ -27,11 +28,9 @@ module QA finished_loading? end - def has_branch_title?(branch_title) + def has_no_branch?(branch_name) within_element(:all_branches) do - within(".item-title") do - has_text?(branch_title) - end + has_no_element?(:branch_name, text: branch_name, wait: Support::Waiter::DEFAULT_MAX_WAIT_TIME) end end @@ -48,15 +47,6 @@ module QA click_element(:delete_merged_branches) end end - - def wait_for_texts_not_to_be_visible(texts) - text_not_visible = wait do - texts.all? do |text| - has_no_text?(text) - end - end - raise "Expected text(s) #{texts} not to be visible" unless text_not_visible - end end end end diff --git a/qa/qa/specs/features/browser_ui/3_create/repository/add_list_delete_branches_spec.rb b/qa/qa/specs/features/browser_ui/3_create/repository/add_list_delete_branches_spec.rb index c2c2b6da90a..cf6c24fa873 100644 --- a/qa/qa/specs/features/browser_ui/3_create/repository/add_list_delete_branches_spec.rb +++ b/qa/qa/specs/features/browser_ui/3_create/repository/add_list_delete_branches_spec.rb @@ -73,10 +73,9 @@ module QA Page::Project::Branches::Show.perform do |branches_view| branches_view.delete_branch(third_branch) + expect(branches_view).to have_no_branch(third_branch) end - expect(page).not_to have_content(third_branch) - Page::Project::Branches::Show.perform(&:delete_merged_branches) expect(page).to have_content( @@ -85,8 +84,7 @@ module QA page.refresh Page::Project::Branches::Show.perform do |branches_view| - branches_view.wait_for_texts_not_to_be_visible([commit_message_of_second_branch]) - expect(branches_view).not_to have_branch_title(second_branch) + expect(branches_view).to have_no_branch(second_branch) end end end diff --git a/qa/qa/support/page/logging.rb b/qa/qa/support/page/logging.rb index 69b6332ecce..ff505fdbddd 100644 --- a/qa/qa/support/page/logging.rb +++ b/qa/qa/support/page/logging.rb @@ -76,23 +76,18 @@ module QA super end - def has_element?(name, text: nil, wait: Capybara.default_max_wait_time) + def has_element?(name, **kwargs) found = super - msg = ["has_element? :#{name}"] - msg << %Q(with text "#{text}") if text - msg << "(wait: #{wait})" - msg << "returned: #{found}" - - log(msg.compact.join(' ')) + log_has_element_or_not('has_element?', name, found, **kwargs) found end - def has_no_element?(name, wait: Capybara.default_max_wait_time) + def has_no_element?(name, **kwargs) found = super - log("has_no_element? :#{name} returned #{found}") + log_has_element_or_not('has_no_element?', name, found, **kwargs) found end @@ -149,6 +144,15 @@ module QA def log(msg) QA::Runtime::Logger.debug(msg) end + + def log_has_element_or_not(method, name, found, **kwargs) + msg = ["#{method} :#{name}"] + msg << %Q(with text "#{kwargs[:text]}") if kwargs[:text] + msg << "(wait: #{kwargs[:wait] || Capybara.default_max_wait_time})" + msg << "returned: #{found}" + + log(msg.compact.join(' ')) + end end end end diff --git a/qa/qa/support/waiter.rb b/qa/qa/support/waiter.rb index 21a399b4a5f..fdcf2d7e157 100644 --- a/qa/qa/support/waiter.rb +++ b/qa/qa/support/waiter.rb @@ -3,9 +3,11 @@ module QA module Support module Waiter + DEFAULT_MAX_WAIT_TIME = 60 + module_function - def wait(max: 60, interval: 0.1) + def wait(max: DEFAULT_MAX_WAIT_TIME, interval: 0.1) QA::Runtime::Logger.debug("with wait: max #{max}; interval #{interval}") start = Time.now diff --git a/qa/spec/page/logging_spec.rb b/qa/spec/page/logging_spec.rb index 707a7ff6d98..99e96b81a51 100644 --- a/qa/spec/page/logging_spec.rb +++ b/qa/spec/page/logging_spec.rb @@ -93,7 +93,14 @@ describe QA::Support::Page::Logging do allow(page).to receive(:has_no_css?).and_return(true) expect { subject.has_no_element?(:element) } - .to output(/has_no_element\? :element returned true/).to_stdout_from_any_process + .to output(/has_no_element\? :element \(wait: 2\) returned: true/).to_stdout_from_any_process + end + + it 'logs has_no_element? with text' do + allow(page).to receive(:has_no_css?).and_return(true) + + expect { subject.has_no_element?(:element, text: "more text") } + .to output(/has_no_element\? :element with text \"more text\" \(wait: 2\) returned: true/).to_stdout_from_any_process end it 'logs has_text?' do diff --git a/spec/controllers/concerns/send_file_upload_spec.rb b/spec/controllers/concerns/send_file_upload_spec.rb index 8408578a7db..a3ce08f736c 100644 --- a/spec/controllers/concerns/send_file_upload_spec.rb +++ b/spec/controllers/concerns/send_file_upload_spec.rb @@ -1,3 +1,4 @@ +# coding: utf-8 # frozen_string_literal: true require 'spec_helper' @@ -13,7 +14,7 @@ describe SendFileUpload do # user/:id def dynamic_segment - File.join(model.class.to_s.underscore, model.id.to_s) + File.join(model.class.underscore, model.id.to_s) end end end diff --git a/spec/factories/uploads.rb b/spec/factories/uploads.rb index 7256f785e1f..426abdc2a6c 100644 --- a/spec/factories/uploads.rb +++ b/spec/factories/uploads.rb @@ -13,7 +13,7 @@ FactoryBot.define do end # this needs to comply with RecordsUpload::Concern#upload_path - path { File.join("uploads/-/system", model.class.to_s.underscore, mount_point.to_s, 'avatar.jpg') } + path { File.join("uploads/-/system", model.class.underscore, mount_point.to_s, 'avatar.jpg') } trait :personal_snippet_upload do uploader "PersonalFileUploader" diff --git a/spec/lib/gitlab/favicon_spec.rb b/spec/lib/gitlab/favicon_spec.rb index 49a423191bb..dce56bbd2c4 100644 --- a/spec/lib/gitlab/favicon_spec.rb +++ b/spec/lib/gitlab/favicon_spec.rb @@ -7,7 +7,7 @@ RSpec.describe Gitlab::Favicon, :request_store do expect(described_class.main).to match_asset_path '/assets/favicon.png' end - it 'has blue favicon for development' do + it 'has blue favicon for development', unless: Gitlab.ee? do allow(Rails).to receive(:env).and_return(ActiveSupport::StringInquirer.new('development')) expect(described_class.main).to match_asset_path '/assets/favicon-blue.png' end diff --git a/spec/models/application_record_spec.rb b/spec/models/application_record_spec.rb index cc90a998d3f..74573d0941c 100644 --- a/spec/models/application_record_spec.rb +++ b/spec/models/application_record_spec.rb @@ -52,4 +52,10 @@ describe ApplicationRecord do expect { Suggestion.find_or_create_by!(note: nil) }.to raise_error(ActiveRecord::RecordInvalid) end end + + describe '.underscore' do + it 'returns the underscored value of the class as a string' do + expect(MergeRequest.underscore).to eq('merge_request') + end + end end diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb index e91b5c4c86f..62663c247d1 100644 --- a/spec/models/event_spec.rb +++ b/spec/models/event_spec.rb @@ -88,7 +88,7 @@ describe Event do let(:event) { create_push_event(project, user) } it do - expect(event.push?).to be_truthy + expect(event.push_action?).to be_truthy expect(event.visible_to_user?(user)).to be_truthy expect(event.visible_to_user?(nil)).to be_falsey expect(event.tag?).to be_falsey diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 2a17bd6002e..425096d7e80 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -214,6 +214,13 @@ describe Project do expect(project2).not_to be_valid end + it 'validates the visibility' do + expect_any_instance_of(described_class).to receive(:visibility_level_allowed_as_fork).and_call_original + expect_any_instance_of(described_class).to receive(:visibility_level_allowed_by_group).and_call_original + + create(:project) + end + describe 'wiki path conflict' do context "when the new path has been used by the wiki of other Project" do it 'has an error on the name attribute' do diff --git a/spec/models/push_event_spec.rb b/spec/models/push_event_spec.rb index f86500f91cd..5509ed87308 100644 --- a/spec/models/push_event_spec.rb +++ b/spec/models/push_event_spec.rb @@ -123,9 +123,9 @@ describe PushEvent do end end - describe '#push?' do + describe '#push_action?' do it 'returns true' do - expect(event).to be_push + expect(event).to be_push_action end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 9266bee34d6..69589c9aa33 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -138,7 +138,7 @@ RSpec.configure do |config| .and_return(false) end - config.before(:example, :quarantine) do + config.around(:example, :quarantine) do # Skip tests in quarantine unless we explicitly focus on them. skip('In quarantine') unless config.inclusion_filter[:quarantine] end diff --git a/spec/support/helpers/features/notes_helpers.rb b/spec/support/helpers/features/notes_helpers.rb index 38f30a14409..8a139fafac2 100644 --- a/spec/support/helpers/features/notes_helpers.rb +++ b/spec/support/helpers/features/notes_helpers.rb @@ -25,12 +25,10 @@ module Spec page.within('.js-main-target-form') do filled_text = fill_in('note[note]', with: text) - begin - # Dismiss quick action prompt if it appears - filled_text.parent.send_keys(:escape) - rescue Selenium::WebDriver::Error::ElementNotInteractableError - # It's fine if we can't escape when there's no prompt. - end + # Wait for quick action prompt to load and then dismiss it with ESC + # because it may block the Preview button + wait_for_requests + filled_text.send_keys(:escape) click_on('Preview') diff --git a/spec/support/helpers/graphql_helpers.rb b/spec/support/helpers/graphql_helpers.rb index f15944652fd..44ed9da25fc 100644 --- a/spec/support/helpers/graphql_helpers.rb +++ b/spec/support/helpers/graphql_helpers.rb @@ -197,3 +197,7 @@ module GraphqlHelpers allow(GitlabSchema).to receive(:max_query_depth).with(any_args).and_return nil end end + +# This warms our schema, doing this as part of loading the helpers to avoid +# duplicate loading error when Rails tries autoload the types. +GitlabSchema.graphql_definition diff --git a/spec/uploaders/object_storage_spec.rb b/spec/uploaders/object_storage_spec.rb index a62830c35f1..6bad5d49b1c 100644 --- a/spec/uploaders/object_storage_spec.rb +++ b/spec/uploaders/object_storage_spec.rb @@ -12,7 +12,7 @@ class Implementation < GitlabUploader # user/:id def dynamic_segment - File.join(model.class.to_s.underscore, model.id.to_s) + File.join(model.class.underscore, model.id.to_s) end end |
