diff options
84 files changed, 815 insertions, 1770 deletions
diff --git a/app/assets/javascripts/diffs/components/commit_item.vue b/app/assets/javascripts/diffs/components/commit_item.vue index c970f0f8942..5a45797ed98 100644 --- a/app/assets/javascripts/diffs/components/commit_item.vue +++ b/app/assets/javascripts/diffs/components/commit_item.vue @@ -172,7 +172,7 @@ export default { v-if="commit.description_html" v-safe-html:[$options.safeHtmlConfig]="commitDescription" :class="{ 'js-toggle-content': collapsible, 'd-block': !collapsible }" - class="commit-row-description gl-mb-3 gl-text-body" + class="commit-row-description gl-mb-3 gl-text-body gl-white-space-pre-line" ></pre> </div> </li> diff --git a/app/assets/javascripts/environments/components/enable_review_app_modal.vue b/app/assets/javascripts/environments/components/enable_review_app_modal.vue index 6343fe8702a..420ad3d9c42 100644 --- a/app/assets/javascripts/environments/components/enable_review_app_modal.vue +++ b/app/assets/javascripts/environments/components/enable_review_app_modal.vue @@ -1,18 +1,19 @@ <script> -import { GlLink, GlModal, GlSprintf } from '@gitlab/ui'; +import { GlLink, GlModal, GlSprintf, GlIcon, GlPopover } from '@gitlab/ui'; import { uniqueId } from 'lodash'; import { helpPagePath } from '~/helpers/help_page_helper'; -import { s__ } from '~/locale'; import ModalCopyButton from '~/vue_shared/components/modal_copy_button.vue'; +import { REVIEW_APP_MODAL_I18N as i18n } from '../constants'; export default { components: { GlLink, GlModal, GlSprintf, + GlIcon, + GlPopover, ModalCopyButton, }, - inject: ['defaultBranchName'], model: { prop: 'visible', event: 'change', @@ -28,25 +29,6 @@ export default { default: false, }, }, - instructionText: { - step1: s__( - 'EnableReviewApp|%{stepStart}Step 1%{stepEnd}. Ensure you have Kubernetes set up and have a base domain for your %{linkStart}cluster%{linkEnd}.', - ), - step2: s__('EnableReviewApp|%{stepStart}Step 2%{stepEnd}. Copy the following snippet:'), - step3: s__( - `EnableReviewApp|%{stepStart}Step 3%{stepEnd}. Add it to the project %{linkStart}gitlab-ci.yml%{linkEnd} file.`, - ), - step4: s__( - `EnableReviewApp|%{stepStart}Step 4 (optional)%{stepEnd}. Enable Visual Reviews by following the %{linkStart}setup instructions%{linkEnd}.`, - ), - }, - modalInfo: { - closeText: s__('EnableReviewApp|Close'), - copyToClipboardText: s__('EnableReviewApp|Copy snippet text'), - title: s__('ReviewApp|Enable Review App'), - }, - visualReviewsDocs: helpPagePath('ci/review_apps/index.md', { anchor: 'visual-reviews' }), - connectClusterDocs: helpPagePath('user/clusters/agent/index'), data() { const modalInfoCopyId = uniqueId('enable-review-app-copy-string-'); @@ -57,81 +39,99 @@ export default { return `deploy_review: stage: deploy script: - - echo "Deploy a review app" + - echo "Add script here that deploys the code to your infrastructure" environment: name: review/$CI_COMMIT_REF_NAME url: https://$CI_ENVIRONMENT_SLUG.example.com - only: - - branches - except: - - ${this.defaultBranchName}`; + rules: + - if: $CI_PIPELINE_SOURCE == "merge_request_event"`; + }, + }, + methods: { + commaOrPeriod(index, length) { + return index + 1 === length ? '.' : ','; }, }, + i18n, + configuringReviewAppsPath: helpPagePath('ci/review_apps/index.md', { + anchor: 'configuring-review-apps', + }), + reviewAppsExamplesPath: helpPagePath('ci/review_apps/index.md', { + anchor: 'review-apps-examples', + }), }; </script> <template> <gl-modal :visible="visible" :modal-id="modalId" - :title="$options.modalInfo.title" + :title="$options.i18n.title" static size="lg" - ok-only - ok-variant="light" - :ok-title="$options.modalInfo.closeText" + hide-footer @change="$emit('change', $event)" > + <p>{{ $options.i18n.intro }}</p> <p> - <gl-sprintf :message="$options.instructionText.step1"> - <template #step="{ content }"> - <strong>{{ content }}</strong> - </template> - <template #link="{ content }"> - <gl-link :href="$options.connectClusterDocs" target="_blank">{{ content }}</gl-link> - </template> - </gl-sprintf> + <strong>{{ $options.i18n.instructions.title }}</strong> </p> - <div> - <p> - <gl-sprintf :message="$options.instructionText.step2"> - <template #step="{ content }"> - <strong>{{ content }}</strong> - </template> - </gl-sprintf> - </p> - <div class="gl-display-flex align-items-start"> - <pre :id="modalInfoCopyId" class="gl-w-full" data-testid="enable-review-app-copy-string"> - {{ modalInfoCopyStr }} </pre - > - <modal-copy-button - :title="$options.modalInfo.copyToClipboardText" - :modal-id="modalId" - css-classes="border-0" - :target="`#${modalInfoCopyId}`" - /> - </div> + <div class="gl-mb-6"> + <ol class="gl-px-6"> + <li> + {{ $options.i18n.instructions.step1 }} + <gl-icon + ref="informationIcon" + name="information-o" + class="gl-text-blue-600 gl-hover-cursor-pointer" + /> + <gl-popover + :target="() => $refs.informationIcon.$el" + :title="$options.i18n.staticSitePopover.title" + triggers="hover focus" + > + {{ $options.i18n.staticSitePopover.body }} + </gl-popover> + </li> + <li>{{ $options.i18n.instructions.step2 }}</li> + <li> + {{ $options.i18n.instructions.step3 }} + <ul class="gl-px-4 gl-py-2"> + <li>{{ $options.i18n.instructions.step3a }}</li> + <li> + <gl-sprintf :message="$options.i18n.instructions.step3b"> + <template #code="{ content }" + ><code>{{ content }}</code></template + > + </gl-sprintf> + </li> + <li class="gl-list-style-none"> + <div class="gl-display-flex align-items-start"> + <pre + :id="modalInfoCopyId" + class="gl-w-full" + data-testid="enable-review-app-copy-string" + >{{ modalInfoCopyStr }}</pre + > + <modal-copy-button + :title="$options.i18n.copyToClipboardText" + :modal-id="modalId" + css-classes="border-0" + :target="`#${modalInfoCopyId}`" + /> + </div> + </li> + </ul> + </li> + <li>{{ $options.i18n.instructions.step4 }}</li> + </ol> + <gl-link :href="$options.configuringReviewAppsPath" target="_blank"> + {{ $options.i18n.learnMore }} + <gl-icon name="external-link" /> + </gl-link> + <gl-link :href="$options.reviewAppsExamplesPath" target="_blank" class="gl-ml-6"> + {{ $options.i18n.viewMoreExampleProjects }} + <gl-icon name="external-link" /> + </gl-link> </div> - <p> - <gl-sprintf :message="$options.instructionText.step3"> - <template #step="{ content }"> - <strong>{{ content }}</strong> - </template> - <template #link="{ content }"> - <gl-link :href="`blob/${defaultBranchName}/.gitlab-ci.yml`" target="_blank">{{ - content - }}</gl-link> - </template> - </gl-sprintf> - </p> - <p> - <gl-sprintf :message="$options.instructionText.step4"> - <template #step="{ content }"> - <strong>{{ content }}</strong> - </template> - <template #link="{ content }"> - <gl-link :href="$options.visualReviewsDocs" target="_blank">{{ content }}</gl-link> - </template> - </gl-sprintf> - </p> </gl-modal> </template> diff --git a/app/assets/javascripts/environments/components/environment_external_url.vue b/app/assets/javascripts/environments/components/environment_external_url.vue index b8def676e7d..04a390fbba7 100644 --- a/app/assets/javascripts/environments/components/environment_external_url.vue +++ b/app/assets/javascripts/environments/components/environment_external_url.vue @@ -1,6 +1,8 @@ <script> import { GlTooltipDirective, GlButton } from '@gitlab/ui'; -import { s__ } from '~/locale'; +import ModalCopyButton from '~/vue_shared/components/modal_copy_button.vue'; +import { s__, __ } from '~/locale'; +import { isSafeURL } from '~/lib/utils/url_utility'; /** * Renders the external url link in environments table. @@ -8,6 +10,7 @@ import { s__ } from '~/locale'; export default { components: { GlButton, + ModalCopyButton, }, directives: { GlTooltip: GlTooltipDirective, @@ -21,11 +24,19 @@ export default { i18n: { title: s__('Environments|Open live environment'), open: s__('Environments|Open'), + copy: __('Copy URL'), + copyTitle: s__('Environments|Copy live environment URL'), + }, + computed: { + isSafeUrl() { + return isSafeURL(this.externalUrl); + }, }, }; </script> <template> <gl-button + v-if="isSafeUrl" v-gl-tooltip :title="$options.i18n.title" :aria-label="$options.i18n.title" @@ -37,4 +48,7 @@ export default { > {{ $options.i18n.open }} </gl-button> + <modal-copy-button v-else :title="$options.i18n.copyTitle" :text="externalUrl"> + {{ $options.i18n.copy }} + </modal-copy-button> </template> diff --git a/app/assets/javascripts/environments/components/environments_detail_header.vue b/app/assets/javascripts/environments/components/environments_detail_header.vue index bd67908a6b4..bb2f053b3fc 100644 --- a/app/assets/javascripts/environments/components/environments_detail_header.vue +++ b/app/assets/javascripts/environments/components/environments_detail_header.vue @@ -4,6 +4,8 @@ import csrf from '~/lib/utils/csrf'; import { __, s__ } from '~/locale'; import TimeAgo from '~/vue_shared/components/time_ago_tooltip.vue'; import timeagoMixin from '~/vue_shared/mixins/timeago'; +import ModalCopyButton from '~/vue_shared/components/modal_copy_button.vue'; +import { isSafeURL } from '~/lib/utils/url_utility'; import DeleteEnvironmentModal from './delete_environment_modal.vue'; import StopEnvironmentModal from './stop_environment_modal.vue'; @@ -16,6 +18,7 @@ export default { TimeAgo, DeleteEnvironmentModal, StopEnvironmentModal, + ModalCopyButton, }, directives: { GlModalDirective, @@ -73,6 +76,8 @@ export default { deleteButtonText: s__('Environments|Delete'), externalButtonTitle: s__('Environments|Open live environment'), externalButtonText: __('View deployment'), + copyUrlText: __('Copy URL'), + copyUrlTitle: s__('Environments|Copy live environment URL'), cancelAutoStopButtonTitle: __('Prevent environment from auto-stopping'), }, computed: { @@ -82,6 +87,9 @@ export default { shouldShowExternalUrlButton() { return Boolean(this.environment.externalUrl); }, + isSafeUrl() { + return isSafeURL(this.environment.externalUrl); + }, shouldShowStopButton() { return this.canStopEnvironment && this.environment.isAvailable; }, @@ -123,16 +131,25 @@ export default { :href="terminalPath" icon="terminal" /> - <gl-button - v-if="shouldShowExternalUrlButton" - v-gl-tooltip.hover - data-testid="external-url-button" - :title="$options.i18n.externalButtonTitle" - :href="environment.externalUrl" - icon="external-link" - target="_blank" - >{{ $options.i18n.externalButtonText }}</gl-button - > + <template v-if="shouldShowExternalUrlButton"> + <gl-button + v-if="isSafeUrl" + v-gl-tooltip.hover + data-testid="external-url-button" + :title="$options.i18n.externalButtonTitle" + :href="environment.externalUrl" + icon="external-link" + target="_blank" + >{{ $options.i18n.externalButtonText }}</gl-button + > + <modal-copy-button + v-else + :title="$options.i18n.copyUrlTitle" + :text="environment.externalUrl" + > + {{ $options.i18n.copyUrlText }} + </modal-copy-button> + </template> <gl-button v-if="shouldShowExternalUrlButton" v-gl-tooltip.hover diff --git a/app/assets/javascripts/environments/constants.js b/app/assets/javascripts/environments/constants.js index 942491039d6..c4d02da9d21 100644 --- a/app/assets/javascripts/environments/constants.js +++ b/app/assets/javascripts/environments/constants.js @@ -1,4 +1,4 @@ -import { __ } from '~/locale'; +import { __, s__ } from '~/locale'; // These statuses are based on how the backend defines pod phases here // lib/gitlab/kubernetes/pod.rb @@ -48,3 +48,32 @@ export const ENVIRONMENT_COUNT_BY_SCOPE = { [ENVIRONMENTS_SCOPE.AVAILABLE]: 'availableCount', [ENVIRONMENTS_SCOPE.STOPPED]: 'stoppedCount', }; + +export const REVIEW_APP_MODAL_I18N = { + title: s__('ReviewApp|Enable Review App'), + intro: s__( + 'EnableReviewApp|Review apps are dynamic environments that you can use to provide a live preview of changes made in a feature branch.', + ), + instructions: { + title: s__('EnableReviewApp|To configure a dynamic review app, you must:'), + step1: s__( + 'EnableReviewApp|Have access to infrastructure that can host and deploy the review apps.', + ), + step2: s__('EnableReviewApp|Install and configure a runner to do the deployment.'), + step3: s__('EnableReviewApp|Add a job in your CI/CD configuration that:'), + step3a: s__('EnableReviewApp|Only runs for feature branches or merge requests.'), + step3b: s__( + 'EnableReviewApp|Uses a predefined CI/CD variable like %{codeStart}$(CI_COMMIT_REF_SLUG)%{codeEnd} to dynamically create the review app environments. For example, for a configuration using merge request pipelines:', + ), + step4: s__('EnableReviewApp|Recommended: Set up a job that manually stops the Review Apps.'), + }, + staticSitePopover: { + title: s__('EnableReviewApp|Using a static site?'), + body: s__( + 'EnableReviewApp|Make sure your project has an environment configured with the target URL set to your website URL. If not, create a new one before continuing.', + ), + }, + learnMore: __('Learn more'), + viewMoreExampleProjects: s__('EnableReviewApp|View more example projects'), + copyToClipboardText: s__('EnableReviewApp|Copy snippet'), +}; diff --git a/app/assets/javascripts/import_entities/import_groups/components/import_table.vue b/app/assets/javascripts/import_entities/import_groups/components/import_table.vue index 1be7d40190f..0cdd64b1b98 100644 --- a/app/assets/javascripts/import_entities/import_groups/components/import_table.vue +++ b/app/assets/javascripts/import_entities/import_groups/components/import_table.vue @@ -150,6 +150,10 @@ export default { }, groupsTableData() { + if (!this.availableNamespaces) { + return []; + } + return this.groups.map((group) => { const importTarget = this.getImportTarget(group); const status = this.getStatus(group); @@ -232,6 +236,10 @@ export default { version: this.bulkImportSourceGroups.versionValidation.features.sourceInstanceVersion, }); }, + + pageInfo() { + return this.bulkImportSourceGroups?.pageInfo ?? {}; + }, }, watch: { @@ -503,6 +511,7 @@ export default { permissionsHelpPath: helpPagePath('user/permissions', { anchor: 'group-members-permissions' }), popoverOptions: { title: __('What is listed here?') }, i18n, + LOCAL_STORAGE_KEY: 'gl-bulk-imports-status-page-size-v1', }; </script> @@ -696,14 +705,15 @@ export default { /> </template> </gl-table> - <pagination-bar - v-if="hasGroups" - :page-info="bulkImportSourceGroups.pageInfo" - class="gl-mt-3" - @set-page="setPage" - @set-page-size="setPageSize" - /> </template> </template> + <pagination-bar + v-show="!$apollo.loading && hasGroups" + :page-info="pageInfo" + class="gl-mt-3" + :storage-key="$options.LOCAL_STORAGE_KEY" + @set-page="setPage" + @set-page-size="setPageSize" + /> </div> </template> diff --git a/app/assets/javascripts/repository/components/last_commit.vue b/app/assets/javascripts/repository/components/last_commit.vue index f2f3d7ebeab..05d64077866 100644 --- a/app/assets/javascripts/repository/components/last_commit.vue +++ b/app/assets/javascripts/repository/components/last_commit.vue @@ -169,7 +169,7 @@ export default { v-if="commitDescription" v-safe-html:[$options.safeHtmlConfig]="commitDescription" :class="{ 'd-block': showDescription }" - class="commit-row-description gl-mb-3" + class="commit-row-description gl-mb-3 gl-white-space-pre-line" ></pre> </div> <div class="gl-flex-grow-1"></div> diff --git a/app/assets/javascripts/vue_merge_request_widget/components/deployment/deployment_view_button.vue b/app/assets/javascripts/vue_merge_request_widget/components/deployment/deployment_view_button.vue index 1e363b0f5fb..5efa0e2879e 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/deployment/deployment_view_button.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/deployment/deployment_view_button.vue @@ -7,7 +7,10 @@ import { GlLink, GlSearchBoxByType, } from '@gitlab/ui'; +import { isSafeURL } from '~/lib/utils/url_utility'; +import { s__, __ } from '~/locale'; import autofocusonshow from '~/vue_shared/directives/autofocusonshow'; +import ModalCopyButton from '~/vue_shared/components/modal_copy_button.vue'; import ReviewAppLink from '../review_app_link.vue'; export default { @@ -19,6 +22,7 @@ export default { GlIcon, GlLink, GlSearchBoxByType, + ModalCopyButton, ReviewAppLink, }, directives: { @@ -50,6 +54,13 @@ export default { filteredChanges() { return this.deployment?.changes?.filter((change) => change.path.includes(this.searchTerm)); }, + isSafeUrl() { + return isSafeURL(this.deploymentExternalUrl); + }, + }, + i18n: { + copy: __('Copy URL'), + copyTitle: s__('Environments|Copy live environment URL'), }, }; </script> @@ -57,11 +68,20 @@ export default { <span class="gl-display-inline-flex"> <gl-button-group v-if="shouldRenderDropdown" size="small"> <review-app-link + v-if="isSafeUrl" :display="appButtonText" :link="deploymentExternalUrl" size="small" css-class="deploy-link js-deploy-url inline gl-ml-3" /> + <modal-copy-button + v-else + :title="$options.i18n.copyTitle" + :text="deploymentExternalUrl" + size="small" + > + {{ $options.i18n.copy }} + </modal-copy-button> <gl-dropdown toggle-class="gl-px-2!" size="small" class="js-mr-wigdet-deployment-dropdown"> <template #button-content> <gl-icon @@ -90,12 +110,22 @@ export default { </gl-dropdown-item> </gl-dropdown> </gl-button-group> - <review-app-link - v-else - :display="appButtonText" - :link="deploymentExternalUrl" - size="small" - css-class="js-deploy-url deploy-link btn btn-default btn-sm inline gl-ml-3" - /> + <template v-else> + <review-app-link + v-if="isSafeUrl" + :display="appButtonText" + :link="deploymentExternalUrl" + size="small" + css-class="deploy-link js-deploy-url inline gl-ml-3" + /> + <modal-copy-button + v-else + :title="$options.i18n.copyTitle" + :text="deploymentExternalUrl" + size="small" + > + {{ $options.i18n.copy }} + </modal-copy-button> + </template> </span> </template> diff --git a/app/assets/javascripts/vue_shared/components/modal_copy_button.vue b/app/assets/javascripts/vue_shared/components/modal_copy_button.vue index d4f50e347cb..41c92fdba4f 100644 --- a/app/assets/javascripts/vue_shared/components/modal_copy_button.vue +++ b/app/assets/javascripts/vue_shared/components/modal_copy_button.vue @@ -61,6 +61,11 @@ export default { required: false, default: 'primary', }, + size: { + type: String, + required: false, + default: 'medium', + }, }, computed: { modalDomId() { @@ -103,6 +108,9 @@ export default { :title="title" :aria-label="title" :category="category" + :size="size" icon="copy-to-clipboard" - /> + > + <slot></slot> + </gl-button> </template> diff --git a/app/assets/javascripts/vue_shared/components/pagination_bar/pagination_bar.vue b/app/assets/javascripts/vue_shared/components/pagination_bar/pagination_bar.vue index b4d565991f5..c1246b2bf44 100644 --- a/app/assets/javascripts/vue_shared/components/pagination_bar/pagination_bar.vue +++ b/app/assets/javascripts/vue_shared/components/pagination_bar/pagination_bar.vue @@ -2,6 +2,7 @@ import { GlDropdown, GlDropdownItem, GlIcon, GlSprintf } from '@gitlab/ui'; import { __ } from '~/locale'; import PaginationLinks from '~/vue_shared/components/pagination_links.vue'; +import LocalStorageSync from '~/vue_shared/components/local_storage_sync.vue'; const DEFAULT_PAGE_SIZES = [20, 50, 100]; @@ -12,6 +13,7 @@ export default { GlDropdownItem, GlIcon, GlSprintf, + LocalStorageSync, }, props: { pageInfo: { @@ -23,6 +25,11 @@ export default { type: Array, default: () => DEFAULT_PAGE_SIZES, }, + storageKey: { + required: false, + type: String, + default: null, + }, }, computed: { @@ -66,6 +73,12 @@ export default { <template> <div class="gl-display-flex gl-align-items-center"> + <local-storage-sync + v-if="storageKey" + :storage-key="storageKey" + :value="pageInfo.perPage" + @input="setPageSize" + /> <pagination-links :change="setPage" :page-info="pageInfo" class="gl-m-0" /> <gl-dropdown category="tertiary" class="gl-ml-auto" data-testid="page-size"> <template #button-content> diff --git a/app/assets/javascripts/work_items/components/work_item_labels.vue b/app/assets/javascripts/work_items/components/work_item_labels.vue index 9a36ddc0204..05077862690 100644 --- a/app/assets/javascripts/work_items/components/work_item_labels.vue +++ b/app/assets/javascripts/work_items/components/work_item_labels.vue @@ -7,6 +7,7 @@ import labelSearchQuery from '~/vue_shared/components/sidebar/labels_select_widg import LabelItem from '~/vue_shared/components/sidebar/labels_select_widget/label_item.vue'; import { DEFAULT_DEBOUNCE_AND_THROTTLE_MS } from '~/lib/utils/constants'; import { isScopedLabel } from '~/lib/utils/common_utils'; +import workItemLabelsSubscription from 'ee_else_ce/work_items/graphql/work_item_labels.subscription.graphql'; import workItemQuery from '../graphql/work_item.query.graphql'; import updateWorkItemMutation from '../graphql/update_work_item.mutation.graphql'; @@ -75,6 +76,14 @@ export default { error() { this.$emit('error', i18n.fetchError); }, + subscribeToMore: { + document: workItemLabelsSubscription, + variables() { + return { + issuableId: this.workItemId, + }; + }, + }, }, searchLabels: { query: labelSearchQuery, diff --git a/app/assets/javascripts/work_items/graphql/work_item_labels.subscription.graphql b/app/assets/javascripts/work_items/graphql/work_item_labels.subscription.graphql new file mode 100644 index 00000000000..86d936bf4dd --- /dev/null +++ b/app/assets/javascripts/work_items/graphql/work_item_labels.subscription.graphql @@ -0,0 +1,19 @@ +#import "~/graphql_shared/fragments/label.fragment.graphql" + +subscription workItemLabels($issuableId: IssuableID!) { + issuableLabelsUpdated(issuableId: $issuableId) { + ... on WorkItem { + id + widgets { + ... on WorkItemWidgetLabels { + type + labels { + nodes { + ...Label + } + } + } + } + } + } +} diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 71d9910b4b8..84efb8b0da8 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -16,7 +16,6 @@ class ApplicationController < ActionController::Base include SessionlessAuthentication include SessionsHelper include ConfirmEmailWarning - include Gitlab::Experimentation::ControllerConcern include InitializesCurrentUserMode include Impersonation include Gitlab::Logging::CloudflareHelper diff --git a/app/controllers/oauth/applications_controller.rb b/app/controllers/oauth/applications_controller.rb index ff466fd5fbb..3b78b997da1 100644 --- a/app/controllers/oauth/applications_controller.rb +++ b/app/controllers/oauth/applications_controller.rb @@ -4,7 +4,6 @@ class Oauth::ApplicationsController < Doorkeeper::ApplicationsController include Gitlab::GonHelper include PageLayoutHelper include OauthApplications - include Gitlab::Experimentation::ControllerConcern include InitializesCurrentUserMode # Defined by the `Doorkeeper::ApplicationsController` and is redundant as we call `authenticate_user!` below. Not diff --git a/app/controllers/oauth/authorizations_controller.rb b/app/controllers/oauth/authorizations_controller.rb index 2e9fbb1d0d9..bf8b61db2e5 100644 --- a/app/controllers/oauth/authorizations_controller.rb +++ b/app/controllers/oauth/authorizations_controller.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true class Oauth::AuthorizationsController < Doorkeeper::AuthorizationsController - include Gitlab::Experimentation::ControllerConcern include InitializesCurrentUserMode include Gitlab::Utils::StrongMemoize diff --git a/app/controllers/projects/merge_requests/diffs_controller.rb b/app/controllers/projects/merge_requests/diffs_controller.rb index a68c2ffa06d..418e7233e21 100644 --- a/app/controllers/projects/merge_requests/diffs_controller.rb +++ b/app/controllers/projects/merge_requests/diffs_controller.rb @@ -4,6 +4,7 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic include DiffHelper include RendersNotes include Gitlab::Cache::Helpers + include Gitlab::Tracking::Helpers before_action :commit before_action :define_diff_vars diff --git a/app/models/bulk_imports/export_status.rb b/app/models/bulk_imports/export_status.rb index d9c4fd722eb..cbd7b189007 100644 --- a/app/models/bulk_imports/export_status.rb +++ b/app/models/bulk_imports/export_status.rb @@ -30,13 +30,17 @@ module BulkImports private - attr_reader :client, :entity, :relation + attr_reader :client, :entity, :relation, :pipeline_tracker def export_status strong_memoize(:export_status) do fetch_export_status&.find { |item| item['relation'] == relation } + rescue BulkImports::NetworkError => e + raise BulkImports::RetryPipelineError.new(e.message, 2.seconds) if e.retriable?(pipeline_tracker) + + default_error_response(e.message) rescue StandardError => e - { 'status' => Export::FAILED, 'error' => e.message } + default_error_response(e.message) end end @@ -47,5 +51,9 @@ module BulkImports def status_endpoint File.join(entity.export_relations_url_path, 'status') end + + def default_error_response(message) + { 'status' => Export::FAILED, 'error' => message } + end end end diff --git a/app/models/bulk_imports/tracker.rb b/app/models/bulk_imports/tracker.rb index fa38b7617d2..357f4629078 100644 --- a/app/models/bulk_imports/tracker.rb +++ b/app/models/bulk_imports/tracker.rb @@ -60,6 +60,8 @@ class BulkImports::Tracker < ApplicationRecord event :retry do transition started: :enqueued + # To avoid errors when retrying a pipeline in case of network errors + transition enqueued: :enqueued end event :enqueue do diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index e05a32ccb5b..b8511536e32 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -417,18 +417,10 @@ module Ci pipeline.manual_actions.reject { |action| action.name == self.name } end - def environment_manual_actions - pipeline.manual_actions.filter { |action| action.expanded_environment_name == self.expanded_environment_name } - end - def other_scheduled_actions pipeline.scheduled_actions.reject { |action| action.name == self.name } end - def environment_scheduled_actions - pipeline.scheduled_actions.filter { |action| action.expanded_environment_name == self.expanded_environment_name } - end - def pages_generator? Gitlab.config.pages.enabled && self.name == 'pages' diff --git a/app/models/deployment.rb b/app/models/deployment.rb index 4d10499f48d..20841bc14cd 100644 --- a/app/models/deployment.rb +++ b/app/models/deployment.rb @@ -283,27 +283,11 @@ class Deployment < ApplicationRecord end def manual_actions - environment_manual_actions - end - - def other_manual_actions - @other_manual_actions ||= deployable.try(:other_manual_actions) - end - - def environment_manual_actions - @environment_manual_actions ||= deployable.try(:environment_manual_actions) + @manual_actions ||= deployable.try(:other_manual_actions) end def scheduled_actions - environment_scheduled_actions - end - - def environment_scheduled_actions - @environment_scheduled_actions ||= deployable.try(:environment_scheduled_actions) - end - - def other_scheduled_actions - @other_scheduled_actions ||= deployable.try(:other_scheduled_actions) + @scheduled_actions ||= deployable.try(:other_scheduled_actions) end def playable_build diff --git a/app/models/environment.rb b/app/models/environment.rb index e8588c8d022..a601b3a88a8 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -71,7 +71,7 @@ class Environment < ApplicationRecord validate :safe_external_url validate :merge_request_not_changed - delegate :manual_actions, :other_manual_actions, to: :last_deployment, allow_nil: true + delegate :manual_actions, to: :last_deployment, allow_nil: true delegate :auto_rollback_enabled?, to: :project scope :available, -> { with_state(:available) } @@ -332,9 +332,9 @@ class Environment < ApplicationRecord end def actions_for(environment) - return [] unless other_manual_actions + return [] unless manual_actions - other_manual_actions.select do |action| + manual_actions.select do |action| action.expanded_environment_name == environment end end diff --git a/app/models/event.rb b/app/models/event.rb index 74c3367f4b2..4c1793d3f13 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -10,8 +10,6 @@ class Event < ApplicationRecord include UsageStatistics include ShaAttribute - default_scope { Feature.enabled?(:skip_default_scope_for_events) ? self : reorder(nil) } # rubocop:disable Cop/DefaultScope - ACTIONS = HashWithIndifferentAccess.new( created: 1, updated: 2, diff --git a/app/services/packages/debian/create_package_file_service.rb b/app/services/packages/debian/create_package_file_service.rb index 53275fdc9bb..19e68183ea2 100644 --- a/app/services/packages/debian/create_package_file_service.rb +++ b/app/services/packages/debian/create_package_file_service.rb @@ -5,18 +5,20 @@ module Packages class CreatePackageFileService include ::Packages::FIPS - def initialize(package, params) + def initialize(package:, current_user:, params: {}) @package = package + @current_user = current_user @params = params end def execute raise DisabledError, 'Debian registry is not FIPS compliant' if Gitlab::FIPS.enabled? raise ArgumentError, "Invalid package" unless package.present? + raise ArgumentError, "Invalid user" unless current_user.present? # Debian package file are first uploaded to incoming with empty metadata, # and are moved later by Packages::Debian::ProcessChangesService - package.package_files.create!( + package_file = package.package_files.create!( file: params[:file], size: params[:file]&.size, file_name: params[:file_name], @@ -29,11 +31,17 @@ module Packages fields: nil } ) + + if params[:file_name].end_with? '.changes' + ::Packages::Debian::ProcessChangesWorker.perform_async(package_file.id, current_user.id) + end + + package_file end private - attr_reader :package, :params + attr_reader :package, :current_user, :params end end end diff --git a/app/views/projects/commits/_commit.html.haml b/app/views/projects/commits/_commit.html.haml index 511d017f023..bf6b628dd36 100644 --- a/app/views/projects/commits/_commit.html.haml +++ b/app/views/projects/commits/_commit.html.haml @@ -51,7 +51,7 @@ = render_if_exists 'projects/commits/project_namespace', show_project_name: show_project_name, project: project - if commit.description? - %pre{ class: ["commit-row-description gl-mb-3", (collapsible ? "js-toggle-content" : "d-block")] } + %pre{ class: ["commit-row-description gl-mb-3 gl-white-space-pre-line", (collapsible ? "js-toggle-content" : "d-block")] } = preserve(markdown_field(commit, :description)) .commit-actions.flex-row diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index 0624197183c..a0f6da57f9e 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -2397,15 +2397,6 @@ :weight: 1 :idempotent: false :tags: [] -- :name: experiments_record_conversion_event - :worker_name: Experiments::RecordConversionEventWorker - :feature_category: :users - :has_external_dependencies: false - :urgency: :low - :resource_boundary: :unknown - :weight: 1 - :idempotent: true - :tags: [] - :name: export_csv :worker_name: ExportCsvWorker :feature_category: :team_planning diff --git a/app/workers/bulk_imports/export_request_worker.rb b/app/workers/bulk_imports/export_request_worker.rb index d2a7eee1064..a57071ddcf1 100644 --- a/app/workers/bulk_imports/export_request_worker.rb +++ b/app/workers/bulk_imports/export_request_worker.rb @@ -19,9 +19,13 @@ module BulkImports BulkImports::EntityWorker.perform_async(entity_id) rescue BulkImports::NetworkError => e - log_export_failure(e, entity) + if e.retriable?(entity) + retry_request(e, entity) + else + log_export_failure(e, entity) - entity.fail_op! + entity.fail_op! + end end private @@ -101,5 +105,20 @@ module BulkImports BulkImports::Projects::Graphql::GetProjectQuery.new(context: nil) end end + + def retry_request(exception, entity) + Gitlab::Import::Logger.error( + structured_payload( + log_attributes(exception, entity).merge( + message: 'Retrying export request', + bulk_import_id: entity.bulk_import_id, + bulk_import_entity_type: entity.source_type, + importer: 'gitlab_migration' + ) + ) + ) + + self.class.perform_in(2.seconds, entity.id) + end end end diff --git a/app/workers/experiments/record_conversion_event_worker.rb b/app/workers/experiments/record_conversion_event_worker.rb deleted file mode 100644 index 6487f030628..00000000000 --- a/app/workers/experiments/record_conversion_event_worker.rb +++ /dev/null @@ -1,22 +0,0 @@ -# frozen_string_literal: true - -module Experiments - class RecordConversionEventWorker - include ApplicationWorker - - data_consistency :always - - sidekiq_options retry: 3 - - feature_category :users - urgency :low - - idempotent! - - def perform(experiment, user_id) - return unless Gitlab::Experimentation.active?(experiment) - - ::Experiment.record_conversion_event(experiment, user_id) - end - end -end diff --git a/config/feature_flags/development/ci_increase_includes_to_250.yml b/config/feature_flags/development/ci_increase_includes_to_250.yml deleted file mode 100644 index b6291ab0cd3..00000000000 --- a/config/feature_flags/development/ci_increase_includes_to_250.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: ci_increase_includes_to_250 -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/64934 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/344449 -milestone: '15.2' -type: development -group: group::pipeline authoring -default_enabled: false diff --git a/config/feature_flags/development/skip_default_scope_for_events.yml b/config/feature_flags/development/skip_default_scope_for_events.yml deleted file mode 100644 index 72957727fd3..00000000000 --- a/config/feature_flags/development/skip_default_scope_for_events.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: skip_default_scope_for_events -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/96874 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/372464 -milestone: '15.4' -type: development -group: group::optimize -default_enabled: true diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index 9fd69458111..1f904338249 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -177,8 +177,6 @@ - 1 - - error_tracking_issue_link - 1 -- - experiments_record_conversion_event - - 1 - - export_csv - 1 - - external_service_reactive_caching diff --git a/doc/administration/monitoring/prometheus/gitlab_metrics.md b/doc/administration/monitoring/prometheus/gitlab_metrics.md index ba33bcc47f5..e8bdacb0e14 100644 --- a/doc/administration/monitoring/prometheus/gitlab_metrics.md +++ b/doc/administration/monitoring/prometheus/gitlab_metrics.md @@ -11,7 +11,7 @@ To enable the GitLab Prometheus metrics: 1. Log in to GitLab as a user with administrator access. 1. On the top bar, select **Main menu > Admin**. 1. On the left sidebar, select **Settings > Metrics and profiling**. -1. Find the **Metrics - Prometheus** section, and select **Add link to Prometheus**. +1. Find the **Metrics - Prometheus** section, and select **Enable GitLab Prometheus metrics endpoint**. 1. [Restart GitLab](../../restart_gitlab.md#omnibus-gitlab-restart) for the changes to take effect. For installations from source you must configure it yourself. diff --git a/doc/administration/reference_architectures/3k_users.md b/doc/administration/reference_architectures/3k_users.md index e3147e262bf..801eeb99720 100644 --- a/doc/administration/reference_architectures/3k_users.md +++ b/doc/administration/reference_architectures/3k_users.md @@ -2203,12 +2203,12 @@ cluster alongside your instance, read how to ## Supported modifications for lower user counts (HA) -The 3k GitLab reference architecture is the smallest we recommend that achieves High Availability (HA). -However, for environments that need to serve less users but maintain HA, there's several +The 3,000 user GitLab reference architecture is the smallest we recommend that achieves High Availability (HA). +However, for environments that need to serve fewer users but maintain HA, there are several supported modifications you can make to this architecture to reduce complexity and cost. -It should be noted that to achieve HA with GitLab, this architecture's makeup is ultimately what is -required. Each component has various considerations and rules to follow and this architecture +It should be noted that to achieve HA with GitLab, the 3,000 user architecture's makeup is ultimately what is +required. Each component has various considerations and rules to follow, and the 3,000 user architecture meets all of these. Smaller versions of this architecture will be fundamentally the same, but with smaller performance requirements, several modifications can be considered as follows: diff --git a/doc/administration/sidekiq/extra_sidekiq_routing.md b/doc/administration/sidekiq/extra_sidekiq_routing.md index f13e2a75a70..56c51beb758 100644 --- a/doc/administration/sidekiq/extra_sidekiq_routing.md +++ b/doc/administration/sidekiq/extra_sidekiq_routing.md @@ -161,20 +161,3 @@ After the Sidekiq routing rules are changed, administrators must take care with the migration to avoid losing jobs entirely, especially in a system with long queues of jobs. The migration can be done by following the migration steps mentioned in [Sidekiq job migration](sidekiq_job_migration.md) - -### Workers that cannot be migrated - -Some workers cannot share a queue with other workers - typically because -they check the size of their own queue - and so must be excluded from -this process. We recommend excluding these from any further worker -routing by adding a rule to keep them in their own queue, for example: - -```ruby -sidekiq['routing_rules'] = [ - ['tags=needs_own_queue', nil], - # ... -] -``` - -These queues must also be included in at least one -[Sidekiq queue group](extra_sidekiq_processes.md#start-multiple-processes). diff --git a/doc/architecture/blueprints/ci_data_decay/index.md b/doc/architecture/blueprints/ci_data_decay/index.md index bf6f2b8e774..221c2364f79 100644 --- a/doc/architecture/blueprints/ci_data_decay/index.md +++ b/doc/architecture/blueprints/ci_data_decay/index.md @@ -102,9 +102,9 @@ Epic: [Reduce the rate of builds metadata table growth](https://gitlab.com/group ### Partition CI/CD pipelines database tables -After we move CI/CD metadata to a different store, or reduce the rate of +Even if we move CI/CD metadata to a different store, or reduce the rate of metadata growth in a different way, the problem of having billions of rows -describing pipelines, builds and artifacts, remains. We still need to keep +describing pipelines, builds and artifacts, remains. We still may need to keep reference to the metadata we might store in object storage and we still do need to be able to retrieve this information reliably in bulk (or search through it). @@ -123,12 +123,12 @@ multiple smaller ones, using PostgreSQL partitioning features. There are a few approaches we can take to partition CI/CD data. A promising one is using list-based partitioning where a partition number is assigned a pipeline, and gets propagated to all resources that are related to this -pipeline. We assign the partition number based on when the pipeline was created -or when we observed the last processing activity in it. This is very flexible -because we can extend this partitioning strategy at will; for example with this -strategy we can assign an arbitrary partition number based on multiple -partitioning keys, combining time-decay-based partitioning with tenant-based -partitioning on the application level. +pipeline. We will assign a partition number using a +[uniform logical partition ID](pipeline_partitioning.md#why-do-we-want-to-use-explicit-logical-partition-ids) +This is very flexible because we can extend this partitioning strategy at will; +for example with this strategy we can assign an arbitrary partition number +based on multiple partitioning keys, combining time-decay-based partitioning +with tenant-based partitioning on the application level if desired. Partitioning rarely accessed data should also follow the policy defined for builds archival, to make it consistent and reliable. diff --git a/doc/architecture/blueprints/ci_data_decay/pipeline_partitioning.md b/doc/architecture/blueprints/ci_data_decay/pipeline_partitioning.md index 5140166be44..52642e04155 100644 --- a/doc/architecture/blueprints/ci_data_decay/pipeline_partitioning.md +++ b/doc/architecture/blueprints/ci_data_decay/pipeline_partitioning.md @@ -87,6 +87,7 @@ incidents, over the last couple of months, for example: - S2: 2022-04-12 [Transactions detected that have been running for more than 10m](https://gitlab.com/gitlab-com/gl-infra/production/-/issues/6821) - S2: 2022-04-06 [Database contention plausibly caused by excessive `ci_builds` reads](https://gitlab.com/gitlab-com/gl-infra/production/-/issues/6773) - S2: 2022-03-18 [Unable to remove a foreign key on `ci_builds`](https://gitlab.com/gitlab-com/gl-infra/production/-/issues/6642) +- S2: 2022-10-10 [The queuing_queries_duration SLI apdex violating SLO](https://gitlab.com/gitlab-com/gl-infra/production/-/issues/7852#note_1130123525) We have approximately 50 `ci_*` prefixed database tables, and some of them would benefit from partitioning. @@ -278,6 +279,14 @@ also find information about which logical partitions are "active" or "archived", which will help us to implement a time-decay pattern using database declarative partitioning. +Doing that will also allow us to use a Unified Resource Identifier for +partitioned resources, that will contain a pointer to a pipeline ID, we could +then use to efficiently lookup a partition the resource is stored in. We could +use an ID like `1e240-5ba0` for pipeline `123456`, build `23456`. If we decide +to update the primary identifier of a partitioned resource (today it is just a +big integer) it is important to design a system that is resilient to migrating +data between partitions, to avoid changing idenfiers when rebalancing happens. + `ci_partitions` table will store information about a partition identifier, pipeline ids range it is valid for and whether the partitions have been archived or not. Additional columns with timestamps may be helpful too. @@ -698,8 +707,8 @@ Authors: Recommenders: -| Role | Who | -|------------------------|-----------------| -| Distingiushed Engineer | Kamil Trzciński | +| Role | Who | +|-------------------------------|-----------------| +| Senior Distingiushed Engineer | Kamil Trzciński | <!-- vale gitlab.Spelling = YES --> diff --git a/doc/ci/runners/runners_scope.md b/doc/ci/runners/runners_scope.md index 8f9ab29ff27..4be4ed33feb 100644 --- a/doc/ci/runners/runners_scope.md +++ b/doc/ci/runners/runners_scope.md @@ -194,7 +194,8 @@ From this page, you can edit, pause, and remove runners from the group, its subg #### Filter group runners to show only inherited -> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/337838/) in GitLab 15.5. +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/337838/) in GitLab 15.5. +> - [Generally available](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/101099) in GitLab 15.5. Feature flag `runners_finder_all_available` removed. You can choose to show all runners in the list, or show only those that are inherited from the instance or other groups. diff --git a/doc/subscriptions/index.md b/doc/subscriptions/index.md index 93a0b46f206..78b9a586c0b 100644 --- a/doc/subscriptions/index.md +++ b/doc/subscriptions/index.md @@ -142,7 +142,7 @@ To change the GitLab.com account linked to your Customers Portal account: 1. In a separate browser tab, go to [GitLab SaaS](https://gitlab.com) and ensure you are not logged in. 1. On the Customers Portal page, select **My account > Account details**. -1. Under **Your GitLab.com account**, select **Change linked account**. +1. Under **Your GitLab.com account**, select **Change linked account**. If the account is not yet linked, select **Link my GitLab.com account**. 1. Log in to the [GitLab SaaS](https://gitlab.com) account you want to link to the Customers Portal account. diff --git a/doc/user/admin_area/appearance.md b/doc/user/admin_area/appearance.md index e3721105501..5513fa3585d 100644 --- a/doc/user/admin_area/appearance.md +++ b/doc/user/admin_area/appearance.md @@ -30,7 +30,7 @@ supported by many email clients. ## Favicon By default, the favicon (used by the browser as the tab icon, as well as the CI status icon) -uses the GitLab logo, but this can be customized with any icon desired. It must be a +uses the GitLab logo. This can be customized with any icon desired. It must be a 32x32 `.png` or `.ico` image. After you select and upload an icon, select **Update appearance settings** at the bottom @@ -47,7 +47,7 @@ Limited [Markdown](../markdown.md) is supported, such as bold, italics, and link example. Other Markdown features, including lists, images, and quotes are not supported as the header and footer messages can only be a single line. -If desired, you can select **Enable header and footer in emails** to have the text of +You can select **Enable header and footer in emails** to have the text of the header and footer added to all emails sent by the GitLab instance. After you add a message, select **Update appearance settings** at the bottom of the page @@ -71,7 +71,7 @@ You can add also add a [customized help message](settings/help_page.md) below th ## New project pages -You can add a new project guidelines message to the **New project page** within GitLab. +You can add a new project guidelines message to the **New project page** in GitLab. You can make full use of [Markdown](../markdown.md) in the description: The message is displayed below the **New Project** message, on the left side diff --git a/doc/user/group/manage.md b/doc/user/group/manage.md index d6c779e3d38..8c71169d6ba 100644 --- a/doc/user/group/manage.md +++ b/doc/user/group/manage.md @@ -515,6 +515,34 @@ include: # Execute individual project's configuration (if project contains .git ref: '$CI_COMMIT_REF_NAME' # Must be defined or MR pipelines always use the use default branch ``` +##### CF pipelines in Merge Requests originating in project forks + +When an MR originates in a fork, the branch to be merged usually only exists in the fork. +When creating such an MR against a project with CF pipelines, the above snippet will fail with a +`Project <project-name> reference <branch-name> does not exist!` error message. +This is because in the context of the target project, `$CI_COMMIT_REF_NAME` evaluates to a non-existing branch name. + +To get the correct context, use `$CI_MERGE_REQUEST_SOURCE_PROJECT_PATH` instead of `$CI_PROJECT_PATH`. +This variable is only availabe in +[merge request pipelines](../../ci/pipelines/merge_request_pipelines.md). + +For example, for a configuration that supports both branch pipelines, as well as merge request pipelines originating in project forks, +you need to [combine both `include` directives with `rules:if`](../../ci/yaml/includes.md#use-rules-with-include): + +```yaml +include: # Execute individual project's configuration (if project contains .gitlab-ci.yml) + - project: '$CI_MERGE_REQUEST_SOURCE_PROJECT_PATH' + file: '$CI_CONFIG_PATH' + ref: '$CI_COMMIT_REF_NAME' + rules: + - if: $CI_PIPELINE_SOURCE == 'merge_request_event' + - project: '$CI_PROJECT_PATH' + file: '$CI_CONFIG_PATH' + ref: '$CI_COMMIT_REF_NAME' + rules: + - if: $CI_PIPELINE_SOURCE != 'merge_request_event' +``` + ### Ensure compliance jobs are always run Compliance pipelines [use GitLab CI/CD](../../ci/index.md) to give you an incredible amount of flexibility diff --git a/lib/api/debian_project_packages.rb b/lib/api/debian_project_packages.rb index 9dedc4390f7..03f0f97b805 100644 --- a/lib/api/debian_project_packages.rb +++ b/lib/api/debian_project_packages.rb @@ -81,11 +81,7 @@ module API package = ::Packages::Debian::FindOrCreateIncomingService.new(authorized_user_project, current_user).execute - package_file = ::Packages::Debian::CreatePackageFileService.new(package, file_params).execute - - if params['file_name'].end_with? '.changes' - ::Packages::Debian::ProcessChangesWorker.perform_async(package_file.id, current_user.id) # rubocop:disable CodeReuse/Worker - end + ::Packages::Debian::CreatePackageFileService.new(package: package, current_user: current_user, params: file_params).execute created! rescue ObjectStorage::RemoteStoreError => e diff --git a/lib/bulk_imports/network_error.rb b/lib/bulk_imports/network_error.rb index 335d992d5a7..fda4bb74a30 100644 --- a/lib/bulk_imports/network_error.rb +++ b/lib/bulk_imports/network_error.rb @@ -2,7 +2,8 @@ module BulkImports class NetworkError < Error - COUNTER_KEY = 'bulk_imports/%{entity_id}/%{stage}/%{tracker_id}/network_error/%{error}' + TRACKER_COUNTER_KEY = 'bulk_imports/%{entity_id}/%{stage}/%{tracker_id}/network_error/%{error}' + ENTITY_COUNTER_KEY = 'bulk_imports/%{entity_id}/network_error/%{error}' RETRIABLE_EXCEPTIONS = Gitlab::HTTP::HTTP_TIMEOUT_ERRORS + [ EOFError, SocketError, OpenSSL::SSL::SSLError, OpenSSL::OpenSSLError, @@ -24,9 +25,9 @@ module BulkImports @response = response end - def retriable?(tracker) + def retriable?(object) if retriable_exception? || retriable_http_code? - increment(tracker) <= MAX_RETRIABLE_COUNT + increment(object) <= MAX_RETRIABLE_COUNT else false end @@ -50,15 +51,27 @@ module BulkImports RETRIABLE_HTTP_CODES.include?(response&.code) end - def increment(tracker) - key = COUNTER_KEY % { + def increment(object) + key = object.is_a?(BulkImports::Entity) ? entity_cache_key(object) : tracker_cache_key(object) + + Gitlab::Cache::Import::Caching.increment(key) + end + + def tracker_cache_key(tracker) + TRACKER_COUNTER_KEY % { stage: tracker.stage, tracker_id: tracker.id, entity_id: tracker.entity.id, error: cause.class.name } + end - Gitlab::Cache::Import::Caching.increment(key) + def entity_cache_key(entity) + ENTITY_COUNTER_KEY % { + import_id: entity.bulk_import_id, + entity_id: entity.id, + error: cause.class.name + } end end end diff --git a/lib/feature/shared.rb b/lib/feature/shared.rb index edfc39aea0c..6af24451322 100644 --- a/lib/feature/shared.rb +++ b/lib/feature/shared.rb @@ -53,8 +53,6 @@ module Feature default_enabled: false, example: <<-EOS experiment(:my_experiment, project: project, actor: current_user) { ...variant code... } - # or - Gitlab::Experimentation.in_experiment_group?(:my_experiment, subject: current_user) EOS } }.freeze diff --git a/lib/gitlab/ci/config/external/context.rb b/lib/gitlab/ci/config/external/context.rb index ec628399785..138e79db331 100644 --- a/lib/gitlab/ci/config/external/context.rb +++ b/lib/gitlab/ci/config/external/context.rb @@ -10,7 +10,6 @@ module Gitlab TimeoutError = Class.new(StandardError) MAX_INCLUDES = 100 - TRIAL_MAX_INCLUDES = 250 include ::Gitlab::Utils::StrongMemoize @@ -31,7 +30,7 @@ module Gitlab @expandset = Set.new @execution_deadline = 0 @logger = logger || Gitlab::Ci::Pipeline::Logger.new(project: project) - @max_includes = Feature.enabled?(:ci_increase_includes_to_250, project) ? TRIAL_MAX_INCLUDES : MAX_INCLUDES + @max_includes = MAX_INCLUDES yield self if block_given? end diff --git a/lib/gitlab/ci/pipeline/logger.rb b/lib/gitlab/ci/pipeline/logger.rb index 44d905faced..4b7cbae5004 100644 --- a/lib/gitlab/ci/pipeline/logger.rb +++ b/lib/gitlab/ci/pipeline/logger.rb @@ -86,6 +86,7 @@ module Gitlab 'count' => values.size, 'min' => values.min, 'max' => values.max, + 'sum' => values.sum, 'avg' => values.sum / values.size } end.compact diff --git a/lib/gitlab/experimentation.rb b/lib/gitlab/experimentation.rb deleted file mode 100644 index 142d0e55593..00000000000 --- a/lib/gitlab/experimentation.rb +++ /dev/null @@ -1,110 +0,0 @@ -# frozen_string_literal: true - -# == Experimentation -# -# Utility module for A/B testing experimental features. Define your experiments in the `EXPERIMENTS` constant. -# Experiment options: -# - tracking_category (optional, used to set the category when tracking an experiment event) -# - rollout_strategy: default is `:cookie` based rollout. We may also set it to `:user` based rollout -# -# The experiment is controlled by a Feature Flag (https://docs.gitlab.com/ee/development/feature_flags/controls.html), -# which is named "#{experiment_key}_experiment_percentage" and *must* be set with a percentage and not be used for other purposes. -# -# To enable the experiment for 10% of the time: -# -# chatops: `/chatops run feature set experiment_key_experiment_percentage 10 --random` -# console: `Feature.enable_percentage_of_time(:experiment_key_experiment_percentage, 10)` -# -# To disable the experiment: -# -# chatops: `/chatops run feature delete experiment_key_experiment_percentage` -# console: `Feature.remove(:experiment_key_experiment_percentage)` -# -# To check the current rollout percentage: -# -# chatops: `/chatops run feature get experiment_key_experiment_percentage` -# console: `Feature.get(:experiment_key_experiment_percentage).percentage_of_time_value` -# - -# TODO: see https://gitlab.com/gitlab-org/gitlab/-/issues/217490 -module Gitlab - module Experimentation - EXPERIMENTS = { - }.freeze - - class << self - def get_experiment(experiment_key) - return unless EXPERIMENTS.key?(experiment_key) - - ::Gitlab::Experimentation::Experiment.new(experiment_key, **EXPERIMENTS[experiment_key]) - end - - def active?(experiment_key) - experiment = get_experiment(experiment_key) - return false unless experiment - - experiment.active? - end - - def in_experiment_group?(experiment_key, subject:) - return false if subject.blank? - return false unless active?(experiment_key) - - log_invalid_rollout(experiment_key, subject) - - experiment = get_experiment(experiment_key) - return false unless experiment - - experiment.enabled_for_index?(index_for_subject(experiment, subject)) - end - - def rollout_strategy(experiment_key) - experiment = get_experiment(experiment_key) - return unless experiment - - experiment.rollout_strategy - end - - def log_invalid_rollout(experiment_key, subject) - return if valid_subject_for_rollout_strategy?(experiment_key, subject) - - logger = Gitlab::ExperimentationLogger.build - logger.warn message: 'Subject must conform to the rollout strategy', - experiment_key: experiment_key, - subject: subject.class.to_s, - rollout_strategy: rollout_strategy(experiment_key) - end - - def valid_subject_for_rollout_strategy?(experiment_key, subject) - case rollout_strategy(experiment_key) - when :user - subject.is_a?(User) - when :group - subject.is_a?(Group) - when :cookie - subject.nil? || subject.is_a?(String) - else - false - end - end - - private - - def index_for_subject(experiment, subject) - index = Zlib.crc32("#{experiment.key}#{subject_id(subject)}") - - index % 100 - end - - def subject_id(subject) - if subject.respond_to?(:to_global_id) - subject.to_global_id.to_s - elsif subject.respond_to?(:to_s) - subject.to_s - else - raise ArgumentError, 'Subject must respond to `to_global_id` or `to_s`' - end - end - end - end -end diff --git a/lib/gitlab/experimentation/controller_concern.rb b/lib/gitlab/experimentation/controller_concern.rb deleted file mode 100644 index b09d67b8d5f..00000000000 --- a/lib/gitlab/experimentation/controller_concern.rb +++ /dev/null @@ -1,156 +0,0 @@ -# frozen_string_literal: true - -require 'zlib' - -# Controller concern that checks if an `experimentation_subject_id cookie` is present and sets it if absent. -# Used for A/B testing of experimental features. Exposes the `experiment_enabled?(experiment_name, subject: nil)` method -# to controllers and views. It returns true when the experiment is enabled and the user is selected as part -# of the experimental group. -# -module Gitlab - module Experimentation - module ControllerConcern - include ::Gitlab::Experimentation::GroupTypes - include Gitlab::Tracking::Helpers - extend ActiveSupport::Concern - - included do - before_action :set_experimentation_subject_id_cookie, unless: :dnt_enabled? - helper_method :experiment_enabled?, :experiment_tracking_category_and_group, :record_experiment_group - end - - def set_experimentation_subject_id_cookie - if Gitlab.com? - return if cookies[:experimentation_subject_id].present? - - cookies.permanent.signed[:experimentation_subject_id] = { - value: SecureRandom.uuid, - secure: ::Gitlab.config.gitlab.https, - httponly: true - } - else - # We set the cookie before, although experiments are not conducted on self managed instances. - cookies.delete(:experimentation_subject_id) - end - end - - def push_frontend_experiment(experiment_key, subject: nil) - var_name = experiment_key.to_s.camelize(:lower) - - enabled = experiment_enabled?(experiment_key, subject: subject) - - gon.push({ experiments: { var_name => enabled } }, true) - end - - def experiment_enabled?(experiment_key, subject: nil) - return true if forced_enabled?(experiment_key) - return false if dnt_enabled? - - Experimentation.log_invalid_rollout(experiment_key, subject) - - subject ||= experimentation_subject_id - - Experimentation.in_experiment_group?(experiment_key, subject: subject) - end - - def track_experiment_event(experiment_key, action, value = nil, subject: nil) - return if dnt_enabled? - - track_experiment_event_for(experiment_key, action, value, subject: subject) do |tracking_data| - ::Gitlab::Tracking.event(tracking_data.delete(:category), tracking_data.delete(:action), **tracking_data.merge!(user: current_user)) - end - end - - def frontend_experimentation_tracking_data(experiment_key, action, value = nil, subject: nil) - return if dnt_enabled? - - track_experiment_event_for(experiment_key, action, value, subject: subject) do |tracking_data| - gon.push(tracking_data: tracking_data) - end - end - - def record_experiment_user(experiment_key, context = {}) - return if dnt_enabled? - return unless Experimentation.active?(experiment_key) && current_user - - subject = Experimentation.rollout_strategy(experiment_key) == :cookie ? nil : current_user - - ::Experiment.add_user(experiment_key, tracking_group(experiment_key, nil, subject: subject), current_user, context) - end - - def record_experiment_group(experiment_key, group) - return if dnt_enabled? - return unless Experimentation.active?(experiment_key) && group - - variant_subject = Experimentation.rollout_strategy(experiment_key) == :cookie ? nil : group - variant = tracking_group(experiment_key, nil, subject: variant_subject) - - ::Experiment.add_group(experiment_key, group: group, variant: variant) - end - - def record_experiment_conversion_event(experiment_key, context = {}) - return if dnt_enabled? - return unless current_user - return unless Experimentation.active?(experiment_key) - - ::Experiment.record_conversion_event(experiment_key, current_user, context) - end - - def experiment_tracking_category_and_group(experiment_key, subject: nil) - "#{tracking_category(experiment_key)}:#{tracking_group(experiment_key, '_group', subject: subject)}" - end - - private - - def experimentation_subject_id - cookies.signed[:experimentation_subject_id] - end - - def track_experiment_event_for(experiment_key, action, value, subject: nil) - return unless Experimentation.active?(experiment_key) - - yield experimentation_tracking_data(experiment_key, action, value, subject: subject) - end - - def experimentation_tracking_data(experiment_key, action, value, subject: nil) - { - category: tracking_category(experiment_key), - action: action, - property: tracking_group(experiment_key, "_group", subject: subject), - label: tracking_label(subject), - value: value - }.compact - end - - def tracking_category(experiment_key) - Experimentation.get_experiment(experiment_key).tracking_category - end - - def tracking_group(experiment_key, suffix = nil, subject: nil) - return unless Experimentation.active?(experiment_key) - - subject ||= experimentation_subject_id - group = experiment_enabled?(experiment_key, subject: subject) ? GROUP_EXPERIMENTAL : GROUP_CONTROL - - suffix ? "#{group}#{suffix}" : group - end - - def forced_enabled?(experiment_key) - return true if params.has_key?(:force_experiment) && params[:force_experiment] == experiment_key.to_s - return false if cookies[:force_experiment].blank? - - cookies[:force_experiment].to_s.split(',').any? { |experiment| experiment.strip == experiment_key.to_s } - end - - def tracking_label(subject = nil) - return experimentation_subject_id if subject.blank? - - if subject.respond_to?(:to_global_id) - Digest::SHA256.hexdigest(subject.to_global_id.to_s) - else - Digest::SHA256.hexdigest(subject.to_s) - end - end - end - end -end diff --git a/lib/gitlab/experimentation/experiment.rb b/lib/gitlab/experimentation/experiment.rb deleted file mode 100644 index 0c7091d19e3..00000000000 --- a/lib/gitlab/experimentation/experiment.rb +++ /dev/null @@ -1,45 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module Experimentation - class Experiment - FEATURE_FLAG_SUFFIX = "_experiment_percentage" - - attr_reader :key, :tracking_category, :rollout_strategy - - def initialize(key, **params) - @key = key - @tracking_category = params[:tracking_category] - @rollout_strategy = params[:rollout_strategy] || :cookie - end - - def active? - # TODO: just touch a feature flag - # Temporary change, we will change `experiment_percentage` in future to `Feature.enabled? - Feature.enabled?(feature_flag_name, type: :experiment) - - ::Gitlab.com? && experiment_percentage > 0 - end - - def enabled_for_index?(index) - return false if index.blank? - - index <= experiment_percentage - end - - private - - def experiment_percentage - feature_flag.percentage_of_time_value - end - - def feature_flag - Feature.get(feature_flag_name) # rubocop:disable Gitlab/AvoidFeatureGet - end - - def feature_flag_name - :"#{key}#{FEATURE_FLAG_SUFFIX}" - end - end - end -end diff --git a/lib/gitlab/experimentation_logger.rb b/lib/gitlab/experimentation_logger.rb deleted file mode 100644 index ba1b60d6b4c..00000000000 --- a/lib/gitlab/experimentation_logger.rb +++ /dev/null @@ -1,9 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - class ExperimentationLogger < ::Gitlab::JsonLogger - def self.file_name_noext - 'experimentation_json' - end - end -end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index d3a6e015019..e40527d7301 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -14902,22 +14902,40 @@ msgstr "" msgid "Enable version check" msgstr "" -msgid "EnableReviewApp|%{stepStart}Step 1%{stepEnd}. Ensure you have Kubernetes set up and have a base domain for your %{linkStart}cluster%{linkEnd}." +msgid "EnableReviewApp|Add a job in your CI/CD configuration that:" msgstr "" -msgid "EnableReviewApp|%{stepStart}Step 2%{stepEnd}. Copy the following snippet:" +msgid "EnableReviewApp|Copy snippet" msgstr "" -msgid "EnableReviewApp|%{stepStart}Step 3%{stepEnd}. Add it to the project %{linkStart}gitlab-ci.yml%{linkEnd} file." +msgid "EnableReviewApp|Have access to infrastructure that can host and deploy the review apps." msgstr "" -msgid "EnableReviewApp|%{stepStart}Step 4 (optional)%{stepEnd}. Enable Visual Reviews by following the %{linkStart}setup instructions%{linkEnd}." +msgid "EnableReviewApp|Install and configure a runner to do the deployment." msgstr "" -msgid "EnableReviewApp|Close" +msgid "EnableReviewApp|Make sure your project has an environment configured with the target URL set to your website URL. If not, create a new one before continuing." msgstr "" -msgid "EnableReviewApp|Copy snippet text" +msgid "EnableReviewApp|Only runs for feature branches or merge requests." +msgstr "" + +msgid "EnableReviewApp|Recommended: Set up a job that manually stops the Review Apps." +msgstr "" + +msgid "EnableReviewApp|Review apps are dynamic environments that you can use to provide a live preview of changes made in a feature branch." +msgstr "" + +msgid "EnableReviewApp|To configure a dynamic review app, you must:" +msgstr "" + +msgid "EnableReviewApp|Uses a predefined CI/CD variable like %{codeStart}$(CI_COMMIT_REF_SLUG)%{codeEnd} to dynamically create the review app environments. For example, for a configuration using merge request pipelines:" +msgstr "" + +msgid "EnableReviewApp|Using a static site?" +msgstr "" + +msgid "EnableReviewApp|View more example projects" msgstr "" msgid "Enabled" @@ -15145,6 +15163,9 @@ msgstr "" msgid "Environments|Commit" msgstr "" +msgid "Environments|Copy live environment URL" +msgstr "" + msgid "Environments|Delete" msgstr "" diff --git a/qa/Gemfile b/qa/Gemfile index 5895addff81..16413642eb9 100644 --- a/qa/Gemfile +++ b/qa/Gemfile @@ -9,7 +9,7 @@ gem 'capybara', '~> 3.35.0' gem 'capybara-screenshot', '~> 1.0.26' gem 'rake', '~> 13' gem 'rspec', '~> 3.10' -gem 'selenium-webdriver', '~> 4.0' +gem 'selenium-webdriver', '~> 4.5' gem 'airborne', '~> 0.3.4', require: false # airborne is messing with rspec sandboxed mode so not requiring by default gem 'rest-client', '~> 2.1.0' gem 'rspec-retry', '~> 0.6.1', require: 'rspec/retry' @@ -41,7 +41,7 @@ gem 'chemlab-library-www-gitlab-com', '~> 0.1' # dependencies for jenkins client gem 'nokogiri', '~> 1.13', '>= 1.13.8' -gem 'deprecation_toolkit', '~> 1.5.1', require: false +gem 'deprecation_toolkit', '~> 2.0.0', require: false group :development do gem 'pry-byebug', '~> 3.5.1', platform: :mri diff --git a/qa/Gemfile.lock b/qa/Gemfile.lock index 1d86f3fba08..99bfea9a760 100644 --- a/qa/Gemfile.lock +++ b/qa/Gemfile.lock @@ -54,8 +54,8 @@ GEM gitlab (>= 4.17) zeitwerk (~> 2.5.1) declarative (0.0.20) - deprecation_toolkit (1.5.1) - activesupport (>= 4.2) + deprecation_toolkit (2.0.0) + activesupport (>= 5.2) diff-lcs (1.3) domain_name (0.5.20190701) unf (>= 0.0.5, < 1.0.0) @@ -250,10 +250,11 @@ GEM sawyer (0.9.2) addressable (>= 2.3.5) faraday (>= 0.17.3, < 3) - selenium-webdriver (4.0.3) + selenium-webdriver (4.5.0) childprocess (>= 0.5, < 5.0) rexml (~> 3.2, >= 3.2.5) - rubyzip (>= 1.2.2) + rubyzip (>= 1.2.2, < 3.0) + websocket (~> 1.0) signet (0.17.0) addressable (~> 2.8) faraday (>= 0.17.5, < 3.a) @@ -287,6 +288,7 @@ GEM rubyzip (>= 1.3.0) selenium-webdriver (~> 4.0) webrick (1.7.0) + websocket (1.2.9) xpath (3.2.0) nokogiri (~> 1.8) zeitwerk (2.5.4) @@ -303,7 +305,7 @@ DEPENDENCIES chemlab (~> 0.9) chemlab-library-www-gitlab-com (~> 0.1) confiner (~> 0.3) - deprecation_toolkit (~> 1.5.1) + deprecation_toolkit (~> 2.0.0) faker (~> 2.19, >= 2.19.0) faraday-retry (~> 2.0) fog-core (= 2.1.0) @@ -325,7 +327,7 @@ DEPENDENCIES rspec-retry (~> 0.6.1) rspec_junit_formatter (~> 0.4.1) ruby-debug-ide (~> 0.7.3) - selenium-webdriver (~> 4.0) + selenium-webdriver (~> 4.5) slack-notifier (~> 2.4) terminal-table (~> 3.0.0) timecop (~> 0.9.5) diff --git a/rubocop/cop/gitlab/mark_used_feature_flags.rb b/rubocop/cop/gitlab/mark_used_feature_flags.rb index 8d8c84e78f5..23de0644385 100644 --- a/rubocop/cop/gitlab/mark_used_feature_flags.rb +++ b/rubocop/cop/gitlab/mark_used_feature_flags.rb @@ -13,11 +13,8 @@ module RuboCop include RuboCop::CodeReuseHelpers FEATURE_METHODS = %i[enabled? disabled?].freeze - EXPERIMENTATION_METHODS = %i[active?].freeze EXPERIMENT_METHODS = %i[ experiment - experiment_enabled? - push_frontend_experiment ].freeze RUGGED_METHODS = %i[ use_rugged? @@ -33,7 +30,7 @@ module RuboCop limit_feature_flag_for_override= ].freeze + EXPERIMENT_METHODS + RUGGED_METHODS + WORKER_METHODS - RESTRICT_ON_SEND = FEATURE_METHODS + EXPERIMENTATION_METHODS + SELF_METHODS + RESTRICT_ON_SEND = FEATURE_METHODS + SELF_METHODS USAGE_DATA_COUNTERS_EVENTS_YAML_GLOBS = [ File.expand_path("../../../config/metrics/aggregates/*.yml", __dir__), @@ -79,15 +76,6 @@ module RuboCop else save_used_feature_flag(flag_value) end - - if experiment_method?(node) || experimentation_method?(node) - # Additionally, mark experiment-related feature flag as used as well - matching_feature_flags = defined_feature_flags.select { |flag| flag == "#{flag_value}_experiment_percentage" } - matching_feature_flags.each do |matching_feature_flag| - puts_if_debug(node, "The '#{matching_feature_flag}' feature flag tracks the #{flag_value} experiment, which is still in use, so we'll mark it as used.") - save_used_feature_flag(matching_feature_flag) - end - end elsif flag_arg_is_send_type?(flag_arg) puts_if_debug(node, "Feature flag is dynamic: '#{flag_value}.") elsif flag_arg_is_dstr_or_dsym?(flag_arg) @@ -176,14 +164,6 @@ module RuboCop class_caller(node) == "Feature::Gitaly" end - def caller_is_experimentation?(node) - class_caller(node) == "Gitlab::Experimentation" - end - - def experiment_method?(node) - EXPERIMENT_METHODS.include?(method_name(node)) - end - def rugged_method?(node) RUGGED_METHODS.include?(method_name(node)) end @@ -192,10 +172,6 @@ module RuboCop FEATURE_METHODS.include?(method_name(node)) && (caller_is_feature?(node) || caller_is_feature_gitaly?(node)) end - def experimentation_method?(node) - EXPERIMENTATION_METHODS.include?(method_name(node)) && caller_is_experimentation?(node) - end - def worker_method?(node) WORKER_METHODS.include?(method_name(node)) end @@ -205,7 +181,7 @@ module RuboCop end def trackable_flag?(node) - feature_method?(node) || experimentation_method?(node) || self_method?(node) + feature_method?(node) || self_method?(node) end # Marking all event's feature flags as used as Gitlab::UsageDataCounters::HLLRedisCounter.track_event{,context} diff --git a/scripts/packages/automated_cleanup.rb b/scripts/packages/automated_cleanup.rb index 8b2803100e0..6f1e4c7f561 100755 --- a/scripts/packages/automated_cleanup.rb +++ b/scripts/packages/automated_cleanup.rb @@ -114,6 +114,10 @@ if $PROGRAM_NAME == __FILE__ automated_cleanup = Packages::AutomatedCleanup.new(options: options) + timed('"gitlab-workhorse" packages cleanup') do + automated_cleanup.perform_gitlab_package_cleanup!(package_name: 'gitlab-workhorse', days_for_delete: 30) + end + timed('"assets" packages cleanup') do automated_cleanup.perform_gitlab_package_cleanup!(package_name: 'assets', days_for_delete: 7) end diff --git a/spec/frontend/environments/enable_review_app_modal_spec.js b/spec/frontend/environments/enable_review_app_modal_spec.js index b6dac811ea6..7939bd600dc 100644 --- a/spec/frontend/environments/enable_review_app_modal_spec.js +++ b/spec/frontend/environments/enable_review_app_modal_spec.js @@ -1,7 +1,8 @@ import { shallowMount } from '@vue/test-utils'; import { GlModal } from '@gitlab/ui'; import { extendedWrapper } from 'helpers/vue_test_utils_helper'; -import EnableReviewAppButton from '~/environments/components/enable_review_app_modal.vue'; +import EnableReviewAppModal from '~/environments/components/enable_review_app_modal.vue'; +import { REVIEW_APP_MODAL_I18N as i18n } from '~/environments/constants'; import ModalCopyButton from '~/vue_shared/components/modal_copy_button.vue'; // hardcode uniqueId for determinism @@ -9,10 +10,12 @@ jest.mock('lodash/uniqueId', () => (x) => `${x}77`); const EXPECTED_COPY_PRE_ID = 'enable-review-app-copy-string-77'; -describe('Enable Review App Button', () => { +describe('Enable Review App Modal', () => { let wrapper; let modal; + const findInstructions = () => wrapper.findAll('ol li'); + const findInstructionAt = (i) => wrapper.findAll('ol li').at(i); const findCopyString = () => wrapper.find(`#${EXPECTED_COPY_PRE_ID}`); afterEach(() => { @@ -22,29 +25,31 @@ describe('Enable Review App Button', () => { describe('renders the modal', () => { beforeEach(() => { wrapper = extendedWrapper( - shallowMount(EnableReviewAppButton, { + shallowMount(EnableReviewAppModal, { propsData: { modalId: 'fake-id', visible: true, }, - provide: { - defaultBranchName: 'main', - }, }), ); modal = wrapper.findComponent(GlModal); }); - it('renders the defaultBranchName copy', () => { - expect(findCopyString().text()).toContain('- main'); + it('displays instructions', () => { + expect(findInstructions().length).toBe(7); + expect(findInstructionAt(0).text()).toContain(i18n.instructions.step1); + }); + + it('renders the snippet to copy', () => { + expect(findCopyString().text()).toBe(wrapper.vm.modalInfoCopyStr); }); it('renders the copyToClipboard button', () => { expect(wrapper.findComponent(ModalCopyButton).props()).toMatchObject({ modalId: 'fake-id', target: `#${EXPECTED_COPY_PRE_ID}`, - title: 'Copy snippet text', + title: i18n.copyToClipboardText, }); }); diff --git a/spec/frontend/environments/environment_external_url_spec.js b/spec/frontend/environments/environment_external_url_spec.js index 4c133665979..5966993166b 100644 --- a/spec/frontend/environments/environment_external_url_spec.js +++ b/spec/frontend/environments/environment_external_url_spec.js @@ -1,16 +1,35 @@ import { mount } from '@vue/test-utils'; +import { s__, __ } from '~/locale'; import ExternalUrlComp from '~/environments/components/environment_external_url.vue'; +import ModalCopyButton from '~/vue_shared/components/modal_copy_button.vue'; describe('External URL Component', () => { let wrapper; - const externalUrl = 'https://gitlab.com'; + let externalUrl; - beforeEach(() => { - wrapper = mount(ExternalUrlComp, { propsData: { externalUrl } }); + describe('with safe link', () => { + beforeEach(() => { + externalUrl = 'https://gitlab.com'; + wrapper = mount(ExternalUrlComp, { propsData: { externalUrl } }); + }); + + it('should link to the provided externalUrl prop', () => { + expect(wrapper.attributes('href')).toBe(externalUrl); + expect(wrapper.find('a').exists()).toBe(true); + }); }); - it('should link to the provided externalUrl prop', () => { - expect(wrapper.attributes('href')).toEqual(externalUrl); - expect(wrapper.find('a').exists()).toBe(true); + describe('with unsafe link', () => { + beforeEach(() => { + externalUrl = 'postgres://gitlab'; + wrapper = mount(ExternalUrlComp, { propsData: { externalUrl } }); + }); + + it('should show a copy button instead', () => { + const button = wrapper.findComponent(ModalCopyButton); + expect(button.props('text')).toBe(externalUrl); + expect(button.text()).toBe(__('Copy URL')); + expect(button.props('title')).toBe(s__('Environments|Copy live environment URL')); + }); }); }); diff --git a/spec/frontend/environments/environments_detail_header_spec.js b/spec/frontend/environments/environments_detail_header_spec.js index 4687119127d..1f233c05fbf 100644 --- a/spec/frontend/environments/environments_detail_header_spec.js +++ b/spec/frontend/environments/environments_detail_header_spec.js @@ -1,10 +1,12 @@ import { GlSprintf } from '@gitlab/ui'; import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; import { createMockDirective, getBinding } from 'helpers/vue_mock_directive'; +import { __, s__ } from '~/locale'; import DeleteEnvironmentModal from '~/environments/components/delete_environment_modal.vue'; import EnvironmentsDetailHeader from '~/environments/components/environments_detail_header.vue'; import StopEnvironmentModal from '~/environments/components/stop_environment_modal.vue'; import TimeAgo from '~/vue_shared/components/time_ago_tooltip.vue'; +import ModalCopyButton from '~/vue_shared/components/modal_copy_button.vue'; import { createEnvironment } from './mock_data'; describe('Environments detail header component', () => { @@ -243,4 +245,23 @@ describe('Environments detail header component', () => { expect(findDeleteEnvironmentModal().exists()).toBe(true); }); }); + + describe('when the environment has an unsafe external url', () => { + const externalUrl = 'postgres://staging'; + + beforeEach(() => { + createWrapper({ + props: { + environment: createEnvironment({ externalUrl }), + }, + }); + }); + + it('should show a copy button instead', () => { + const button = wrapper.findComponent(ModalCopyButton); + expect(button.props('title')).toBe(s__('Environments|Copy live environment URL')); + expect(button.props('text')).toBe(externalUrl); + expect(button.text()).toBe(__('Copy URL')); + }); + }); }); diff --git a/spec/frontend/import_entities/import_groups/components/import_table_spec.js b/spec/frontend/import_entities/import_groups/components/import_table_spec.js index cb967267b7f..a0115cb9349 100644 --- a/spec/frontend/import_entities/import_groups/components/import_table_spec.js +++ b/spec/frontend/import_entities/import_groups/components/import_table_spec.js @@ -12,6 +12,7 @@ import { STATUSES } from '~/import_entities/constants'; import { i18n, ROOT_NAMESPACE } from '~/import_entities/import_groups/constants'; import ImportTable from '~/import_entities/import_groups/components/import_table.vue'; import importGroupsMutation from '~/import_entities/import_groups/graphql/mutations/import_groups.mutation.graphql'; +import PaginationBar from '~/vue_shared/components/pagination_bar/pagination_bar.vue'; import PaginationLinks from '~/vue_shared/components/pagination_links.vue'; import { availableNamespacesFixture, generateFakeEntry } from '../graphql/fixtures'; @@ -528,6 +529,17 @@ describe('import table', () => { }); }); + it('renders pagination bar with storage key', async () => { + createComponent({ + bulkImportSourceGroups: () => new Promise(() => {}), + }); + await waitForPromises(); + + expect(wrapper.getComponent(PaginationBar).props('storageKey')).toBe( + ImportTable.LOCAL_STORAGE_KEY, + ); + }); + describe('unavailable features warning', () => { it('renders alert when there are unavailable features', async () => { createComponent({ diff --git a/spec/frontend/repository/components/last_commit_spec.js b/spec/frontend/repository/components/last_commit_spec.js index bf9528953b6..964b135bee3 100644 --- a/spec/frontend/repository/components/last_commit_spec.js +++ b/spec/frontend/repository/components/last_commit_spec.js @@ -185,7 +185,7 @@ describe('Repository last commit component', () => { it('strips the first newline of the description', () => { expect(findCommitRowDescription().html()).toBe( - '<pre class="commit-row-description gl-mb-3">Update ADOPTERS.md</pre>', + '<pre class="commit-row-description gl-mb-3 gl-white-space-pre-line">Update ADOPTERS.md</pre>', ); }); diff --git a/spec/frontend/vue_merge_request_widget/deployment/deployment_view_button_spec.js b/spec/frontend/vue_merge_request_widget/deployment/deployment_view_button_spec.js index eb6e3711e2e..8994fa522d0 100644 --- a/spec/frontend/vue_merge_request_widget/deployment/deployment_view_button_spec.js +++ b/spec/frontend/vue_merge_request_widget/deployment/deployment_view_button_spec.js @@ -2,6 +2,7 @@ import { GlDropdown, GlLink } from '@gitlab/ui'; import { mountExtended } from 'helpers/vue_test_utils_helper'; import DeploymentViewButton from '~/vue_merge_request_widget/components/deployment/deployment_view_button.vue'; import ReviewAppLink from '~/vue_merge_request_widget/components/review_app_link.vue'; +import ModalCopyButton from '~/vue_shared/components/modal_copy_button.vue'; import { deploymentMockData } from './deployment_mock_data'; const appButtonText = { @@ -36,6 +37,7 @@ describe('Deployment View App button', () => { const findMrWigdetDeploymentDropdownIcon = () => wrapper.findByTestId('mr-wigdet-deployment-dropdown-icon'); const findDeployUrlMenuItems = () => wrapper.findAllComponents(GlLink); + const findCopyButton = () => wrapper.findComponent(ModalCopyButton); describe('text', () => { it('renders text as passed', () => { @@ -44,39 +46,93 @@ describe('Deployment View App button', () => { }); describe('without changes', () => { + let deployment; + beforeEach(() => { - createComponent({ - propsData: { - deployment: { ...deploymentMockData, changes: null }, - appButtonText, - }, + deployment = { ...deploymentMockData, changes: null }; + }); + + describe('with safe url', () => { + beforeEach(() => { + createComponent({ + propsData: { + deployment, + appButtonText, + }, + }); + }); + + it('renders the link to the review app without dropdown', () => { + expect(findMrWigdetDeploymentDropdown().exists()).toBe(false); + expect(findReviewAppLink().attributes('href')).toBe(deployment.external_url); }); }); - it('renders the link to the review app without dropdown', () => { - expect(findMrWigdetDeploymentDropdown().exists()).toBe(false); + describe('without safe URL', () => { + beforeEach(() => { + deployment = { ...deployment, external_url: 'postgres://example' }; + createComponent({ + propsData: { + deployment, + appButtonText, + }, + }); + }); + + it('renders the link as a copy button', () => { + expect(findMrWigdetDeploymentDropdown().exists()).toBe(false); + expect(findCopyButton().props('text')).toBe(deployment.external_url); + }); }); }); describe('with a single change', () => { + let deployment; + let change; + beforeEach(() => { - createComponent({ - propsData: { - deployment: { ...deploymentMockData, changes: [deploymentMockData.changes[0]] }, - appButtonText, - }, - }); + [change] = deploymentMockData.changes; + deployment = { ...deploymentMockData, changes: [change] }; }); - it('renders the link to the review app without dropdown', () => { - expect(findMrWigdetDeploymentDropdown().exists()).toBe(false); - expect(findMrWigdetDeploymentDropdownIcon().exists()).toBe(false); + describe('with safe URL', () => { + beforeEach(() => { + createComponent({ + propsData: { + deployment, + appButtonText, + }, + }); + }); + + it('renders the link to the review app without dropdown', () => { + expect(findMrWigdetDeploymentDropdown().exists()).toBe(false); + expect(findMrWigdetDeploymentDropdownIcon().exists()).toBe(false); + }); + + it('renders the link to the review app linked to to the first change', () => { + const expectedUrl = deploymentMockData.changes[0].external_url; + + expect(findReviewAppLink().attributes('href')).toBe(expectedUrl); + }); }); - it('renders the link to the review app linked to to the first change', () => { - const expectedUrl = deploymentMockData.changes[0].external_url; + describe('with unsafe URL', () => { + beforeEach(() => { + change = { ...change, external_url: 'postgres://example' }; + deployment = { ...deployment, changes: [change] }; + createComponent({ + propsData: { + deployment, + appButtonText, + }, + }); + }); - expect(findReviewAppLink().attributes('href')).toBe(expectedUrl); + it('renders the link as a copy button', () => { + expect(findMrWigdetDeploymentDropdown().exists()).toBe(false); + expect(findCopyButton().props('text')).toBe(change.external_url); + }); }); }); diff --git a/spec/frontend/vue_shared/components/modal_copy_button_spec.js b/spec/frontend/vue_shared/components/modal_copy_button_spec.js index b57efc88d57..61e4e774420 100644 --- a/spec/frontend/vue_shared/components/modal_copy_button_spec.js +++ b/spec/frontend/vue_shared/components/modal_copy_button_spec.js @@ -17,9 +17,16 @@ describe('modal copy button', () => { title: 'Copy this value', id: 'test-id', }, + slots: { + default: 'test', + }, }); }); + it('should show the default slot', () => { + expect(wrapper.text()).toBe('test'); + }); + describe('clipboard', () => { it('should fire a `success` event on click', async () => { const root = createWrapper(wrapper.vm.$root); diff --git a/spec/frontend/vue_shared/components/pagination_bar/pagination_bar_spec.js b/spec/frontend/vue_shared/components/pagination_bar/pagination_bar_spec.js index b3be2f8a775..112cdaf74c6 100644 --- a/spec/frontend/vue_shared/components/pagination_bar/pagination_bar_spec.js +++ b/spec/frontend/vue_shared/components/pagination_bar/pagination_bar_spec.js @@ -2,6 +2,7 @@ import { GlPagination, GlDropdown, GlDropdownItem } from '@gitlab/ui'; import { mount } from '@vue/test-utils'; import PaginationBar from '~/vue_shared/components/pagination_bar/pagination_bar.vue'; import PaginationLinks from '~/vue_shared/components/pagination_links.vue'; +import LocalStorageSync from '~/vue_shared/components/local_storage_sync.vue'; describe('Pagination bar', () => { const DEFAULT_PROPS = { @@ -20,6 +21,7 @@ describe('Pagination bar', () => { ...DEFAULT_PROPS, ...propsData, }, + stubs: { LocalStorageSync: true }, }); }; @@ -90,4 +92,28 @@ describe('Pagination bar', () => { 'Showing 21 - 40 of 1000+', ); }); + + describe('local storage sync', () => { + it('does not perform local storage sync when no storage key is provided', () => { + createComponent(); + + expect(wrapper.findComponent(LocalStorageSync).exists()).toBe(false); + }); + + it('passes current page size to local storage sync when storage key is provided', () => { + const STORAGE_KEY = 'fakeStorageKey'; + createComponent({ storageKey: STORAGE_KEY }); + + expect(wrapper.getComponent(LocalStorageSync).props('storageKey')).toBe(STORAGE_KEY); + }); + + it('emits set-page event when local storage sync provides new value', () => { + const SAVED_SIZE = 50; + createComponent({ storageKey: 'some storage key' }); + + wrapper.getComponent(LocalStorageSync).vm.$emit('input', SAVED_SIZE); + + expect(wrapper.emitted('set-page-size')).toEqual([[SAVED_SIZE]]); + }); + }); }); diff --git a/spec/frontend/work_items/components/work_item_labels_spec.js b/spec/frontend/work_items/components/work_item_labels_spec.js index 3f5eb4f0def..e6ff7e8502d 100644 --- a/spec/frontend/work_items/components/work_item_labels_spec.js +++ b/spec/frontend/work_items/components/work_item_labels_spec.js @@ -7,6 +7,7 @@ import { mountExtended } from 'helpers/vue_test_utils_helper'; import { DEFAULT_DEBOUNCE_AND_THROTTLE_MS } from '~/lib/utils/constants'; import labelSearchQuery from '~/vue_shared/components/sidebar/labels_select_widget/graphql/project_labels.query.graphql'; import workItemQuery from '~/work_items/graphql/work_item.query.graphql'; +import workItemLabelsSubscription from 'ee_else_ce/work_items/graphql/work_item_labels.subscription.graphql'; import updateWorkItemMutation from '~/work_items/graphql/update_work_item.mutation.graphql'; import WorkItemLabels from '~/work_items/components/work_item_labels.vue'; import { i18n, I18N_WORK_ITEM_ERROR_FETCHING_LABELS } from '~/work_items/constants'; @@ -16,6 +17,7 @@ import { workItemQueryResponse, workItemResponseFactory, updateWorkItemMutationResponse, + workItemLabelsSubscriptionResponse, } from '../mock_data'; Vue.use(VueApollo); @@ -35,6 +37,7 @@ describe('WorkItemLabels component', () => { const successUpdateWorkItemMutationHandler = jest .fn() .mockResolvedValue(updateWorkItemMutationResponse); + const subscriptionHandler = jest.fn().mockResolvedValue(workItemLabelsSubscriptionResponse); const errorHandler = jest.fn().mockRejectedValue('Houston, we have a problem'); const createComponent = ({ @@ -47,6 +50,7 @@ describe('WorkItemLabels component', () => { [workItemQuery, workItemQueryHandler], [labelSearchQuery, searchQueryHandler], [updateWorkItemMutation, updateWorkItemMutationHandler], + [workItemLabelsSubscription, subscriptionHandler], ]); wrapper = mountExtended(WorkItemLabels, { @@ -211,5 +215,15 @@ describe('WorkItemLabels component', () => { expect(wrapper.emitted('error')).toEqual([[i18n.updateError]]); expect(updatedLabels).toEqual(initialLabels); }); + + it('has a subscription', async () => { + createComponent(); + + await waitForPromises(); + + expect(subscriptionHandler).toHaveBeenCalledWith({ + issuableId: workItemId, + }); + }); }); }); diff --git a/spec/frontend/work_items/mock_data.js b/spec/frontend/work_items/mock_data.js index 01dd2f7f6c2..66bee44ae49 100644 --- a/spec/frontend/work_items/mock_data.js +++ b/spec/frontend/work_items/mock_data.js @@ -502,6 +502,24 @@ export const workItemAssigneesSubscriptionResponse = { }, }; +export const workItemLabelsSubscriptionResponse = { + data: { + issuableLabelsUpdated: { + id: 'gid://gitlab/WorkItem/1', + widgets: [ + { + __typename: 'WorkItemWidgetLabels', + type: 'LABELS', + allowsScopedLabels: false, + labels: { + nodes: mockLabels, + }, + }, + ], + }, + }, +}; + export const workItemHierarchyEmptyResponse = { data: { workItem: { diff --git a/spec/helpers/invite_members_helper_spec.rb b/spec/helpers/invite_members_helper_spec.rb index 4d47732e008..c753d553371 100644 --- a/spec/helpers/invite_members_helper_spec.rb +++ b/spec/helpers/invite_members_helper_spec.rb @@ -11,10 +11,6 @@ RSpec.describe InviteMembersHelper do let(:owner) { project.owner } - before do - helper.extend(Gitlab::Experimentation::ControllerConcern) - end - describe '#common_invite_group_modal_data' do it 'has expected common attributes' do attributes = { diff --git a/spec/lib/bulk_imports/network_error_spec.rb b/spec/lib/bulk_imports/network_error_spec.rb index 11f555fee09..54d6554df96 100644 --- a/spec/lib/bulk_imports/network_error_spec.rb +++ b/spec/lib/bulk_imports/network_error_spec.rb @@ -46,6 +46,22 @@ RSpec.describe BulkImports::NetworkError, :clean_gitlab_redis_cache do expect(exception.retriable?(tracker)).to eq(false) end end + + context 'when entity is passed' do + it 'increments entity cache key' do + entity = create(:bulk_import_entity) + exception = described_class.new('Error!') + + allow(exception).to receive(:cause).and_return(SocketError.new('Error!')) + + expect(Gitlab::Cache::Import::Caching) + .to receive(:increment) + .with("bulk_imports/#{entity.id}/network_error/SocketError") + .and_call_original + + exception.retriable?(entity) + end + end end describe '#retry_delay' do diff --git a/spec/lib/gitlab/ci/lint_spec.rb b/spec/lib/gitlab/ci/lint_spec.rb index 3d46d266c13..cf07e952f26 100644 --- a/spec/lib/gitlab/ci/lint_spec.rb +++ b/spec/lib/gitlab/ci/lint_spec.rb @@ -342,6 +342,7 @@ RSpec.describe Gitlab::Ci::Lint do { 'count' => a_kind_of(Numeric), 'avg' => a_kind_of(Numeric), + 'sum' => a_kind_of(Numeric), 'max' => a_kind_of(Numeric), 'min' => a_kind_of(Numeric) } diff --git a/spec/lib/gitlab/ci/pipeline/logger_spec.rb b/spec/lib/gitlab/ci/pipeline/logger_spec.rb index f31361431f2..3af0ebe7484 100644 --- a/spec/lib/gitlab/ci/pipeline/logger_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/logger_spec.rb @@ -25,6 +25,7 @@ RSpec.describe ::Gitlab::Ci::Pipeline::Logger do loggable_data = { 'expensive_operation_duration_s' => { 'count' => 1, + 'sum' => a_kind_of(Numeric), 'avg' => a_kind_of(Numeric), 'max' => a_kind_of(Numeric), 'min' => a_kind_of(Numeric) @@ -62,6 +63,7 @@ RSpec.describe ::Gitlab::Ci::Pipeline::Logger do accumulator[key] = { 'count' => count, 'avg' => a_kind_of(Numeric), + 'sum' => a_kind_of(Numeric), 'max' => a_kind_of(Numeric), 'min' => a_kind_of(Numeric) } @@ -71,6 +73,7 @@ RSpec.describe ::Gitlab::Ci::Pipeline::Logger do data['expensive_operation_db_count']['max'] = db_count data['expensive_operation_db_count']['min'] = db_count data['expensive_operation_db_count']['avg'] = db_count + data['expensive_operation_db_count']['sum'] = count * db_count end data @@ -131,7 +134,7 @@ RSpec.describe ::Gitlab::Ci::Pipeline::Logger do it 'records durations of observed operations' do loggable_data = { 'pipeline_creation_duration_s' => { - 'avg' => 30, 'count' => 1, 'max' => 30, 'min' => 30 + 'avg' => 30, 'sum' => 30, 'count' => 1, 'max' => 30, 'min' => 30 } } @@ -165,10 +168,10 @@ RSpec.describe ::Gitlab::Ci::Pipeline::Logger do 'pipeline_creation_caller' => 'source', 'pipeline_source' => pipeline.source, 'pipeline_save_duration_s' => { - 'avg' => 60, 'count' => 1, 'max' => 60, 'min' => 60 + 'avg' => 60, 'sum' => 60, 'count' => 1, 'max' => 60, 'min' => 60 }, 'pipeline_creation_duration_s' => { - 'avg' => 20, 'count' => 2, 'max' => 30, 'min' => 10 + 'avg' => 20, 'sum' => 40, 'count' => 2, 'max' => 30, 'min' => 10 } } end @@ -215,10 +218,10 @@ RSpec.describe ::Gitlab::Ci::Pipeline::Logger do 'pipeline_creation_service_duration_s' => a_kind_of(Numeric), 'pipeline_creation_caller' => 'source', 'pipeline_save_duration_s' => { - 'avg' => 60, 'count' => 1, 'max' => 60, 'min' => 60 + 'avg' => 60, 'sum' => 60, 'count' => 1, 'max' => 60, 'min' => 60 }, 'pipeline_creation_duration_s' => { - 'avg' => 20, 'count' => 2, 'max' => 30, 'min' => 10 + 'avg' => 20, 'sum' => 40, 'count' => 2, 'max' => 30, 'min' => 10 } } end diff --git a/spec/lib/gitlab/experimentation/controller_concern_spec.rb b/spec/lib/gitlab/experimentation/controller_concern_spec.rb deleted file mode 100644 index 799884d7a74..00000000000 --- a/spec/lib/gitlab/experimentation/controller_concern_spec.rb +++ /dev/null @@ -1,675 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::Experimentation::ControllerConcern, type: :controller do - include TrackingHelpers - - before do - stub_const('Gitlab::Experimentation::EXPERIMENTS', { - test_experiment: { - tracking_category: 'Team', - rollout_strategy: rollout_strategy - }, - my_experiment: { - tracking_category: 'Team' - } - } - ) - - allow(Gitlab).to receive(:com?).and_return(is_gitlab_com) - - Feature.enable_percentage_of_time(:test_experiment_experiment_percentage, enabled_percentage) - end - - let(:enabled_percentage) { 10 } - let(:rollout_strategy) { nil } - let(:is_gitlab_com) { true } - - controller(ApplicationController) do - include Gitlab::Experimentation::ControllerConcern - - def index - head :ok - end - end - - describe '#set_experimentation_subject_id_cookie' do - let(:do_not_track) { nil } - let(:cookie) { cookies.permanent.signed[:experimentation_subject_id] } - let(:cookie_value) { nil } - - before do - stub_do_not_track(do_not_track) if do_not_track.present? - request.cookies[:experimentation_subject_id] = cookie_value if cookie_value - - get :index - end - - context 'cookie is present' do - let(:cookie_value) { 'test' } - - it 'does not change the cookie' do - expect(cookies[:experimentation_subject_id]).to eq 'test' - end - end - - context 'cookie is not present' do - it 'sets a permanent signed cookie' do - expect(cookie).to be_present - end - - context 'DNT: 0' do - let(:do_not_track) { '0' } - - it 'sets a permanent signed cookie' do - expect(cookie).to be_present - end - end - - context 'DNT: 1' do - let(:do_not_track) { '1' } - - it 'does nothing' do - expect(cookie).not_to be_present - end - end - end - - context 'when not on gitlab.com' do - let(:is_gitlab_com) { false } - - context 'when cookie was set' do - let(:cookie_value) { 'test' } - - it 'cookie gets deleted' do - expect(cookie).not_to be_present - end - end - - context 'when no cookie was set before' do - it 'does nothing' do - expect(cookie).not_to be_present - end - end - end - end - - describe '#push_frontend_experiment' do - it 'pushes an experiment to the frontend' do - gon = class_double('Gon') - stub_experiment_for_subject(my_experiment: true) - allow(controller).to receive(:gon).and_return(gon) - - expect(gon).to receive(:push).with({ experiments: { 'myExperiment' => true } }, true) - - controller.push_frontend_experiment(:my_experiment) - end - end - - describe '#experiment_enabled?' do - def check_experiment(exp_key = :test_experiment, subject = nil) - controller.experiment_enabled?(exp_key, subject: subject) - end - - subject { check_experiment } - - context 'cookie is not present' do - it { is_expected.to eq(false) } - end - - context 'cookie is present' do - before do - cookies.permanent.signed[:experimentation_subject_id] = 'abcd-1234' - get :index - end - - it 'calls Gitlab::Experimentation.in_experiment_group? with the name of the experiment and the calculated experimentation_subject_index based on the uuid' do - expect(Gitlab::Experimentation).to receive(:in_experiment_group?).with(:test_experiment, subject: 'abcd-1234') - - check_experiment(:test_experiment) - end - - context 'when subject is given' do - let(:rollout_strategy) { :user } - let(:user) { build(:user) } - - it 'uses the subject' do - expect(Gitlab::Experimentation).to receive(:in_experiment_group?).with(:test_experiment, subject: user) - - check_experiment(:test_experiment, user) - end - end - end - - context 'do not track' do - before do - allow(Gitlab::Experimentation).to receive(:in_experiment_group?) { true } - end - - context 'when do not track is disabled' do - before do - controller.request.headers['DNT'] = '0' - end - - it { is_expected.to eq(true) } - end - - context 'when do not track is enabled' do - before do - controller.request.headers['DNT'] = '1' - end - - it { is_expected.to eq(false) } - end - end - - context 'URL parameter to force enable experiment' do - it 'returns true unconditionally' do - get :index, params: { force_experiment: :test_experiment } - - is_expected.to eq(true) - end - end - - context 'Cookie parameter to force enable experiment' do - it 'returns true unconditionally' do - cookies[:force_experiment] = 'test_experiment,another_experiment' - get :index - - expect(check_experiment(:test_experiment)).to eq(true) - expect(check_experiment(:another_experiment)).to eq(true) - end - end - end - - describe '#track_experiment_event', :snowplow do - let(:user) { build(:user) } - - context 'when the experiment is enabled' do - before do - stub_experiment(test_experiment: true) - allow(controller).to receive(:current_user).and_return(user) - end - - context 'the user is part of the experimental group' do - before do - stub_experiment_for_subject(test_experiment: true) - end - - it 'tracks the event with the right parameters' do - controller.track_experiment_event(:test_experiment, 'start', 1) - - expect_snowplow_event( - category: 'Team', - action: 'start', - property: 'experimental_group', - value: 1, - user: user - ) - end - end - - context 'the user is part of the control group' do - before do - stub_experiment_for_subject(test_experiment: false) - end - - it 'tracks the event with the right parameters' do - controller.track_experiment_event(:test_experiment, 'start', 1) - - expect_snowplow_event( - category: 'Team', - action: 'start', - property: 'control_group', - value: 1, - user: user - ) - end - end - - context 'do not track is disabled' do - before do - stub_do_not_track('0') - end - - it 'does track the event' do - controller.track_experiment_event(:test_experiment, 'start', 1) - - expect_snowplow_event( - category: 'Team', - action: 'start', - property: 'control_group', - value: 1, - user: user - ) - end - end - - context 'do not track enabled' do - before do - stub_do_not_track('1') - end - - it 'does not track the event' do - controller.track_experiment_event(:test_experiment, 'start', 1) - - expect_no_snowplow_event - end - end - - context 'subject is provided' do - before do - stub_experiment_for_subject(test_experiment: false) - end - - it "provides the subject's hashed global_id as label" do - experiment_subject = double(:subject, to_global_id: 'abc') - allow(Gitlab::Experimentation).to receive(:valid_subject_for_rollout_strategy?).and_return(true) - - controller.track_experiment_event(:test_experiment, 'start', 1, subject: experiment_subject) - - expect_snowplow_event( - category: 'Team', - action: 'start', - property: 'control_group', - value: 1, - label: Digest::SHA256.hexdigest('abc'), - user: user - ) - end - - it "provides the subject's hashed string representation as label" do - experiment_subject = 'somestring' - - controller.track_experiment_event(:test_experiment, 'start', 1, subject: experiment_subject) - - expect_snowplow_event( - category: 'Team', - action: 'start', - property: 'control_group', - value: 1, - label: Digest::SHA256.hexdigest('somestring'), - user: user - ) - end - end - - context 'no subject is provided but cookie is set' do - before do - get :index - stub_experiment_for_subject(test_experiment: false) - end - - it 'uses the experimentation_subject_id as fallback' do - controller.track_experiment_event(:test_experiment, 'start', 1) - - expect_snowplow_event( - category: 'Team', - action: 'start', - property: 'control_group', - value: 1, - label: cookies.permanent.signed[:experimentation_subject_id], - user: user - ) - end - end - end - - context 'when the experiment is disabled' do - before do - stub_experiment(test_experiment: false) - end - - it 'does not track the event' do - controller.track_experiment_event(:test_experiment, 'start') - - expect_no_snowplow_event - end - end - end - - describe '#frontend_experimentation_tracking_data' do - context 'when the experiment is enabled' do - before do - stub_experiment(test_experiment: true) - end - - context 'the user is part of the experimental group' do - before do - stub_experiment_for_subject(test_experiment: true) - end - - it 'pushes the right parameters to gon' do - controller.frontend_experimentation_tracking_data(:test_experiment, 'start', 'team_id') - expect(Gon.tracking_data).to eq( - { - category: 'Team', - action: 'start', - property: 'experimental_group', - value: 'team_id' - } - ) - end - end - - context 'the user is part of the control group' do - before do - stub_experiment_for_subject(test_experiment: false) - end - - it 'pushes the right parameters to gon' do - controller.frontend_experimentation_tracking_data(:test_experiment, 'start', 'team_id') - expect(Gon.tracking_data).to eq( - { - category: 'Team', - action: 'start', - property: 'control_group', - value: 'team_id' - } - ) - end - - it 'does not send nil value to gon' do - controller.frontend_experimentation_tracking_data(:test_experiment, 'start') - expect(Gon.tracking_data).to eq( - { - category: 'Team', - action: 'start', - property: 'control_group' - } - ) - end - end - - context 'do not track disabled' do - before do - stub_do_not_track('0') - end - - it 'pushes the right parameters to gon' do - controller.frontend_experimentation_tracking_data(:test_experiment, 'start') - - expect(Gon.tracking_data).to eq( - { - category: 'Team', - action: 'start', - property: 'control_group' - } - ) - end - end - - context 'do not track enabled' do - before do - stub_do_not_track('1') - end - - it 'does not push data to gon' do - controller.frontend_experimentation_tracking_data(:test_experiment, 'start') - - expect(Gon.method_defined?(:tracking_data)).to eq(false) - end - end - end - - context 'when the experiment is disabled' do - before do - stub_experiment(test_experiment: false) - end - - it 'does not push data to gon' do - expect(Gon.method_defined?(:tracking_data)).to eq(false) - controller.track_experiment_event(:test_experiment, 'start') - end - end - end - - describe '#record_experiment_user' do - let(:user) { build(:user) } - let(:context) { { a: 42 } } - - context 'when the experiment is enabled' do - before do - stub_experiment(test_experiment: true) - allow(controller).to receive(:current_user).and_return(user) - end - - context 'the user is part of the experimental group' do - before do - stub_experiment_for_subject(test_experiment: true) - end - - it 'calls add_user on the Experiment model' do - expect(::Experiment).to receive(:add_user).with(:test_experiment, :experimental, user, context) - - controller.record_experiment_user(:test_experiment, context) - end - - context 'with a cookie based rollout strategy' do - it 'calls tracking_group with a nil subject' do - expect(controller).to receive(:tracking_group).with(:test_experiment, nil, subject: nil).and_return(:experimental) - allow(::Experiment).to receive(:add_user).with(:test_experiment, :experimental, user, context) - - controller.record_experiment_user(:test_experiment, context) - end - end - - context 'with a user based rollout strategy' do - let(:rollout_strategy) { :user } - - it 'calls tracking_group with a user subject' do - expect(controller).to receive(:tracking_group).with(:test_experiment, nil, subject: user).and_return(:experimental) - allow(::Experiment).to receive(:add_user).with(:test_experiment, :experimental, user, context) - - controller.record_experiment_user(:test_experiment, context) - end - end - end - - context 'the user is part of the control group' do - before do - stub_experiment_for_subject(test_experiment: false) - end - - it 'calls add_user on the Experiment model' do - expect(::Experiment).to receive(:add_user).with(:test_experiment, :control, user, context) - - controller.record_experiment_user(:test_experiment, context) - end - end - end - - context 'when the experiment is disabled' do - before do - stub_experiment(test_experiment: false) - allow(controller).to receive(:current_user).and_return(user) - end - - it 'does not call add_user on the Experiment model' do - expect(::Experiment).not_to receive(:add_user) - - controller.record_experiment_user(:test_experiment, context) - end - end - - context 'when there is no current_user' do - before do - stub_experiment(test_experiment: true) - end - - it 'does not call add_user on the Experiment model' do - expect(::Experiment).not_to receive(:add_user) - - controller.record_experiment_user(:test_experiment, context) - end - end - - context 'do not track' do - before do - stub_experiment(test_experiment: true) - allow(controller).to receive(:current_user).and_return(user) - end - - context 'is disabled' do - before do - stub_do_not_track('0') - stub_experiment_for_subject(test_experiment: false) - end - - it 'calls add_user on the Experiment model' do - expect(::Experiment).to receive(:add_user).with(:test_experiment, :control, user, context) - - controller.record_experiment_user(:test_experiment, context) - end - end - - context 'is enabled' do - before do - stub_do_not_track('1') - end - - it 'does not call add_user on the Experiment model' do - expect(::Experiment).not_to receive(:add_user) - - controller.record_experiment_user(:test_experiment, context) - end - end - end - end - - describe '#record_experiment_group' do - let(:group) { 'a group object' } - let(:experiment_key) { :some_experiment_key } - let(:dnt_enabled) { false } - let(:experiment_active) { true } - let(:rollout_strategy) { :whatever } - let(:variant) { 'variant' } - - before do - allow(controller).to receive(:dnt_enabled?).and_return(dnt_enabled) - allow(::Gitlab::Experimentation).to receive(:active?).and_return(experiment_active) - allow(::Gitlab::Experimentation).to receive(:rollout_strategy).and_return(rollout_strategy) - allow(controller).to receive(:tracking_group).and_return(variant) - allow(::Experiment).to receive(:add_group) - end - - subject(:record_experiment_group) { controller.record_experiment_group(experiment_key, group) } - - shared_examples 'exits early without recording' do - it 'returns early without recording the group as an ExperimentSubject' do - expect(::Experiment).not_to receive(:add_group) - record_experiment_group - end - end - - shared_examples 'calls tracking_group' do |using_cookie_rollout| - it "calls tracking_group with #{using_cookie_rollout ? 'a nil' : 'the group as the'} subject" do - expect(controller).to receive(:tracking_group).with(experiment_key, nil, subject: using_cookie_rollout ? nil : group).and_return(variant) - record_experiment_group - end - end - - shared_examples 'records the group' do - it 'records the group' do - expect(::Experiment).to receive(:add_group).with(experiment_key, group: group, variant: variant) - record_experiment_group - end - end - - context 'when DNT is enabled' do - let(:dnt_enabled) { true } - - include_examples 'exits early without recording' - end - - context 'when the experiment is not active' do - let(:experiment_active) { false } - - include_examples 'exits early without recording' - end - - context 'when a nil group is given' do - let(:group) { nil } - - include_examples 'exits early without recording' - end - - context 'when the experiment uses a cookie-based rollout strategy' do - let(:rollout_strategy) { :cookie } - - include_examples 'calls tracking_group', true - include_examples 'records the group' - end - - context 'when the experiment uses a non-cookie-based rollout strategy' do - let(:rollout_strategy) { :group } - - include_examples 'calls tracking_group', false - include_examples 'records the group' - end - end - - describe '#record_experiment_conversion_event' do - let(:user) { build(:user) } - - before do - allow(controller).to receive(:dnt_enabled?).and_return(false) - allow(controller).to receive(:current_user).and_return(user) - stub_experiment(test_experiment: true) - end - - subject(:record_conversion_event) do - controller.record_experiment_conversion_event(:test_experiment) - end - - it 'records the conversion event for the experiment & user' do - expect(::Experiment).to receive(:record_conversion_event).with(:test_experiment, user, {}) - record_conversion_event - end - - shared_examples 'does not record the conversion event' do - it 'does not record the conversion event' do - expect(::Experiment).not_to receive(:record_conversion_event) - record_conversion_event - end - end - - context 'when DNT is enabled' do - before do - allow(controller).to receive(:dnt_enabled?).and_return(true) - end - - include_examples 'does not record the conversion event' - end - - context 'when there is no current user' do - before do - allow(controller).to receive(:current_user).and_return(nil) - end - - include_examples 'does not record the conversion event' - end - - context 'when the experiment is not enabled' do - before do - stub_experiment(test_experiment: false) - end - - include_examples 'does not record the conversion event' - end - end - - describe '#experiment_tracking_category_and_group' do - let_it_be(:experiment_key) { :test_something } - - subject { controller.experiment_tracking_category_and_group(experiment_key) } - - it 'returns a string with the experiment tracking category & group joined with a ":"' do - expect(controller).to receive(:tracking_category).with(experiment_key).and_return('Experiment::Category') - expect(controller).to receive(:tracking_group).with(experiment_key, '_group', subject: nil).and_return('experimental_group') - - expect(subject).to eq('Experiment::Category:experimental_group') - end - end -end diff --git a/spec/lib/gitlab/experimentation/experiment_spec.rb b/spec/lib/gitlab/experimentation/experiment_spec.rb deleted file mode 100644 index a5cc69b9538..00000000000 --- a/spec/lib/gitlab/experimentation/experiment_spec.rb +++ /dev/null @@ -1,58 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::Experimentation::Experiment do - using RSpec::Parameterized::TableSyntax - - let(:percentage) { 50 } - let(:params) do - { - tracking_category: 'Category1', - rollout_strategy: nil - } - end - - before do - skip_feature_flags_yaml_validation - skip_default_enabled_yaml_check - allow(Feature).to receive(:log_feature_flag_states?).and_return(false) - feature = double('FeatureFlag', percentage_of_time_value: percentage, enabled?: true) - allow(Feature).to receive(:get).with(:experiment_key_experiment_percentage).and_return(feature) - end - - subject(:experiment) { described_class.new(:experiment_key, **params) } - - describe '#active?' do - before do - allow(Gitlab).to receive(:com?).and_return(on_gitlab_com) - end - - subject { experiment.active? } - - where(:on_gitlab_com, :percentage, :is_active) do - true | 0 | false - true | 10 | true - false | 0 | false - false | 10 | false - end - - with_them do - it { is_expected.to eq(is_active) } - end - end - - describe '#enabled_for_index?' do - subject { experiment.enabled_for_index?(index) } - - where(:index, :percentage, :is_enabled) do - 50 | 40 | false - 40 | 50 | true - nil | 50 | false - end - - with_them do - it { is_expected.to eq(is_enabled) } - end - end -end diff --git a/spec/lib/gitlab/experimentation_spec.rb b/spec/lib/gitlab/experimentation_spec.rb deleted file mode 100644 index c482874b725..00000000000 --- a/spec/lib/gitlab/experimentation_spec.rb +++ /dev/null @@ -1,161 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::Experimentation do - using RSpec::Parameterized::TableSyntax - - before do - stub_const('Gitlab::Experimentation::EXPERIMENTS', { - test_experiment: { - tracking_category: 'Team' - }, - tabular_experiment: { - tracking_category: 'Team', - rollout_strategy: rollout_strategy - } - }) - - skip_feature_flags_yaml_validation - skip_default_enabled_yaml_check - Feature.enable_percentage_of_time(:test_experiment_experiment_percentage, enabled_percentage) - allow(Gitlab).to receive(:com?).and_return(true) - end - - let(:enabled_percentage) { 10 } - let(:rollout_strategy) { nil } - - describe '.get_experiment' do - subject { described_class.get_experiment(:test_experiment) } - - context 'returns experiment' do - it { is_expected.to be_instance_of(Gitlab::Experimentation::Experiment) } - end - - context 'experiment is not defined' do - subject { described_class.get_experiment(:missing_experiment) } - - it { is_expected.to be_nil } - end - end - - describe '.active?' do - subject { described_class.active?(:test_experiment) } - - context 'feature toggle is enabled' do - it { is_expected.to eq(true) } - end - - describe 'experiment is not defined' do - it 'returns false' do - expect(described_class.active?(:missing_experiment)).to eq(false) - end - end - - describe 'experiment is disabled' do - let(:enabled_percentage) { 0 } - - it { is_expected.to eq(false) } - end - end - - describe '.in_experiment_group?' do - let(:enabled_percentage) { 50 } - let(:experiment_subject) { 'z' } # Zlib.crc32('test_experimentz') % 100 = 33 - - subject { described_class.in_experiment_group?(:test_experiment, subject: experiment_subject) } - - context 'when experiment is active' do - context 'when subject is part of the experiment' do - it { is_expected.to eq(true) } - end - - context 'when subject is not part of the experiment' do - let(:experiment_subject) { 'a' } # Zlib.crc32('test_experimenta') % 100 = 61 - - it { is_expected.to eq(false) } - end - - context 'when subject has a global_id' do - let(:experiment_subject) { double(:subject, to_global_id: 'z') } - - it { is_expected.to eq(true) } - end - - context 'when subject is nil' do - let(:experiment_subject) { nil } - - it { is_expected.to eq(false) } - end - - context 'when subject is an empty string' do - let(:experiment_subject) { '' } - - it { is_expected.to eq(false) } - end - end - - context 'when experiment is not active' do - before do - allow(described_class).to receive(:active?).and_return(false) - end - - it { is_expected.to eq(false) } - end - end - - describe '.log_invalid_rollout' do - subject { described_class.log_invalid_rollout(:test_experiment, 1) } - - before do - allow(described_class).to receive(:valid_subject_for_rollout_strategy?).and_return(valid) - end - - context 'subject is not valid for experiment' do - let(:valid) { false } - - it 'logs a warning message' do - expect_next_instance_of(Gitlab::ExperimentationLogger) do |logger| - expect(logger) - .to receive(:warn) - .with( - message: 'Subject must conform to the rollout strategy', - experiment_key: :test_experiment, - subject: 'Integer', - rollout_strategy: :cookie - ) - end - - subject - end - end - - context 'subject is valid for experiment' do - let(:valid) { true } - - it 'does not log a warning message' do - expect(Gitlab::ExperimentationLogger).not_to receive(:build) - - subject - end - end - end - - describe '.valid_subject_for_rollout_strategy?' do - subject { described_class.valid_subject_for_rollout_strategy?(:tabular_experiment, experiment_subject) } - - where(:rollout_strategy, :experiment_subject, :result) do - :cookie | nil | true - nil | nil | true - :cookie | 'string' | true - nil | User.new | false - :user | User.new | true - :group | User.new | false - :group | Group.new | true - end - - with_them do - it { is_expected.to be(result) } - end - end -end diff --git a/spec/models/bulk_imports/export_status_spec.rb b/spec/models/bulk_imports/export_status_spec.rb index 606e663e310..0921c3bdce2 100644 --- a/spec/models/bulk_imports/export_status_spec.rb +++ b/spec/models/bulk_imports/export_status_spec.rb @@ -157,20 +157,36 @@ RSpec.describe BulkImports::ExportStatus do end context 'when something goes wrong during export status fetch' do - it 'returns exception class as error and memoizes return value' do + let(:exception) { BulkImports::NetworkError.new('Error!') } + + before do allow_next_instance_of(BulkImports::Clients::HTTP) do |client| - allow(client).to receive(:get).and_raise(StandardError, 'Error!') + allow(client).to receive(:get).once.and_raise(exception) end + end - expect(subject.error).to eq('Error!') - expect(subject.failed?).to eq(true) + it 'raises RetryPipelineError' do + allow(exception).to receive(:retriable?).with(tracker).and_return(true) - allow_next_instance_of(BulkImports::Clients::HTTP) do |client| - allow(client).to receive(:get).and_return({ 'relation' => relation, 'status' => 'finished' }) + expect { subject.failed? }.to raise_error(BulkImports::RetryPipelineError) + end + + context 'when error is not retriable' do + it 'returns exception class as error' do + expect(subject.error).to eq('Error!') + expect(subject.failed?).to eq(true) end + end - expect(subject.error).to eq('Error!') - expect(subject.failed?).to eq(true) + context 'when error raised is not a network error' do + it 'returns exception class as error' do + allow_next_instance_of(BulkImports::Clients::HTTP) do |client| + allow(client).to receive(:get).once.and_raise(StandardError, 'Standard Error!') + end + + expect(subject.error).to eq('Standard Error!') + expect(subject.failed?).to eq(true) + end end end end diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index bf1cf9856a0..b91d836f82f 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -28,7 +28,7 @@ RSpec.describe Deployment do let(:deployment) { create(:deployment) } it 'delegates to environment_manual_actions' do - expect(deployment.deployable).to receive(:environment_manual_actions).and_call_original + expect(deployment.deployable).to receive(:other_manual_actions).and_call_original deployment.manual_actions end @@ -38,7 +38,7 @@ RSpec.describe Deployment do let(:deployment) { create(:deployment) } it 'delegates to environment_scheduled_actions' do - expect(deployment.deployable).to receive(:environment_scheduled_actions).and_call_original + expect(deployment.deployable).to receive(:other_scheduled_actions).and_call_original deployment.scheduled_actions end diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb index 9403eaa1a33..9700852e567 100644 --- a/spec/models/event_spec.rb +++ b/spec/models/event_spec.rb @@ -16,35 +16,6 @@ RSpec.describe Event do it { is_expected.to respond_to(:design_title) } end - describe '.first' do - let(:recorded_query) do - recorder = ActiveRecord::QueryRecorder.new do - described_class.first(3) - end - recorder.data.each_value.first[:occurrences].first - end - - context 'when skip_default_scope_for_events FF is on' do - before do - stub_feature_flags(skip_default_scope_for_events: true) - end - - it 'orders by id' do - expect(recorded_query).to include('FROM "events" ORDER BY "events"."id" ASC LIMIT 3') - end - end - - context 'when skip_default_scope_for_events FF is off' do - before do - stub_feature_flags(skip_default_scope_for_events: false) - end - - it 'does not have ORDER BY clause' do - expect(recorded_query).to include('FROM "events" LIMIT 3') - end - end - end - describe 'Callbacks' do let(:project) { create(:project) } diff --git a/spec/rubocop/cop/gitlab/mark_used_feature_flags_spec.rb b/spec/rubocop/cop/gitlab/mark_used_feature_flags_spec.rb index ac7e41dda44..a3c9ae8916e 100644 --- a/spec/rubocop/cop/gitlab/mark_used_feature_flags_spec.rb +++ b/spec/rubocop/cop/gitlab/mark_used_feature_flags_spec.rb @@ -7,7 +7,7 @@ require_relative '../../../../rubocop/cop/gitlab/mark_used_feature_flags' RSpec.describe RuboCop::Cop::Gitlab::MarkUsedFeatureFlags do let(:defined_feature_flags) do - %w[a_feature_flag foo_hello foo_world baz_experiment_percentage bar_baz] + %w[a_feature_flag foo_hello foo_world bar_baz baz] end before do @@ -118,40 +118,33 @@ RSpec.describe RuboCop::Cop::Gitlab::MarkUsedFeatureFlags do end end - %w[ - experiment - experiment_enabled? - push_frontend_experiment - Gitlab::Experimentation.active? - ].each do |feature_flag_method| - context "#{feature_flag_method} method" do - context 'a string feature flag' do - include_examples 'sets flag as used', %Q|#{feature_flag_method}("baz")|, %w[baz baz_experiment_percentage] - end + context 'with the experiment method' do + context 'a string feature flag' do + include_examples 'sets flag as used', %q|experiment("baz")|, %w[baz] + end - context 'a symbol feature flag' do - include_examples 'sets flag as used', %Q|#{feature_flag_method}(:baz)|, %w[baz baz_experiment_percentage] - end + context 'a symbol feature flag' do + include_examples 'sets flag as used', %q|experiment(:baz)|, %w[baz] + end - context 'an interpolated string feature flag with a string prefix' do - include_examples 'sets flag as used', %Q|#{feature_flag_method}("foo_\#{bar}")|, %w[foo_hello foo_world] - end + context 'an interpolated string feature flag with a string prefix' do + include_examples 'sets flag as used', %Q|experiment("foo_\#{bar}")|, %w[foo_hello foo_world] + end - context 'an interpolated symbol feature flag with a string prefix' do - include_examples 'sets flag as used', %Q|#{feature_flag_method}(:"foo_\#{bar}")|, %w[foo_hello foo_world] - end + context 'an interpolated symbol feature flag with a string prefix' do + include_examples 'sets flag as used', %Q|experiment(:"foo_\#{bar}")|, %w[foo_hello foo_world] + end - context 'an interpolated string feature flag with a string prefix and suffix' do - include_examples 'does not set any flags as used', %Q|#{feature_flag_method}(:"foo_\#{bar}_baz")| - end + context 'an interpolated string feature flag with a string prefix and suffix' do + include_examples 'does not set any flags as used', %Q|experiment(:"foo_\#{bar}_baz")| + end - context 'a dynamic string feature flag as a variable' do - include_examples 'does not set any flags as used', %Q|#{feature_flag_method}(a_variable, an_arg)| - end + context 'a dynamic string feature flag as a variable' do + include_examples 'does not set any flags as used', %q|experiment(a_variable, an_arg)| + end - context 'an integer feature flag' do - include_examples 'does not set any flags as used', %Q|#{feature_flag_method}(123)| - end + context 'an integer feature flag' do + include_examples 'does not set any flags as used', %q|experiment(123)| end end diff --git a/spec/serializers/deployment_entity_spec.rb b/spec/serializers/deployment_entity_spec.rb index d95aa882a64..0746e68d7c5 100644 --- a/spec/serializers/deployment_entity_spec.rb +++ b/spec/serializers/deployment_entity_spec.rb @@ -58,8 +58,8 @@ RSpec.describe DeploymentEntity do let_it_be(:other_deployment) { create(:deployment, deployable: build, environment: environment) } it 'returns another manual action' do - expect(subject[:manual_actions].count).to eq(2) - expect(subject[:manual_actions].pluck(:name)).to match_array(['test', 'another deploy']) + expect(subject[:manual_actions].count).to eq(1) + expect(subject[:manual_actions].pluck(:name)).to match_array(['another deploy']) end context 'when user is a reporter' do diff --git a/spec/services/ci/create_pipeline_service/include_spec.rb b/spec/services/ci/create_pipeline_service/include_spec.rb index 67d8530525a..3764663fd74 100644 --- a/spec/services/ci/create_pipeline_service/include_spec.rb +++ b/spec/services/ci/create_pipeline_service/include_spec.rb @@ -126,51 +126,5 @@ RSpec.describe Ci::CreatePipelineService, :yaml_processor_feature_flag_corectnes it_behaves_like 'not including the file' end end - - context 'with ci_increase_includes_to_250 enabled on root project' do - let_it_be(:included_project) do - create(:project, :repository).tap { |p| p.add_developer(user) } - end - - before do - stub_const('::Gitlab::Ci::Config::External::Context::MAX_INCLUDES', 0) - stub_const('::Gitlab::Ci::Config::External::Context::TRIAL_MAX_INCLUDES', 3) - - stub_feature_flags(ci_increase_includes_to_250: false) - stub_feature_flags(ci_increase_includes_to_250: project) - - allow(Project) - .to receive(:find_by_full_path) - .with(included_project.full_path) - .and_return(included_project) - - allow(included_project.repository) - .to receive(:blob_data_at).with(included_project.commit.id, '.gitlab-ci.yml') - .and_return(local_config) - - allow(included_project.repository) - .to receive(:blob_data_at).with(included_project.commit.id, file_location) - .and_return(File.read(Rails.root.join(file_location))) - end - - let(:config) do - <<~EOY - include: - - project: #{included_project.full_path} - file: .gitlab-ci.yml - EOY - end - - let(:local_config) do - <<~EOY - include: #{file_location} - - job: - script: exit 0 - EOY - end - - it_behaves_like 'including the file' - end end end diff --git a/spec/services/ci/create_pipeline_service/logger_spec.rb b/spec/services/ci/create_pipeline_service/logger_spec.rb index 2be23802757..3045f8e92b1 100644 --- a/spec/services/ci/create_pipeline_service/logger_spec.rb +++ b/spec/services/ci/create_pipeline_service/logger_spec.rb @@ -20,6 +20,7 @@ RSpec.describe Ci::CreatePipelineService, :yaml_processor_feature_flag_corectnes { 'count' => a_kind_of(Numeric), 'avg' => a_kind_of(Numeric), + 'sum' => a_kind_of(Numeric), 'max' => a_kind_of(Numeric), 'min' => a_kind_of(Numeric) } diff --git a/spec/services/packages/debian/create_package_file_service_spec.rb b/spec/services/packages/debian/create_package_file_service_spec.rb index c8292b2d5c2..291f6df991c 100644 --- a/spec/services/packages/debian/create_package_file_service_spec.rb +++ b/spec/services/packages/debian/create_package_file_service_spec.rb @@ -6,6 +6,7 @@ RSpec.describe Packages::Debian::CreatePackageFileService do include WorkhorseHelpers let_it_be(:package) { create(:debian_incoming, without_package_files: true) } + let_it_be(:current_user) { create(:user) } describe '#execute' do let(:file_name) { 'libsample0_1.2.3~alpha2_amd64.deb' } @@ -20,12 +21,13 @@ RSpec.describe Packages::Debian::CreatePackageFileService do }.with_indifferent_access end - let(:service) { described_class.new(package, params) } + let(:service) { described_class.new(package: package, current_user: current_user, params: params) } subject(:package_file) { service.execute } shared_examples 'a valid deb' do it 'creates a new package file', :aggregate_failures do + expect(::Packages::Debian::ProcessChangesWorker).not_to receive(:perform_async) expect(package_file).to be_valid expect(package_file.file.read).to start_with('!<arch>') expect(package_file.size).to eq(1124) @@ -40,6 +42,24 @@ RSpec.describe Packages::Debian::CreatePackageFileService do end end + shared_examples 'a valid changes' do + it 'creates a new package file', :aggregate_failures do + expect(::Packages::Debian::ProcessChangesWorker).to receive(:perform_async) + + expect(package_file).to be_valid + expect(package_file.file.read).to start_with('Format: 1.8') + expect(package_file.size).to eq(2143) + expect(package_file.file_name).to eq(file_name) + expect(package_file.file_sha1).to eq('54321') + expect(package_file.file_sha256).to eq('543212345') + expect(package_file.file_md5).to eq('12345') + expect(package_file.debian_file_metadatum).to be_valid + expect(package_file.debian_file_metadatum.file_type).to eq('unknown') + expect(package_file.debian_file_metadatum.architecture).to be_nil + expect(package_file.debian_file_metadatum.fields).to be_nil + end + end + context 'with temp file' do let!(:file) do upload_path = ::Packages::PackageFileUploader.workhorse_local_upload_path @@ -52,6 +72,21 @@ RSpec.describe Packages::Debian::CreatePackageFileService do end it_behaves_like 'a valid deb' + + context 'with a .changes file' do + let(:file_name) { 'sample_1.2.3~alpha2_amd64.changes' } + let(:fixture_path) { "spec/fixtures/packages/debian/#{file_name}" } + + it_behaves_like 'a valid changes' + end + + context 'when current_user is missing' do + let(:current_user) { nil } + + it 'raises an error' do + expect { package_file }.to raise_error(ArgumentError, 'Invalid user') + end + end end context 'with remote file' do @@ -77,37 +112,37 @@ RSpec.describe Packages::Debian::CreatePackageFileService do it_behaves_like 'a valid deb' end - context 'package is missing' do + context 'when package is missing' do let(:package) { nil } let(:params) { {} } it 'raises an error' do - expect { subject.execute }.to raise_error(ArgumentError, 'Invalid package') + expect { package_file }.to raise_error(ArgumentError, 'Invalid package') end end - context 'params is empty' do + context 'when params is empty' do let(:params) { {} } it 'raises an error' do - expect { subject.execute }.to raise_error(ActiveRecord::RecordInvalid) + expect { package_file }.to raise_error(ActiveRecord::RecordInvalid) end end - context 'file is missing' do + context 'when file is missing' do let(:file_name) { 'libsample0_1.2.3~alpha2_amd64.deb' } let(:file) { nil } it 'raises an error' do - expect { subject.execute }.to raise_error(ActiveRecord::RecordInvalid) + expect { package_file }.to raise_error(ActiveRecord::RecordInvalid) end end - context 'FIPS mode enabled', :fips_mode do + context 'when FIPS mode enabled', :fips_mode do let(:file) { nil } it 'raises an error' do - expect { subject.execute }.to raise_error(::Packages::FIPS::DisabledError) + expect { package_file }.to raise_error(::Packages::FIPS::DisabledError) end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 74f1a712c30..8a1fa486bde 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -140,7 +140,6 @@ RSpec.configure do |config| config.include FixtureHelpers config.include NonExistingRecordsHelpers config.include GitlabRoutingHelper - config.include StubExperiments config.include StubGitlabCalls config.include NextFoundInstanceOf config.include NextInstanceOf diff --git a/spec/support/helpers/stub_experiments.rb b/spec/support/helpers/stub_experiments.rb deleted file mode 100644 index 8995b8f5f7b..00000000000 --- a/spec/support/helpers/stub_experiments.rb +++ /dev/null @@ -1,37 +0,0 @@ -# frozen_string_literal: true - -module StubExperiments - # Stub Experiment with `key: true/false` - # - # @param [Hash] experiment where key is feature name and value is boolean whether active or not. - # - # Examples - # - `stub_experiment(signup_flow: false)` ... Disables `signup_flow` experiment. - def stub_experiment(experiments) - allow(Gitlab::Experimentation).to receive(:active?).and_call_original - - experiments.each do |experiment_key, enabled| - allow(Gitlab::Experimentation).to receive(:active?).with(experiment_key) { enabled } - end - end - - # Stub Experiment for user with `key: true/false` - # - # @param [Hash] experiment where key is feature name and value is boolean whether enabled or not. - # - # Examples - # - `stub_experiment_for_subject(signup_flow: false)` ... Disable `signup_flow` experiment for user. - def stub_experiment_for_subject(experiments) - allow(Gitlab::Experimentation).to receive(:in_experiment_group?).and_call_original - - experiments.each do |experiment_key, enabled| - allow(Gitlab::Experimentation).to receive(:in_experiment_group?).with(experiment_key, anything) { enabled } - end - end - - private - - def feature_flag_suffix - Gitlab::Experimentation::Experiment::FEATURE_FLAG_SUFFIX - end -end diff --git a/spec/support/shared_examples/requests/api/debian_packages_shared_examples.rb b/spec/support/shared_examples/requests/api/debian_packages_shared_examples.rb index de7032450a5..14a83d2889b 100644 --- a/spec/support/shared_examples/requests/api/debian_packages_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/debian_packages_shared_examples.rb @@ -24,7 +24,7 @@ RSpec.shared_examples 'Debian packages upload request' do |status, body = nil| if status == :created it 'creates package files', :aggregate_failures do expect(::Packages::Debian::FindOrCreateIncomingService).to receive(:new).with(container, user).and_call_original - expect(::Packages::Debian::CreatePackageFileService).to receive(:new).with(be_a(Packages::Package), be_an(Hash)).and_call_original + expect(::Packages::Debian::CreatePackageFileService).to receive(:new).with(package: be_a(Packages::Package), current_user: be_an(User), params: be_an(Hash)).and_call_original if file_name.end_with? '.changes' expect(::Packages::Debian::ProcessChangesWorker).to receive(:perform_async) diff --git a/spec/workers/bulk_imports/export_request_worker_spec.rb b/spec/workers/bulk_imports/export_request_worker_spec.rb index bd9fce7935c..597eed3a9b9 100644 --- a/spec/workers/bulk_imports/export_request_worker_spec.rb +++ b/spec/workers/bulk_imports/export_request_worker_spec.rb @@ -32,30 +32,59 @@ RSpec.describe BulkImports::ExportRequestWorker do end context 'when network error is raised' do - it 'logs export failure and marks entity as failed' do - expect_next_instance_of(BulkImports::Clients::HTTP) do |client| - expect(client).to receive(:post).and_raise(BulkImports::NetworkError, 'Export error').twice + let(:exception) { BulkImports::NetworkError.new('Export error') } + + before do + allow_next_instance_of(BulkImports::Clients::HTTP) do |client| + allow(client).to receive(:post).and_raise(exception).twice end + end - expect(Gitlab::Import::Logger).to receive(:error).with( - hash_including( - 'bulk_import_entity_id' => entity.id, - 'pipeline_class' => 'ExportRequestWorker', - 'exception_class' => 'BulkImports::NetworkError', - 'exception_message' => 'Export error', - 'correlation_id_value' => anything, - 'bulk_import_id' => bulk_import.id, - 'bulk_import_entity_type' => entity.source_type, - 'importer' => 'gitlab_migration' - ) - ).twice + context 'when error is retriable' do + it 'logs retry request and reenqueues' do + allow(exception).to receive(:retriable?).twice.and_return(true) - perform_multiple(job_args) + expect(Gitlab::Import::Logger).to receive(:error).with( + hash_including( + 'bulk_import_entity_id' => entity.id, + 'pipeline_class' => 'ExportRequestWorker', + 'exception_class' => 'BulkImports::NetworkError', + 'exception_message' => 'Export error', + 'bulk_import_id' => bulk_import.id, + 'bulk_import_entity_type' => entity.source_type, + 'importer' => 'gitlab_migration', + 'message' => 'Retrying export request' + ) + ).twice + + expect(described_class).to receive(:perform_in).twice.with(2.seconds, entity.id) + + perform_multiple(job_args) + end + end - failure = entity.failures.last + context 'when error is not retriable' do + it 'logs export failure and marks entity as failed' do + expect(Gitlab::Import::Logger).to receive(:error).with( + hash_including( + 'bulk_import_entity_id' => entity.id, + 'pipeline_class' => 'ExportRequestWorker', + 'exception_class' => 'BulkImports::NetworkError', + 'exception_message' => 'Export error', + 'correlation_id_value' => anything, + 'bulk_import_id' => bulk_import.id, + 'bulk_import_entity_type' => entity.source_type, + 'importer' => 'gitlab_migration' + ) + ).twice - expect(failure.pipeline_class).to eq('ExportRequestWorker') - expect(failure.exception_message).to eq('Export error') + perform_multiple(job_args) + + failure = entity.failures.last + + expect(failure.pipeline_class).to eq('ExportRequestWorker') + expect(failure.exception_message).to eq('Export error') + end end end diff --git a/spec/workers/every_sidekiq_worker_spec.rb b/spec/workers/every_sidekiq_worker_spec.rb index 0c2ed26459a..322f516fbeb 100644 --- a/spec/workers/every_sidekiq_worker_spec.rb +++ b/spec/workers/every_sidekiq_worker_spec.rb @@ -225,7 +225,6 @@ RSpec.describe 'Every Sidekiq worker' do 'Environments::CanaryIngress::UpdateWorker' => false, 'Epics::UpdateEpicsDatesWorker' => 3, 'ErrorTrackingIssueLinkWorker' => 3, - 'Experiments::RecordConversionEventWorker' => 3, 'ExportCsvWorker' => 3, 'ExternalServiceReactiveCachingWorker' => 3, 'FileHookWorker' => false, diff --git a/spec/workers/experiments/record_conversion_event_worker_spec.rb b/spec/workers/experiments/record_conversion_event_worker_spec.rb deleted file mode 100644 index 05e4ebc13ba..00000000000 --- a/spec/workers/experiments/record_conversion_event_worker_spec.rb +++ /dev/null @@ -1,35 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Experiments::RecordConversionEventWorker, '#perform' do - subject(:perform) { described_class.new.perform(:experiment_key, 1234) } - - before do - stub_experiment(experiment_key: experiment_active) - end - - context 'when the experiment is active' do - let(:experiment_active) { true } - - include_examples 'an idempotent worker' do - subject { perform } - - it 'records the event' do - expect(Experiment).to receive(:record_conversion_event).with(:experiment_key, 1234) - - perform - end - end - end - - context 'when the experiment is not active' do - let(:experiment_active) { false } - - it 'records the event' do - expect(Experiment).not_to receive(:record_conversion_event) - - perform - end - end -end |