From ead6ab29b07201cd30488aa21b88d3e9c270046f Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Tue, 23 Nov 2021 12:14:32 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- .../mixins/ready_to_merge.js | 4 +- .../page_bundles/merge_requests.scss | 2 +- app/models/integrations/base_issue_tracker.rb | 2 +- app/models/integrations/jira.rb | 54 ++++---- app/services/system_note_service.rb | 12 +- .../system_notes/issuables_service.rb | 53 ++++--- ...t_nothing_merge_click_new_file_monthly.yml | 2 +- ...et_nothing_merge_click_new_file_weekly.yml | 2 +- config/sidekiq_queues.yml | 2 + .../testing_guide/frontend_testing.md | 15 +- jest.config.integration.js | 1 + locale/gitlab.pot | 6 +- package.json | 1 - spec/frontend/{ => __helpers__}/matchers.js | 0 .../{ => __helpers__}/matchers_spec.js | 0 .../mocks}/axios_utils.js | 0 .../frontend/__helpers__/shared_test_setup.js | 90 ++++++++++++ spec/frontend/mocks/mocks_helper.js | 58 -------- spec/frontend/mocks/mocks_helper_spec.js | 131 ------------------ spec/frontend/test_setup.js | 98 +------------ .../test_helpers/setup/index.js | 2 +- .../test_helpers/setup/setup_globals.js | 11 +- spec/models/integrations/jira_spec.rb | 2 +- spec/services/system_note_service_spec.rb | 18 +-- .../system_notes/issuables_service_spec.rb | 42 +++--- .../cross-database-modification-allowlist.yml | 1 - yarn.lock | 18 --- 27 files changed, 211 insertions(+), 416 deletions(-) rename spec/frontend/{ => __helpers__}/matchers.js (100%) rename spec/frontend/{ => __helpers__}/matchers_spec.js (100%) rename spec/frontend/{mocks/ce/lib/utils => __helpers__/mocks}/axios_utils.js (100%) create mode 100644 spec/frontend/__helpers__/shared_test_setup.js delete mode 100644 spec/frontend/mocks/mocks_helper.js delete mode 100644 spec/frontend/mocks/mocks_helper_spec.js diff --git a/app/assets/javascripts/vue_merge_request_widget/mixins/ready_to_merge.js b/app/assets/javascripts/vue_merge_request_widget/mixins/ready_to_merge.js index 83789f10285..fa618756bb5 100644 --- a/app/assets/javascripts/vue_merge_request_widget/mixins/ready_to_merge.js +++ b/app/assets/javascripts/vue_merge_request_widget/mixins/ready_to_merge.js @@ -1,6 +1,8 @@ import { __ } from '~/locale'; -export const MERGE_DISABLED_TEXT = __('You can only merge once the items above are resolved.'); +export const MERGE_DISABLED_TEXT = __( + 'Merge blocked: all merge request dependencies must be merged or closed.', +); export const MERGE_DISABLED_SKIPPED_PIPELINE_TEXT = __( "Merge blocked: pipeline must succeed. It's waiting for a manual job to continue.", ); diff --git a/app/assets/stylesheets/page_bundles/merge_requests.scss b/app/assets/stylesheets/page_bundles/merge_requests.scss index 7d1230b0225..02113fe8b58 100644 --- a/app/assets/stylesheets/page_bundles/merge_requests.scss +++ b/app/assets/stylesheets/page_bundles/merge_requests.scss @@ -45,7 +45,7 @@ top: calc(#{$top-pos} + var(--system-header-height, 0px) + var(--performance-bar-height, 0px)); // stylelint-disable-next-line length-zero-no-unit max-height: calc(100vh - #{$top-pos} - var(--system-header-height, 0px) - var(--performance-bar-height, 0px) - var(--review-bar-height, 0px)); - z-index: 202; + z-index: 205; .drag-handle { bottom: 16px; diff --git a/app/models/integrations/base_issue_tracker.rb b/app/models/integrations/base_issue_tracker.rb index 3fd67205e92..42a6a3a19c8 100644 --- a/app/models/integrations/base_issue_tracker.rb +++ b/app/models/integrations/base_issue_tracker.rb @@ -128,7 +128,7 @@ module Integrations false end - def create_cross_reference_note(mentioned, noteable, author) + def create_cross_reference_note(external_issue, mentioned_in, author) # implement inside child end diff --git a/app/models/integrations/jira.rb b/app/models/integrations/jira.rb index 42c291abf55..d46299de1be 100644 --- a/app/models/integrations/jira.rb +++ b/app/models/integrations/jira.rb @@ -234,19 +234,19 @@ module Integrations end override :create_cross_reference_note - def create_cross_reference_note(mentioned, noteable, author) - unless can_cross_reference?(noteable) - return s_("JiraService|Events for %{noteable_model_name} are disabled.") % { noteable_model_name: noteable.model_name.plural.humanize(capitalize: false) } + def create_cross_reference_note(external_issue, mentioned_in, author) + unless can_cross_reference?(mentioned_in) + return s_("JiraService|Events for %{noteable_model_name} are disabled.") % { noteable_model_name: mentioned_in.model_name.plural.humanize(capitalize: false) } end - jira_issue = find_issue(mentioned.id) + jira_issue = find_issue(external_issue.id) return unless jira_issue.present? - noteable_id = noteable.respond_to?(:iid) ? noteable.iid : noteable.id - noteable_type = noteable_name(noteable) - entity_url = build_entity_url(noteable_type, noteable_id) - entity_meta = build_entity_meta(noteable) + mentioned_in_id = mentioned_in.respond_to?(:iid) ? mentioned_in.iid : mentioned_in.id + mentioned_in_type = mentionable_name(mentioned_in) + entity_url = build_entity_url(mentioned_in_type, mentioned_in_id) + entity_meta = build_entity_meta(mentioned_in) data = { user: { @@ -259,9 +259,9 @@ module Integrations }, entity: { id: entity_meta[:id], - name: noteable_type.humanize.downcase, + name: mentioned_in_type.humanize.downcase, url: entity_url, - title: noteable.title, + title: mentioned_in.title, description: entity_meta[:description], branch: entity_meta[:branch] } @@ -302,11 +302,11 @@ module Integrations private - def branch_name(noteable) + def branch_name(commit) if Feature.enabled?(:jira_use_first_ref_by_oid, project, default_enabled: :yaml) - noteable.first_ref_by_oid(project.repository) + commit.first_ref_by_oid(project.repository) else - noteable.ref_names(project.repository).first + commit.ref_names(project.repository).first end end @@ -316,8 +316,8 @@ module Integrations end end - def can_cross_reference?(noteable) - case noteable + def can_cross_reference?(mentioned_in) + case mentioned_in when Commit then commit_events when MergeRequest then merge_requests_events else true @@ -487,36 +487,36 @@ module Integrations "#{Settings.gitlab.base_url.chomp("/")}#{resource}" end - def build_entity_url(noteable_type, entity_id) + def build_entity_url(entity_type, entity_id) polymorphic_url( [ self.project, - noteable_type.to_sym + entity_type.to_sym ], id: entity_id, host: Settings.gitlab.base_url ) end - def build_entity_meta(noteable) - if noteable.is_a?(Commit) + def build_entity_meta(entity) + if entity.is_a?(Commit) { - id: noteable.short_id, - description: noteable.safe_message, - branch: branch_name(noteable) + id: entity.short_id, + description: entity.safe_message, + branch: branch_name(entity) } - elsif noteable.is_a?(MergeRequest) + elsif entity.is_a?(MergeRequest) { - id: noteable.to_reference, - branch: noteable.source_branch + id: entity.to_reference, + branch: entity.source_branch } else {} end end - def noteable_name(noteable) - name = noteable.model_name.singular + def mentionable_name(mentionable) + name = mentionable.model_name.singular # ProjectSnippet inherits from Snippet class so it causes # routing error building the URL. diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb index dc5cf0fe554..e98dfb872fe 100644 --- a/app/services/system_note_service.rb +++ b/app/services/system_note_service.rb @@ -213,12 +213,12 @@ module SystemNoteService ::SystemNotes::MergeRequestsService.new(noteable: issue, project: project, author: author).new_merge_request(merge_request) end - def cross_reference(noteable, mentioner, author) - ::SystemNotes::IssuablesService.new(noteable: noteable, author: author).cross_reference(mentioner) + def cross_reference(mentioned, mentioned_in, author) + ::SystemNotes::IssuablesService.new(noteable: mentioned, author: author).cross_reference(mentioned_in) end - def cross_reference_exists?(noteable, mentioner) - ::SystemNotes::IssuablesService.new(noteable: noteable).cross_reference_exists?(mentioner) + def cross_reference_exists?(mentioned, mentioned_in) + ::SystemNotes::IssuablesService.new(noteable: mentioned).cross_reference_exists?(mentioned_in) end def change_task_status(noteable, project, author, new_task) @@ -249,8 +249,8 @@ module SystemNoteService ::SystemNotes::IssuablesService.new(noteable: issuable, project: issuable.project, author: author).discussion_lock end - def cross_reference_disallowed?(noteable, mentioner) - ::SystemNotes::IssuablesService.new(noteable: noteable).cross_reference_disallowed?(mentioner) + def cross_reference_disallowed?(mentioned, mentioned_in) + ::SystemNotes::IssuablesService.new(noteable: mentioned).cross_reference_disallowed?(mentioned_in) end def zoom_link_added(issue, project, author) diff --git a/app/services/system_notes/issuables_service.rb b/app/services/system_notes/issuables_service.rb index 94629ae7609..92540f957c8 100644 --- a/app/services/system_notes/issuables_service.rb +++ b/app/services/system_notes/issuables_service.rb @@ -154,9 +154,8 @@ module SystemNotes create_note(NoteSummary.new(noteable, project, author, body, action: 'description')) end - # Called when a Mentionable references a Noteable - # - # mentioner - Mentionable object + # Called when a Mentionable (the `mentioned_in`) references another Mentionable (the `mentioned`, + # passed to this service as `noteable`). # # Example Note text: # @@ -168,19 +167,20 @@ module SystemNotes # # See cross_reference_note_content. # - # Returns the created Note object - def cross_reference(mentioner) - return if cross_reference_disallowed?(mentioner) + # @param mentioned_in [Mentionable] + # @return [Note] + def cross_reference(mentioned_in) + return if cross_reference_disallowed?(mentioned_in) - gfm_reference = mentioner.gfm_reference(noteable.project || noteable.group) + gfm_reference = mentioned_in.gfm_reference(noteable.project || noteable.group) body = cross_reference_note_content(gfm_reference) if noteable.is_a?(ExternalIssue) Integrations::CreateExternalCrossReferenceWorker.perform_async( noteable.project_id, noteable.id, - mentioner.class.name, - mentioner.id, + mentioned_in.class.name, + mentioned_in.id, author.id ) else @@ -195,15 +195,14 @@ module SystemNotes # in a merge request. Additionally, it prevents the creation of references to # external issues (which would fail). # - # mentioner - Mentionable object - # - # Returns Boolean - def cross_reference_disallowed?(mentioner) + # @param mentioned_in [Mentionable] + # @return [Boolean] + def cross_reference_disallowed?(mentioned_in) return true if noteable.is_a?(ExternalIssue) && !noteable.project&.external_references_supported? - return false unless mentioner.is_a?(MergeRequest) + return false unless mentioned_in.is_a?(MergeRequest) return false unless noteable.is_a?(Commit) - mentioner.commits.include?(noteable) + mentioned_in.commits.include?(noteable) end # Called when the status of a Task has changed @@ -309,19 +308,19 @@ module SystemNotes create_resource_state_event(status: status, mentionable_source: source) end - # Check if a cross reference to a noteable from a mentioner already exists + # Check if a cross reference to a Mentionable from the `mentioned_in` Mentionable + # already exists. # # This method is used to prevent multiple notes being created for a mention - # when a issue is updated, for example. The method also calls notes_for_mentioner - # to check if the mentioner is a commit, and return matches only on commit hash + # when a issue is updated, for example. The method also calls `existing_mentions_for` + # to check if the mention is in a commit, and return matches only on commit hash # instead of project + commit, to avoid repeated mentions from forks. # - # mentioner - Mentionable object - # - # Returns Boolean - def cross_reference_exists?(mentioner) + # @param mentioned_in [Mentionable] + # @return [Boolean] + def cross_reference_exists?(mentioned_in) notes = noteable.notes.system - notes_for_mentioner(mentioner, noteable, notes).exists? + existing_mentions_for(mentioned_in, noteable, notes).exists? end # Called when a Noteable has been marked as a duplicate of another Issue @@ -398,12 +397,12 @@ module SystemNotes "#{self.class.cross_reference_note_prefix}#{gfm_reference}" end - def notes_for_mentioner(mentioner, noteable, notes) - if mentioner.is_a?(Commit) - text = "#{self.class.cross_reference_note_prefix}%#{mentioner.to_reference(nil)}" + def existing_mentions_for(mentioned_in, noteable, notes) + if mentioned_in.is_a?(Commit) + text = "#{self.class.cross_reference_note_prefix}%#{mentioned_in.to_reference(nil)}" notes.like_note_or_capitalized_note(text) else - gfm_reference = mentioner.gfm_reference(noteable.project || noteable.group) + gfm_reference = mentioned_in.gfm_reference(noteable.project || noteable.group) text = cross_reference_note_content(gfm_reference) notes.for_note_or_capitalized_note(text) end diff --git a/config/metrics/counts_28d/20211104154357_i_code_review_widget_nothing_merge_click_new_file_monthly.yml b/config/metrics/counts_28d/20211104154357_i_code_review_widget_nothing_merge_click_new_file_monthly.yml index 1aa0edf60e6..928dd24b701 100644 --- a/config/metrics/counts_28d/20211104154357_i_code_review_widget_nothing_merge_click_new_file_monthly.yml +++ b/config/metrics/counts_28d/20211104154357_i_code_review_widget_nothing_merge_click_new_file_monthly.yml @@ -8,7 +8,7 @@ product_category: code_review value_type: number status: active milestone: '14.5' -introduced_by_url: +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/73762 time_frame: 28d data_source: redis_hll instrumentation_class: RedisHLLMetric diff --git a/config/metrics/counts_7d/20211104154352_i_code_review_widget_nothing_merge_click_new_file_weekly.yml b/config/metrics/counts_7d/20211104154352_i_code_review_widget_nothing_merge_click_new_file_weekly.yml index 9f8ae151a80..3a647c52f71 100644 --- a/config/metrics/counts_7d/20211104154352_i_code_review_widget_nothing_merge_click_new_file_weekly.yml +++ b/config/metrics/counts_7d/20211104154352_i_code_review_widget_nothing_merge_click_new_file_weekly.yml @@ -8,7 +8,7 @@ product_category: code_review value_type: number status: active milestone: '14.5' -introduced_by_url: +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/73762 time_frame: 7d data_source: redis_hll instrumentation_class: RedisHLLMetric diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index 7d4af9abea2..7e6bcc35e60 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -73,6 +73,8 @@ - 1 - - ci_delete_objects - 1 +- - ci_upstream_projects_subscriptions_cleanup + - 1 - - container_repository - 1 - - create_commit_signature diff --git a/doc/development/testing_guide/frontend_testing.md b/doc/development/testing_guide/frontend_testing.md index 3096386d7c3..574b67bf9f5 100644 --- a/doc/development/testing_guide/frontend_testing.md +++ b/doc/development/testing_guide/frontend_testing.md @@ -716,16 +716,19 @@ Jest supports [manual module mocks](https://jestjs.io/docs/manual-mocks) by plac If a manual mock is needed for a `node_modules` package, use the `spec/frontend/__mocks__` folder. Here's an example of a [Jest mock for the package `monaco-editor`](https://gitlab.com/gitlab-org/gitlab/-/blob/b7f914cddec9fc5971238cdf12766e79fa1629d7/spec/frontend/__mocks__/monaco-editor/index.js#L1). -If a manual mock is needed for a CE module, place it in `spec/frontend/mocks/ce`. +If a manual mock is needed for a CE module, place the implementation in +`spec/frontend/__helpers__/mocks` and add a line to the `frontend/test_setup` +(or the `frontend/shared_test_setup`) that looks something like: -- Files in `spec/frontend/mocks/ce` mocks the corresponding CE module from `app/assets/javascripts`, mirroring the source module's path. - - Example: `spec/frontend/mocks/ce/lib/utils/axios_utils` mocks the module `~/lib/utils/axios_utils`. -- We don't support mocking EE modules yet. -- If a mock is found for which a source module doesn't exist, the test suite fails. 'Virtual' mocks, or mocks that don't have a 1-to-1 association with a source module, are not supported yet. +```javascript +// "~/lib/utils/axios_utils" is the path to the real module +// "helpers/mocks/axios_utils" is the path to the mocked implementation +jest.mock('~/lib/utils/axios_utils', () => jest.requireActual('helpers/mocks/axios_utils')); +``` #### Manual mock examples -- [`mocks/axios_utils`](https://gitlab.com/gitlab-org/gitlab/-/blob/bd20aeb64c4eed117831556c54b40ff4aee9bfd1/spec/frontend/mocks/ce/lib/utils/axios_utils.js#L1) - +- [`__helpers__/mocks/axios_utils`](https://gitlab.com/gitlab-org/gitlab/-/blob/a50edd12b3b1531389624086b6381a042c8143ef/spec/frontend/__helpers__/mocks/axios_utils.js#L1) - This mock is helpful because we don't want any unmocked requests to pass any tests. Also, we are able to inject some test helpers such as `axios.waitForAll`. - [`__mocks__/mousetrap/index.js`](https://gitlab.com/gitlab-org/gitlab/-/blob/cd4c086d894226445be9d18294a060ba46572435/spec/frontend/__mocks__/mousetrap/index.js#L1) - This mock is helpful because the module itself uses AMD format which webpack understands, but is incompatible with the jest environment. This mock doesn't remove diff --git a/jest.config.integration.js b/jest.config.integration.js index 184f88779c8..df25c2b247b 100644 --- a/jest.config.integration.js +++ b/jest.config.integration.js @@ -24,4 +24,5 @@ module.exports = { '^jh_else_ce_test_helpers(/.*)$': '/jh/spec/frontend_integration/test_helpers$1', }, }), + timers: 'real', }; diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 765e066463c..3d094383657 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -21737,6 +21737,9 @@ msgstr "" msgid "Merge automatically (%{strategy})" msgstr "" +msgid "Merge blocked: all merge request dependencies must be merged or closed." +msgstr "" + msgid "Merge blocked: new changes were just added." msgstr "" @@ -39602,9 +39605,6 @@ msgstr "" msgid "You can only edit files when you are on a branch" msgstr "" -msgid "You can only merge once the items above are resolved." -msgstr "" - msgid "You can only merge once this merge request is approved." msgstr "" diff --git a/package.json b/package.json index 02a2b43a23c..4f1c12e72cb 100644 --- a/package.json +++ b/package.json @@ -252,7 +252,6 @@ "prosemirror-test-builder": "^1.0.5", "purgecss": "^4.0.3", "purgecss-from-html": "^4.0.3", - "readdir-enhanced": "^2.2.4", "sass": "^1.32.12", "timezone-mock": "^1.0.8", "vue-jest": "4.0.1", diff --git a/spec/frontend/matchers.js b/spec/frontend/__helpers__/matchers.js similarity index 100% rename from spec/frontend/matchers.js rename to spec/frontend/__helpers__/matchers.js diff --git a/spec/frontend/matchers_spec.js b/spec/frontend/__helpers__/matchers_spec.js similarity index 100% rename from spec/frontend/matchers_spec.js rename to spec/frontend/__helpers__/matchers_spec.js diff --git a/spec/frontend/mocks/ce/lib/utils/axios_utils.js b/spec/frontend/__helpers__/mocks/axios_utils.js similarity index 100% rename from spec/frontend/mocks/ce/lib/utils/axios_utils.js rename to spec/frontend/__helpers__/mocks/axios_utils.js diff --git a/spec/frontend/__helpers__/shared_test_setup.js b/spec/frontend/__helpers__/shared_test_setup.js new file mode 100644 index 00000000000..03389e16b65 --- /dev/null +++ b/spec/frontend/__helpers__/shared_test_setup.js @@ -0,0 +1,90 @@ +/* Common setup for both unit and integration test environments */ +import { config as testUtilsConfig } from '@vue/test-utils'; +import * as jqueryMatchers from 'custom-jquery-matchers'; +import Vue from 'vue'; +import 'jquery'; +import Translate from '~/vue_shared/translate'; +import setWindowLocation from './set_window_location_helper'; +import { setGlobalDateToFakeDate } from './fake_date'; +import { loadHTMLFixture, setHTMLFixture } from './fixtures'; +import { TEST_HOST } from './test_constants'; +import customMatchers from './matchers'; + +import './dom_shims'; +import './jquery'; +import '~/commons/bootstrap'; + +// This module has some fairly decent visual test coverage in it's own repository. +jest.mock('@gitlab/favicon-overlay'); + +process.on('unhandledRejection', global.promiseRejectionHandler); + +// Fake the `Date` for the rest of the jest spec runtime environment. +// https://gitlab.com/gitlab-org/gitlab/-/merge_requests/39496#note_503084332 +setGlobalDateToFakeDate(); + +Vue.config.devtools = false; +Vue.config.productionTip = false; + +Vue.use(Translate); + +// convenience wrapper for migration from Karma +Object.assign(global, { + loadFixtures: loadHTMLFixture, + setFixtures: setHTMLFixture, +}); + +const JQUERY_MATCHERS_TO_EXCLUDE = ['toHaveLength', 'toExist']; + +// custom-jquery-matchers was written for an old Jest version, we need to make it compatible +Object.entries(jqueryMatchers).forEach(([matcherName, matcherFactory]) => { + // Exclude these jQuery matchers + if (JQUERY_MATCHERS_TO_EXCLUDE.includes(matcherName)) { + return; + } + + expect.extend({ + [matcherName]: matcherFactory().compare, + }); +}); + +expect.extend(customMatchers); + +testUtilsConfig.deprecationWarningHandler = (method, message) => { + const ALLOWED_DEPRECATED_METHODS = [ + // https://gitlab.com/gitlab-org/gitlab/-/issues/295679 + 'finding components with `find` or `get`', + + // https://gitlab.com/gitlab-org/gitlab/-/issues/295680 + 'finding components with `findAll`', + ]; + if (!ALLOWED_DEPRECATED_METHODS.includes(method)) { + global.console.error(message); + } +}; + +Object.assign(global, { + requestIdleCallback(cb) { + const start = Date.now(); + return setTimeout(() => { + cb({ + didTimeout: false, + timeRemaining: () => Math.max(0, 50 - (Date.now() - start)), + }); + }); + }, + cancelIdleCallback(id) { + clearTimeout(id); + }, +}); + +beforeEach(() => { + // make sure that each test actually tests something + // see https://jestjs.io/docs/en/expect#expecthasassertions + expect.hasAssertions(); + + // Reset the mocked window.location. This ensures tests don't interfere with + // each other, and removes the need to tidy up if it was changed for a given + // test. + setWindowLocation(TEST_HOST); +}); diff --git a/spec/frontend/mocks/mocks_helper.js b/spec/frontend/mocks/mocks_helper.js deleted file mode 100644 index 295483cd64c..00000000000 --- a/spec/frontend/mocks/mocks_helper.js +++ /dev/null @@ -1,58 +0,0 @@ -/** - * @module - * - * This module implements auto-injected manual mocks that are cleaner than Jest's approach. - * - * See https://docs.gitlab.com/ee/development/testing_guide/frontend_testing.html - */ - -import fs from 'fs'; -import path from 'path'; - -import readdir from 'readdir-enhanced'; - -const MAX_DEPTH = 20; -const prefixMap = [ - // E.g. the mock ce/foo/bar maps to require path ~/foo/bar - { mocksRoot: 'ce', requirePrefix: '~' }, - // { mocksRoot: 'ee', requirePrefix: 'ee' }, // We'll deal with EE-specific mocks later - // { mocksRoot: 'virtual', requirePrefix: '' }, // We'll deal with virtual mocks later -]; - -const mockFileFilter = (stats) => stats.isFile() && stats.path.endsWith('.js'); - -const getMockFiles = (root) => readdir.sync(root, { deep: MAX_DEPTH, filter: mockFileFilter }); - -// Function that performs setting a mock. This has to be overridden by the unit test, because -// jest.setMock can't be overwritten across files. -// Use require() because jest.setMock expects the CommonJS exports object -const defaultSetMock = (srcPath, mockPath) => - jest.mock(srcPath, () => jest.requireActual(mockPath)); - -export const setupManualMocks = function setupManualMocks(setMock = defaultSetMock) { - prefixMap.forEach(({ mocksRoot, requirePrefix }) => { - const mocksRootAbsolute = path.join(__dirname, mocksRoot); - if (!fs.existsSync(mocksRootAbsolute)) { - return; - } - - getMockFiles(path.join(__dirname, mocksRoot)).forEach((mockPath) => { - const mockPathNoExt = mockPath.substring(0, mockPath.length - path.extname(mockPath).length); - const sourcePath = path.join(requirePrefix, mockPathNoExt); - const mockPathRelative = `./${path.join(mocksRoot, mockPathNoExt)}`; - - try { - setMock(sourcePath, mockPathRelative); - } catch (e) { - if (e.message.includes('Could not locate module')) { - // The corresponding mocked module doesn't exist. Raise a better error. - // Eventualy, we may support virtual mocks (mocks whose path doesn't directly correspond - // to a module, like with the `ee_else_ce` prefix). - throw new Error( - `A manual mock was defined for module ${sourcePath}, but the module doesn't exist!`, - ); - } - } - }); - }); -}; diff --git a/spec/frontend/mocks/mocks_helper_spec.js b/spec/frontend/mocks/mocks_helper_spec.js deleted file mode 100644 index 0abe5c6b949..00000000000 --- a/spec/frontend/mocks/mocks_helper_spec.js +++ /dev/null @@ -1,131 +0,0 @@ -/* eslint-disable global-require */ - -import path from 'path'; - -import axios from '~/lib/utils/axios_utils'; - -const absPath = path.join.bind(null, __dirname); - -jest.mock('fs'); -jest.mock('readdir-enhanced'); - -describe('mocks_helper.js', () => { - let setupManualMocks; - const setMock = jest.fn().mockName('setMock'); - let fs; - let readdir; - - beforeAll(() => { - jest.resetModules(); - jest.setMock = jest.fn().mockName('jest.setMock'); - fs = require('fs'); - readdir = require('readdir-enhanced'); - - // We need to provide setupManualMocks with a mock function that pretends to do the setup of - // the mock. This is because we can't mock jest.setMock across files. - setupManualMocks = () => require('./mocks_helper').setupManualMocks(setMock); - }); - - afterEach(() => { - fs.existsSync.mockReset(); - readdir.sync.mockReset(); - setMock.mockReset(); - }); - - it('enumerates through mock file roots', () => { - setupManualMocks(); - expect(fs.existsSync).toHaveBeenCalledTimes(1); - expect(fs.existsSync).toHaveBeenNthCalledWith(1, absPath('ce')); - - expect(readdir.sync).toHaveBeenCalledTimes(0); - }); - - it("doesn't traverse the directory tree infinitely", () => { - fs.existsSync.mockReturnValue(true); - readdir.sync.mockReturnValue([]); - setupManualMocks(); - - const readdirSpy = readdir.sync; - expect(readdirSpy).toHaveBeenCalled(); - readdirSpy.mock.calls.forEach((call) => { - expect(call[1].deep).toBeLessThan(100); - }); - }); - - it('sets up mocks for CE (the ~/ prefix)', () => { - fs.existsSync.mockImplementation((root) => root.endsWith('ce')); - readdir.sync.mockReturnValue(['root.js', 'lib/utils/util.js']); - setupManualMocks(); - - expect(readdir.sync).toHaveBeenCalledTimes(1); - expect(readdir.sync.mock.calls[0][0]).toBe(absPath('ce')); - - expect(setMock).toHaveBeenCalledTimes(2); - expect(setMock).toHaveBeenNthCalledWith(1, '~/root', './ce/root'); - expect(setMock).toHaveBeenNthCalledWith(2, '~/lib/utils/util', './ce/lib/utils/util'); - }); - - it('sets up mocks for all roots', () => { - const files = { - [absPath('ce')]: ['root', 'lib/utils/util'], - [absPath('node')]: ['jquery', '@babel/core'], - }; - - fs.existsSync.mockReturnValue(true); - readdir.sync.mockImplementation((root) => files[root]); - setupManualMocks(); - - expect(readdir.sync).toHaveBeenCalledTimes(1); - expect(readdir.sync.mock.calls[0][0]).toBe(absPath('ce')); - - expect(setMock).toHaveBeenCalledTimes(2); - expect(setMock).toHaveBeenNthCalledWith(1, '~/root', './ce/root'); - expect(setMock).toHaveBeenNthCalledWith(2, '~/lib/utils/util', './ce/lib/utils/util'); - }); - - it('fails when given a virtual mock', () => { - fs.existsSync.mockImplementation((p) => p.endsWith('ce')); - readdir.sync.mockReturnValue(['virtual', 'shouldntBeImported']); - setMock.mockImplementation(() => { - throw new Error('Could not locate module'); - }); - - expect(setupManualMocks).toThrow( - new Error("A manual mock was defined for module ~/virtual, but the module doesn't exist!"), - ); - - expect(readdir.sync).toHaveBeenCalledTimes(1); - expect(readdir.sync.mock.calls[0][0]).toBe(absPath('ce')); - }); - - describe('auto-injection', () => { - it('handles ambiguous paths', () => { - jest.isolateModules(() => { - const axios2 = require('../../../app/assets/javascripts/lib/utils/axios_utils').default; - expect(axios2.isMock).toBe(true); - }); - }); - - it('survives jest.isolateModules()', (done) => { - jest.isolateModules(() => { - const axios2 = require('~/lib/utils/axios_utils').default; - expect(axios2.isMock).toBe(true); - done(); - }); - }); - - it('can be unmocked and remocked', () => { - jest.dontMock('~/lib/utils/axios_utils'); - jest.resetModules(); - const axios2 = require('~/lib/utils/axios_utils').default; - expect(axios2).not.toBe(axios); - expect(axios2.isMock).toBeUndefined(); - - jest.doMock('~/lib/utils/axios_utils'); - jest.resetModules(); - const axios3 = require('~/lib/utils/axios_utils').default; - expect(axios3).not.toBe(axios2); - expect(axios3.isMock).toBe(true); - }); - }); -}); diff --git a/spec/frontend/test_setup.js b/spec/frontend/test_setup.js index 40f68c6385f..4fe51db8412 100644 --- a/spec/frontend/test_setup.js +++ b/spec/frontend/test_setup.js @@ -1,30 +1,10 @@ -import { config as testUtilsConfig } from '@vue/test-utils'; -import * as jqueryMatchers from 'custom-jquery-matchers'; -import Vue from 'vue'; -import 'jquery'; -import { setGlobalDateToFakeDate } from 'helpers/fake_date'; -import setWindowLocation from 'helpers/set_window_location_helper'; -import { TEST_HOST } from 'helpers/test_constants'; -import Translate from '~/vue_shared/translate'; -import { loadHTMLFixture, setHTMLFixture } from './__helpers__/fixtures'; -import { initializeTestTimeout } from './__helpers__/timeout'; -import customMatchers from './matchers'; -import { setupManualMocks } from './mocks/mocks_helper'; +/* Setup for unit test environment */ +import 'helpers/shared_test_setup'; +import { initializeTestTimeout } from 'helpers/timeout'; -import './__helpers__/dom_shims'; -import './__helpers__/jquery'; -import '~/commons/bootstrap'; +jest.mock('~/lib/utils/axios_utils', () => jest.requireActual('helpers/mocks/axios_utils')); -// This module has some fairly decent visual test coverage in it's own repository. -jest.mock('@gitlab/favicon-overlay'); - -process.on('unhandledRejection', global.promiseRejectionHandler); - -setupManualMocks(); - -// Fake the `Date` for the rest of the jest spec runtime environment. -// https://gitlab.com/gitlab-org/gitlab/-/merge_requests/39496#note_503084332 -setGlobalDateToFakeDate(); +initializeTestTimeout(process.env.CI ? 6000 : 500); afterEach(() => // give Promises a bit more time so they fail the right test @@ -33,71 +13,3 @@ afterEach(() => jest.runOnlyPendingTimers(); }), ); - -initializeTestTimeout(process.env.CI ? 6000 : 500); - -Vue.config.devtools = false; -Vue.config.productionTip = false; - -Vue.use(Translate); - -// convenience wrapper for migration from Karma -Object.assign(global, { - loadFixtures: loadHTMLFixture, - setFixtures: setHTMLFixture, -}); - -const JQUERY_MATCHERS_TO_EXCLUDE = ['toHaveLength', 'toExist']; - -// custom-jquery-matchers was written for an old Jest version, we need to make it compatible -Object.entries(jqueryMatchers).forEach(([matcherName, matcherFactory]) => { - // Exclude these jQuery matchers - if (JQUERY_MATCHERS_TO_EXCLUDE.includes(matcherName)) { - return; - } - - expect.extend({ - [matcherName]: matcherFactory().compare, - }); -}); - -expect.extend(customMatchers); - -testUtilsConfig.deprecationWarningHandler = (method, message) => { - const ALLOWED_DEPRECATED_METHODS = [ - // https://gitlab.com/gitlab-org/gitlab/-/issues/295679 - 'finding components with `find` or `get`', - - // https://gitlab.com/gitlab-org/gitlab/-/issues/295680 - 'finding components with `findAll`', - ]; - if (!ALLOWED_DEPRECATED_METHODS.includes(method)) { - global.console.error(message); - } -}; - -Object.assign(global, { - requestIdleCallback(cb) { - const start = Date.now(); - return setTimeout(() => { - cb({ - didTimeout: false, - timeRemaining: () => Math.max(0, 50 - (Date.now() - start)), - }); - }); - }, - cancelIdleCallback(id) { - clearTimeout(id); - }, -}); - -beforeEach(() => { - // make sure that each test actually tests something - // see https://jestjs.io/docs/en/expect#expecthasassertions - expect.hasAssertions(); - - // Reset the mocked window.location. This ensures tests don't interfere with - // each other, and removes the need to tidy up if it was changed for a given - // test. - setWindowLocation(TEST_HOST); -}); diff --git a/spec/frontend_integration/test_helpers/setup/index.js b/spec/frontend_integration/test_helpers/setup/index.js index 946ccbec00c..0c16592f2e2 100644 --- a/spec/frontend_integration/test_helpers/setup/index.js +++ b/spec/frontend_integration/test_helpers/setup/index.js @@ -1,4 +1,4 @@ -import '../../../frontend/test_setup'; +import 'helpers/shared_test_setup'; import './setup_globals'; import './setup_axios'; import './setup_serializers'; diff --git a/spec/frontend_integration/test_helpers/setup/setup_globals.js b/spec/frontend_integration/test_helpers/setup/setup_globals.js index b63a9a96372..ac5aeb1dd72 100644 --- a/spec/frontend_integration/test_helpers/setup/setup_globals.js +++ b/spec/frontend_integration/test_helpers/setup/setup_globals.js @@ -1,15 +1,10 @@ -import { setTestTimeout } from 'helpers/timeout'; +import { initializeTestTimeout } from 'helpers/timeout'; + +initializeTestTimeout(process.env.CI ? 20000 : 7000); beforeEach(() => { window.gon = { api_version: 'v4', relative_url_root: '', }; - - setTestTimeout(7000); - jest.useRealTimers(); -}); - -afterEach(() => { - jest.useFakeTimers(); }); diff --git a/spec/models/integrations/jira_spec.rb b/spec/models/integrations/jira_spec.rb index 1d81668f97d..9163a7ef845 100644 --- a/spec/models/integrations/jira_spec.rb +++ b/spec/models/integrations/jira_spec.rb @@ -863,7 +863,7 @@ RSpec.describe Integrations::Jira do subject { jira_integration.create_cross_reference_note(jira_issue, resource, user) } shared_examples 'handles cross-references' do - let(:resource_name) { jira_integration.send(:noteable_name, resource) } + let(:resource_name) { jira_integration.send(:mentionable_name, resource) } let(:resource_url) { jira_integration.send(:build_entity_url, resource_name, resource.to_param) } let(:issue_url) { "#{url}/rest/api/2/issue/JIRA-123" } let(:comment_url) { "#{issue_url}/comment" } diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index ce0122ae301..11c6dbe92e7 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -287,38 +287,38 @@ RSpec.describe SystemNoteService do end describe '.cross_reference' do - let(:mentioner) { double } + let(:mentioned_in) { double } it 'calls IssuableService' do expect_next_instance_of(::SystemNotes::IssuablesService) do |service| - expect(service).to receive(:cross_reference).with(mentioner) + expect(service).to receive(:cross_reference).with(mentioned_in) end - described_class.cross_reference(double, mentioner, double) + described_class.cross_reference(double, mentioned_in, double) end end describe '.cross_reference_disallowed?' do - let(:mentioner) { double } + let(:mentioned_in) { double } it 'calls IssuableService' do expect_next_instance_of(::SystemNotes::IssuablesService) do |service| - expect(service).to receive(:cross_reference_disallowed?).with(mentioner) + expect(service).to receive(:cross_reference_disallowed?).with(mentioned_in) end - described_class.cross_reference_disallowed?(double, mentioner) + described_class.cross_reference_disallowed?(double, mentioned_in) end end describe '.cross_reference_exists?' do - let(:mentioner) { double } + let(:mentioned_in) { double } it 'calls IssuableService' do expect_next_instance_of(::SystemNotes::IssuablesService) do |service| - expect(service).to receive(:cross_reference_exists?).with(mentioner) + expect(service).to receive(:cross_reference_exists?).with(mentioned_in) end - described_class.cross_reference_exists?(double, mentioner) + described_class.cross_reference_exists?(double, mentioned_in) end end diff --git a/spec/services/system_notes/issuables_service_spec.rb b/spec/services/system_notes/issuables_service_spec.rb index fd481aa6ddb..43760e296bc 100644 --- a/spec/services/system_notes/issuables_service_spec.rb +++ b/spec/services/system_notes/issuables_service_spec.rb @@ -274,9 +274,9 @@ RSpec.describe ::SystemNotes::IssuablesService do describe '#cross_reference' do let(:service) { described_class.new(noteable: noteable, author: author) } - let(:mentioner) { create(:issue, project: project) } + let(:mentioned_in) { create(:issue, project: project) } - subject { service.cross_reference(mentioner) } + subject { service.cross_reference(mentioned_in) } it_behaves_like 'a system note' do let(:action) { 'cross_reference' } @@ -314,35 +314,35 @@ RSpec.describe ::SystemNotes::IssuablesService do describe 'note_body' do context 'cross-project' do let(:project2) { create(:project, :repository) } - let(:mentioner) { create(:issue, project: project2) } + let(:mentioned_in) { create(:issue, project: project2) } context 'from Commit' do - let(:mentioner) { project2.repository.commit } + let(:mentioned_in) { project2.repository.commit } it 'references the mentioning commit' do - expect(subject.note).to eq "mentioned in commit #{mentioner.to_reference(project)}" + expect(subject.note).to eq "mentioned in commit #{mentioned_in.to_reference(project)}" end end context 'from non-Commit' do it 'references the mentioning object' do - expect(subject.note).to eq "mentioned in issue #{mentioner.to_reference(project)}" + expect(subject.note).to eq "mentioned in issue #{mentioned_in.to_reference(project)}" end end end context 'within the same project' do context 'from Commit' do - let(:mentioner) { project.repository.commit } + let(:mentioned_in) { project.repository.commit } it 'references the mentioning commit' do - expect(subject.note).to eq "mentioned in commit #{mentioner.to_reference}" + expect(subject.note).to eq "mentioned in commit #{mentioned_in.to_reference}" end end context 'from non-Commit' do it 'references the mentioning object' do - expect(subject.note).to eq "mentioned in issue #{mentioner.to_reference}" + expect(subject.note).to eq "mentioned in issue #{mentioned_in.to_reference}" end end end @@ -350,14 +350,14 @@ RSpec.describe ::SystemNotes::IssuablesService do context 'with external issue' do let(:noteable) { ExternalIssue.new('JIRA-123', project) } - let(:mentioner) { project.commit } + let(:mentioned_in) { project.commit } it 'queues a background worker' do expect(Integrations::CreateExternalCrossReferenceWorker).to receive(:perform_async).with( project.id, 'JIRA-123', 'Commit', - mentioner.id, + mentioned_in.id, author.id ) @@ -716,28 +716,28 @@ RSpec.describe ::SystemNotes::IssuablesService do end describe '#cross_reference_disallowed?' do - context 'when mentioner is not a MergeRequest' do + context 'when mentioned_in is not a MergeRequest' do it 'is falsey' do - mentioner = noteable.dup + mentioned_in = noteable.dup - expect(service.cross_reference_disallowed?(mentioner)).to be_falsey + expect(service.cross_reference_disallowed?(mentioned_in)).to be_falsey end end - context 'when mentioner is a MergeRequest' do - let(:mentioner) { create(:merge_request, :simple, source_project: project) } - let(:noteable) { project.commit } + context 'when mentioned_in is a MergeRequest' do + let(:mentioned_in) { create(:merge_request, :simple, source_project: project) } + let(:noteable) { project.commit } it 'is truthy when noteable is in commits' do - expect(mentioner).to receive(:commits).and_return([noteable]) + expect(mentioned_in).to receive(:commits).and_return([noteable]) - expect(service.cross_reference_disallowed?(mentioner)).to be_truthy + expect(service.cross_reference_disallowed?(mentioned_in)).to be_truthy end it 'is falsey when noteable is not in commits' do - expect(mentioner).to receive(:commits).and_return([]) + expect(mentioned_in).to receive(:commits).and_return([]) - expect(service.cross_reference_disallowed?(mentioner)).to be_falsey + expect(service.cross_reference_disallowed?(mentioned_in)).to be_falsey end end diff --git a/spec/support/database/cross-database-modification-allowlist.yml b/spec/support/database/cross-database-modification-allowlist.yml index d05812a64eb..badaead950f 100644 --- a/spec/support/database/cross-database-modification-allowlist.yml +++ b/spec/support/database/cross-database-modification-allowlist.yml @@ -14,7 +14,6 @@ - "./ee/spec/services/deployments/auto_rollback_service_spec.rb" - "./ee/spec/services/ee/ci/job_artifacts/destroy_all_expired_service_spec.rb" - "./ee/spec/services/ee/users/destroy_service_spec.rb" -- "./ee/spec/services/projects/transfer_service_spec.rb" - "./ee/spec/services/security/security_orchestration_policies/rule_schedule_service_spec.rb" - "./spec/controllers/abuse_reports_controller_spec.rb" - "./spec/controllers/admin/spam_logs_controller_spec.rb" diff --git a/yarn.lock b/yarn.lock index cabbb32ac2b..f61b3e004cf 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3242,11 +3242,6 @@ call-bind@^1.0.0, call-bind@^1.0.2: function-bind "^1.1.1" get-intrinsic "^1.0.2" -call-me-maybe@^1.0.1: - version "1.0.1" - resolved "https://registry.yarnpkg.com/call-me-maybe/-/call-me-maybe-1.0.1.tgz#26d208ea89e37b5cbde60250a15f031c16a4d66b" - integrity sha1-JtII6onje1y95gJQoV8DHBak1ms= - callsites@^3.0.0: version "3.1.0" resolved "https://registry.yarnpkg.com/callsites/-/callsites-3.1.0.tgz#b3630abd8943432f54b3f0519238e33cd7df2f73" @@ -6077,11 +6072,6 @@ glob-parent@^5.1.1, glob-parent@^5.1.2, glob-parent@~5.1.0: dependencies: is-glob "^4.0.1" -glob-to-regexp@^0.4.0: - version "0.4.1" - resolved "https://registry.yarnpkg.com/glob-to-regexp/-/glob-to-regexp-0.4.1.tgz#c75297087c851b9a578bd217dd59a92f59fe546e" - integrity sha512-lkX1HJXwyMcprw/5YUZc2s7DrpAiHB21/V+E1rHUrVNokkvB6bqMzT0VfV6/86ZNabt1k14YOIaT7nDvOX3Iiw== - "glob@5 - 7", glob@^7.0.0, glob@^7.0.3, glob@^7.1.1, glob@^7.1.2, glob@^7.1.3, glob@^7.1.4, glob@^7.1.6, glob@~7.1.6: version "7.1.7" resolved "https://registry.yarnpkg.com/glob/-/glob-7.1.7.tgz#3b193e9233f01d42d0b3f78294bbeeb418f94a90" @@ -10177,14 +10167,6 @@ readable-stream@~2.0.6: string_decoder "~0.10.x" util-deprecate "~1.0.1" -readdir-enhanced@^2.2.4: - version "2.2.4" - resolved "https://registry.yarnpkg.com/readdir-enhanced/-/readdir-enhanced-2.2.4.tgz#773fb8a8de5f645fb13d9403746d490d4facb3e6" - integrity sha512-JQD83C9gAs5B5j2j40qLn/K83HhR8po3bUonebNeuJQUZbbn7q1HxL9kQuPBtxoXkaUpbtEmpFBw5kzyYnnJDA== - dependencies: - call-me-maybe "^1.0.1" - glob-to-regexp "^0.4.0" - readdirp@~3.4.0: version "3.4.0" resolved "https://registry.yarnpkg.com/readdirp/-/readdirp-3.4.0.tgz#9fdccdf9e9155805449221ac645e8303ab5b9ada"