diff options
82 files changed, 413 insertions, 135 deletions
diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index f0388ab79d2..2a3f16683cf 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -268,11 +268,6 @@ Naming/RescuedExceptionsVariableName: RSpec/ContextWording: Enabled: false -# Offense count: 407 -# Cop supports --auto-correct. -RSpec/EmptyLineAfterFinalLet: - Enabled: false - # Offense count: 719 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle. diff --git a/app/assets/javascripts/ide/stores/actions.js b/app/assets/javascripts/ide/stores/actions.js index dd69e2d6f1f..7a386299eed 100644 --- a/app/assets/javascripts/ide/stores/actions.js +++ b/app/assets/javascripts/ide/stores/actions.js @@ -110,6 +110,7 @@ export const createTempEntry = ( commit(types.ADD_FILE_TO_CHANGED, file.path); dispatch('setFileActive', file.path); dispatch('triggerFilesChange'); + dispatch('burstUnusedSeal'); } if (parentPath && !state.entries[parentPath].opened) { @@ -222,7 +223,9 @@ export const deleteEntry = ({ commit, dispatch, state }, path) => { dispatch('deleteEntry', prevPath); return; } - if (state.unusedSeal) dispatch('burstUnusedSeal'); + + dispatch('burstUnusedSeal'); + if (entry.opened) dispatch('closeFile', entry); if (isTree) { @@ -267,6 +270,7 @@ export const renameEntry = ({ dispatch, commit, state }, { path, name, parentPat commit(types.REMOVE_FILE_FROM_STAGED_AND_CHANGED, newEntry); } else if (!isInChanges) { commit(types.ADD_FILE_TO_CHANGED, newPath); + dispatch('burstUnusedSeal'); } if (!newEntry.tempFile) { diff --git a/changelogs/unreleased/43517-no-changes.yml b/changelogs/unreleased/43517-no-changes.yml new file mode 100644 index 00000000000..e36e0688251 --- /dev/null +++ b/changelogs/unreleased/43517-no-changes.yml @@ -0,0 +1,6 @@ +--- +title: Fix "No changes" empty state showing up in changes tab, despite there being + changes +merge_request: 21713 +author: +type: fixed diff --git a/db/migrate/20180305144721_add_privileged_to_runner.rb b/db/migrate/20180305144721_add_privileged_to_runner.rb index 359498bf9b0..1ad3c045d60 100644 --- a/db/migrate/20180305144721_add_privileged_to_runner.rb +++ b/db/migrate/20180305144721_add_privileged_to_runner.rb @@ -9,7 +9,7 @@ class AddPrivilegedToRunner < ActiveRecord::Migration[4.2] disable_ddl_transaction! def up - add_column_with_default :clusters_applications_runners, :privileged, :boolean, default: true, allow_null: false + add_column_with_default :clusters_applications_runners, :privileged, :boolean, default: true, allow_null: false # rubocop:disable Migration/AddColumnWithDefault end def down diff --git a/db/migrate/20180423204600_add_pages_access_level_to_project_feature.rb b/db/migrate/20180423204600_add_pages_access_level_to_project_feature.rb index 0c536f917ce..c841c7eb77b 100644 --- a/db/migrate/20180423204600_add_pages_access_level_to_project_feature.rb +++ b/db/migrate/20180423204600_add_pages_access_level_to_project_feature.rb @@ -5,7 +5,7 @@ class AddPagesAccessLevelToProjectFeature < ActiveRecord::Migration[4.2] DOWNTIME = false def up - add_column_with_default(:project_features, :pages_access_level, :integer, default: ProjectFeature::PUBLIC, allow_null: false) + add_column_with_default(:project_features, :pages_access_level, :integer, default: ProjectFeature::PUBLIC, allow_null: false) # rubocop:disable Migration/AddColumnWithDefault change_column_default(:project_features, :pages_access_level, ProjectFeature::ENABLED) end diff --git a/db/migrate/20180515005612_add_squash_to_merge_requests.rb b/db/migrate/20180515005612_add_squash_to_merge_requests.rb index 14636d6fd8e..dd301d22614 100644 --- a/db/migrate/20180515005612_add_squash_to_merge_requests.rb +++ b/db/migrate/20180515005612_add_squash_to_merge_requests.rb @@ -10,7 +10,7 @@ class AddSquashToMergeRequests < ActiveRecord::Migration[4.2] def up unless column_exists?(:merge_requests, :squash) # rubocop:disable Migration/UpdateLargeTable - add_column_with_default :merge_requests, :squash, :boolean, default: false, allow_null: false + add_column_with_default :merge_requests, :squash, :boolean, default: false, allow_null: false # rubocop:disable Migration/AddColumnWithDefault end end diff --git a/db/migrate/20180529093006_ensure_remote_mirror_columns.rb b/db/migrate/20180529093006_ensure_remote_mirror_columns.rb index a0a1150f022..3c61729dca8 100644 --- a/db/migrate/20180529093006_ensure_remote_mirror_columns.rb +++ b/db/migrate/20180529093006_ensure_remote_mirror_columns.rb @@ -11,7 +11,7 @@ class EnsureRemoteMirrorColumns < ActiveRecord::Migration[4.2] add_column :remote_mirrors, :remote_name, :string unless column_exists?(:remote_mirrors, :remote_name) # rubocop:disable Migration/AddLimitToStringColumns unless column_exists?(:remote_mirrors, :only_protected_branches) - add_column_with_default(:remote_mirrors, + add_column_with_default(:remote_mirrors, # rubocop:disable Migration/AddColumnWithDefault :only_protected_branches, :boolean, default: false, diff --git a/db/migrate/20180601213245_add_deploy_strategy_to_project_auto_devops.rb b/db/migrate/20180601213245_add_deploy_strategy_to_project_auto_devops.rb index 78a3617ec93..67d20b949d9 100644 --- a/db/migrate/20180601213245_add_deploy_strategy_to_project_auto_devops.rb +++ b/db/migrate/20180601213245_add_deploy_strategy_to_project_auto_devops.rb @@ -10,7 +10,7 @@ class AddDeployStrategyToProjectAutoDevops < ActiveRecord::Migration[4.2] disable_ddl_transaction! def up - add_column_with_default :project_auto_devops, :deploy_strategy, :integer, default: 0, allow_null: false + add_column_with_default :project_auto_devops, :deploy_strategy, :integer, default: 0, allow_null: false # rubocop:disable Migration/AddColumnWithDefault end def down diff --git a/db/migrate/20181016141739_add_status_to_deployments.rb b/db/migrate/20181016141739_add_status_to_deployments.rb index 2ff778448b4..7aaf029b69c 100644 --- a/db/migrate/20181016141739_add_status_to_deployments.rb +++ b/db/migrate/20181016141739_add_status_to_deployments.rb @@ -15,7 +15,7 @@ class AddStatusToDeployments < ActiveRecord::Migration[4.2] # However, we have to use the default value for avoiding `NOT NULL` violation during the transition period. # The default value should be removed in the future release. def up - add_column_with_default(:deployments, + add_column_with_default(:deployments, # rubocop:disable Migration/AddColumnWithDefault :status, :integer, limit: 2, diff --git a/db/migrate/20190218134158_add_masked_to_ci_variables.rb b/db/migrate/20190218134158_add_masked_to_ci_variables.rb index b4999d5b4a9..60dcc0d7af5 100644 --- a/db/migrate/20190218134158_add_masked_to_ci_variables.rb +++ b/db/migrate/20190218134158_add_masked_to_ci_variables.rb @@ -12,7 +12,7 @@ class AddMaskedToCiVariables < ActiveRecord::Migration[5.0] disable_ddl_transaction! def up - add_column_with_default :ci_variables, :masked, :boolean, default: false, allow_null: false + add_column_with_default :ci_variables, :masked, :boolean, default: false, allow_null: false # rubocop:disable Migration/AddColumnWithDefault end def down diff --git a/db/migrate/20190218134209_add_masked_to_ci_group_variables.rb b/db/migrate/20190218134209_add_masked_to_ci_group_variables.rb index 8633875b341..c25881410d0 100644 --- a/db/migrate/20190218134209_add_masked_to_ci_group_variables.rb +++ b/db/migrate/20190218134209_add_masked_to_ci_group_variables.rb @@ -12,7 +12,7 @@ class AddMaskedToCiGroupVariables < ActiveRecord::Migration[5.0] disable_ddl_transaction! def up - add_column_with_default :ci_group_variables, :masked, :boolean, default: false, allow_null: false + add_column_with_default :ci_group_variables, :masked, :boolean, default: false, allow_null: false # rubocop:disable Migration/AddColumnWithDefault end def down diff --git a/db/migrate/20190228192410_add_multi_line_attributes_to_suggestion.rb b/db/migrate/20190228192410_add_multi_line_attributes_to_suggestion.rb index 856dfc89fa3..766ea50161d 100644 --- a/db/migrate/20190228192410_add_multi_line_attributes_to_suggestion.rb +++ b/db/migrate/20190228192410_add_multi_line_attributes_to_suggestion.rb @@ -8,9 +8,11 @@ class AddMultiLineAttributesToSuggestion < ActiveRecord::Migration[5.0] disable_ddl_transaction! def up + # rubocop:disable Migration/AddColumnWithDefault add_column_with_default :suggestions, :lines_above, :integer, default: 0, allow_null: false add_column_with_default :suggestions, :lines_below, :integer, default: 0, allow_null: false add_column_with_default :suggestions, :outdated, :boolean, default: false, allow_null: false + # rubocop:enable Migration/AddColumnWithDefault end def down diff --git a/db/migrate/20190416185130_add_merge_train_enabled_to_ci_cd_settings.rb b/db/migrate/20190416185130_add_merge_train_enabled_to_ci_cd_settings.rb index eb0e7c41f98..e6427534310 100644 --- a/db/migrate/20190416185130_add_merge_train_enabled_to_ci_cd_settings.rb +++ b/db/migrate/20190416185130_add_merge_train_enabled_to_ci_cd_settings.rb @@ -8,7 +8,7 @@ class AddMergeTrainEnabledToCiCdSettings < ActiveRecord::Migration[5.1] disable_ddl_transaction! def up - add_column_with_default :project_ci_cd_settings, :merge_trains_enabled, :boolean, default: false, allow_null: false + add_column_with_default :project_ci_cd_settings, :merge_trains_enabled, :boolean, default: false, allow_null: false # rubocop:disable Migration/AddColumnWithDefault end def down diff --git a/db/migrate/20190426180107_add_deployment_events_to_services.rb b/db/migrate/20190426180107_add_deployment_events_to_services.rb index 1fb137fb5f9..e8e53728010 100644 --- a/db/migrate/20190426180107_add_deployment_events_to_services.rb +++ b/db/migrate/20190426180107_add_deployment_events_to_services.rb @@ -8,7 +8,7 @@ class AddDeploymentEventsToServices < ActiveRecord::Migration[5.0] disable_ddl_transaction! def up - add_column_with_default(:services, :deployment_events, :boolean, default: false, allow_null: false) + add_column_with_default(:services, :deployment_events, :boolean, default: false, allow_null: false) # rubocop:disable Migration/AddColumnWithDefault end def down diff --git a/db/migrate/20190709204413_add_rule_type_to_approval_project_rules.rb b/db/migrate/20190709204413_add_rule_type_to_approval_project_rules.rb index 87a228c9bf9..b11154d0f26 100644 --- a/db/migrate/20190709204413_add_rule_type_to_approval_project_rules.rb +++ b/db/migrate/20190709204413_add_rule_type_to_approval_project_rules.rb @@ -8,7 +8,7 @@ class AddRuleTypeToApprovalProjectRules < ActiveRecord::Migration[5.1] disable_ddl_transaction! def up - add_column_with_default :approval_project_rules, :rule_type, :integer, limit: 2, default: 0, allow_null: false + add_column_with_default :approval_project_rules, :rule_type, :integer, limit: 2, default: 0, allow_null: false # rubocop:disable Migration/AddColumnWithDefault end def down diff --git a/db/migrate/20190907184714_add_show_whitespace_in_diffs_to_user_preferences.rb b/db/migrate/20190907184714_add_show_whitespace_in_diffs_to_user_preferences.rb index 50d5d2b0574..41f9b36278a 100644 --- a/db/migrate/20190907184714_add_show_whitespace_in_diffs_to_user_preferences.rb +++ b/db/migrate/20190907184714_add_show_whitespace_in_diffs_to_user_preferences.rb @@ -11,7 +11,7 @@ class AddShowWhitespaceInDiffsToUserPreferences < ActiveRecord::Migration[5.2] disable_ddl_transaction! def up - add_column_with_default :user_preferences, :show_whitespace_in_diffs, :boolean, default: true, allow_null: false + add_column_with_default :user_preferences, :show_whitespace_in_diffs, :boolean, default: true, allow_null: false # rubocop:disable Migration/AddColumnWithDefault end def down diff --git a/db/migrate/20190918104731_add_cleanup_status_to_cluster.rb b/db/migrate/20190918104731_add_cleanup_status_to_cluster.rb index 0ba9d8e6c89..62290fb0fa6 100644 --- a/db/migrate/20190918104731_add_cleanup_status_to_cluster.rb +++ b/db/migrate/20190918104731_add_cleanup_status_to_cluster.rb @@ -9,7 +9,7 @@ class AddCleanupStatusToCluster < ActiveRecord::Migration[5.2] disable_ddl_transaction! def up - add_column_with_default(:clusters, :cleanup_status, + add_column_with_default(:clusters, :cleanup_status, # rubocop:disable Migration/AddColumnWithDefault :smallint, default: 1, allow_null: false) diff --git a/db/migrate/20191029191901_add_enabled_to_grafana_integrations.rb b/db/migrate/20191029191901_add_enabled_to_grafana_integrations.rb index 8db11724874..40e361e2150 100644 --- a/db/migrate/20191029191901_add_enabled_to_grafana_integrations.rb +++ b/db/migrate/20191029191901_add_enabled_to_grafana_integrations.rb @@ -8,7 +8,7 @@ class AddEnabledToGrafanaIntegrations < ActiveRecord::Migration[5.2] disable_ddl_transaction! def up - add_column_with_default( + add_column_with_default( # rubocop:disable Migration/AddColumnWithDefault :grafana_integrations, :enabled, :boolean, diff --git a/db/migrate/20191112090226_add_artifacts_to_ci_build_need.rb b/db/migrate/20191112090226_add_artifacts_to_ci_build_need.rb index 2fbd003b2e5..b868e0b44a8 100644 --- a/db/migrate/20191112090226_add_artifacts_to_ci_build_need.rb +++ b/db/migrate/20191112090226_add_artifacts_to_ci_build_need.rb @@ -8,7 +8,7 @@ class AddArtifactsToCiBuildNeed < ActiveRecord::Migration[5.2] disable_ddl_transaction! def up - add_column_with_default(:ci_build_needs, :artifacts, + add_column_with_default(:ci_build_needs, :artifacts, # rubocop:disable Migration/AddColumnWithDefault :boolean, default: true, allow_null: false) diff --git a/doc/development/what_requires_downtime.md b/doc/development/what_requires_downtime.md index 9e43758a4aa..356feae4eaf 100644 --- a/doc/development/what_requires_downtime.md +++ b/doc/development/what_requires_downtime.md @@ -34,6 +34,9 @@ blocking access to the table being modified. See ["Adding Columns With Default Values"](migration_style_guide.md#adding-columns-with-default-values) for more information on how to use this method. +Note that usage of `add_column_with_default` with `allow_null: false` to also add +a `NOT NULL` constraint is [discouraged](https://gitlab.com/gitlab-org/gitlab/issues/38060). + ## Dropping Columns Removing columns is tricky because running GitLab processes may still be using diff --git a/rubocop/cop/migration/add_column_with_default.rb b/rubocop/cop/migration/add_column_with_default.rb new file mode 100644 index 00000000000..8d1ab333dcf --- /dev/null +++ b/rubocop/cop/migration/add_column_with_default.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +require_relative '../../migration_helpers' + +module RuboCop + module Cop + module Migration + # Cop that checks if columns are added in a way that doesn't require + # downtime. + class AddColumnWithDefault < RuboCop::Cop::Cop + include MigrationHelpers + + WHITELISTED_TABLES = [:application_settings].freeze + + MSG = '`add_column_with_default` with `allow_null: false` may cause prolonged lock situations and downtime, ' \ + 'see https://gitlab.com/gitlab-org/gitlab/issues/38060'.freeze + + def on_send(node) + return unless in_migration?(node) + + name = node.children[1] + + return unless name == :add_column_with_default + + # Ignore whitelisted tables. + return if table_whitelisted?(node.children[2]) + + opts = node.children.last + + return unless opts && opts.type == :hash + + opts.each_node(:pair) do |pair| + if disallows_null_values?(pair) + add_offense(node, location: :selector) + end + end + end + + def table_whitelisted?(symbol) + symbol && symbol.type == :sym && + WHITELISTED_TABLES.include?(symbol.children[0]) + end + + def disallows_null_values?(pair) + options = [hash_key_type(pair), hash_key_name(pair), hash_value(pair)] + + options == [:sym, :allow_null, :false] # rubocop:disable Lint/BooleanSymbol + end + + def hash_key_type(pair) + pair.children[0].type + end + + def hash_key_name(pair) + pair.children[0].children[0] + end + + def hash_value(pair) + pair.children[1].type + end + end + end + end +end diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb index 1465c73d570..5f95703df01 100644 --- a/rubocop/rubocop.rb +++ b/rubocop/rubocop.rb @@ -17,6 +17,7 @@ require_relative 'cop/prefer_class_methods_over_module' require_relative 'cop/put_project_routes_under_scope' require_relative 'cop/put_group_routes_under_scope' require_relative 'cop/migration/add_column' +require_relative 'cop/migration/add_column_with_default' require_relative 'cop/migration/add_concurrent_foreign_key' require_relative 'cop/migration/add_concurrent_index' require_relative 'cop/migration/add_index' diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb index f11b5e798c9..ebdfbe14dec 100644 --- a/spec/controllers/admin/users_controller_spec.rb +++ b/spec/controllers/admin/users_controller_spec.rb @@ -4,6 +4,7 @@ require 'spec_helper' describe Admin::UsersController do let(:user) { create(:user) } + let_it_be(:admin) { create(:admin) } before do diff --git a/spec/controllers/concerns/continue_params_spec.rb b/spec/controllers/concerns/continue_params_spec.rb index b4b62cbe1e3..6af01aa837c 100644 --- a/spec/controllers/concerns/continue_params_spec.rb +++ b/spec/controllers/concerns/continue_params_spec.rb @@ -12,6 +12,7 @@ describe ContinueParams do end end end + subject(:controller) { controller_class.new } def strong_continue_params(params) diff --git a/spec/controllers/concerns/internal_redirect_spec.rb b/spec/controllers/concerns/internal_redirect_spec.rb index e5e50cfd55e..cc6422f2817 100644 --- a/spec/controllers/concerns/internal_redirect_spec.rb +++ b/spec/controllers/concerns/internal_redirect_spec.rb @@ -12,6 +12,7 @@ describe InternalRedirect do end end end + subject(:controller) { controller_class.new } describe '#safe_redirect_path' do diff --git a/spec/controllers/projects/branches_controller_spec.rb b/spec/controllers/projects/branches_controller_spec.rb index 4f8ab6a5def..757d8704a6a 100644 --- a/spec/controllers/projects/branches_controller_spec.rb +++ b/spec/controllers/projects/branches_controller_spec.rb @@ -35,6 +35,7 @@ describe Projects::BranchesController do context "valid branch name, valid source" do let(:branch) { "merge_branch" } let(:ref) { "master" } + it 'redirects' do expect(subject) .to redirect_to("/#{project.full_path}/tree/merge_branch") @@ -44,6 +45,7 @@ describe Projects::BranchesController do context "invalid branch name, valid ref" do let(:branch) { "<script>alert('merge');</script>" } let(:ref) { "master" } + it 'redirects' do expect(subject) .to redirect_to("/#{project.full_path}/tree/alert('merge');") @@ -53,18 +55,21 @@ describe Projects::BranchesController do context "valid branch name, invalid ref" do let(:branch) { "merge_branch" } let(:ref) { "<script>alert('ref');</script>" } + it { is_expected.to render_template('new') } end context "invalid branch name, invalid ref" do let(:branch) { "<script>alert('merge');</script>" } let(:ref) { "<script>alert('ref');</script>" } + it { is_expected.to render_template('new') } end context "valid branch name with encoded slashes" do let(:branch) { "feature%2Ftest" } let(:ref) { "<script>alert('ref');</script>" } + it { is_expected.to render_template('new') } it { project.repository.branch_exists?('feature/test') } end diff --git a/spec/controllers/projects/clusters_controller_spec.rb b/spec/controllers/projects/clusters_controller_spec.rb index ab8bfc0cabe..642932e2935 100644 --- a/spec/controllers/projects/clusters_controller_spec.rb +++ b/spec/controllers/projects/clusters_controller_spec.rb @@ -26,6 +26,7 @@ describe Projects::ClustersController do let(:project) { create(:project) } let!(:enabled_cluster) { create(:cluster, :provided_by_gcp, projects: [project]) } let!(:disabled_cluster) { create(:cluster, :disabled, :provided_by_gcp, :production_environment, projects: [project]) } + it 'lists available clusters' do go diff --git a/spec/controllers/projects/find_file_controller_spec.rb b/spec/controllers/projects/find_file_controller_spec.rb index a493985f8a0..4d8933f3aaf 100644 --- a/spec/controllers/projects/find_file_controller_spec.rb +++ b/spec/controllers/projects/find_file_controller_spec.rb @@ -28,11 +28,13 @@ describe Projects::FindFileController do context "valid branch" do let(:id) { 'master' } + it { is_expected.to respond_with(:success) } end context "invalid branch" do let(:id) { 'invalid-branch' } + it { is_expected.to respond_with(:not_found) } end end @@ -50,6 +52,7 @@ describe Projects::FindFileController do context "valid branch" do let(:id) { 'master' } + it 'returns an array of file path list' do go diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index f64e928098d..945a56365c8 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -1357,6 +1357,7 @@ describe Projects::IssuesController do describe 'GET #discussions' do let!(:discussion) { create(:discussion_note_on_issue, noteable: issue, project: issue.project) } + context 'when authenticated' do before do project.add_developer(user) diff --git a/spec/controllers/projects/tags_controller_spec.rb b/spec/controllers/projects/tags_controller_spec.rb index 7e5237facf6..15ef1c65c53 100644 --- a/spec/controllers/projects/tags_controller_spec.rb +++ b/spec/controllers/projects/tags_controller_spec.rb @@ -29,11 +29,13 @@ describe Projects::TagsController do context "valid tag" do let(:id) { 'v1.0.0' } + it { is_expected.to respond_with(:success) } end context "invalid tag" do let(:id) { 'latest' } + it { is_expected.to respond_with(:not_found) } end end diff --git a/spec/controllers/projects/wikis_controller_spec.rb b/spec/controllers/projects/wikis_controller_spec.rb index 3100aa2cb96..bfa555aab4c 100644 --- a/spec/controllers/projects/wikis_controller_spec.rb +++ b/spec/controllers/projects/wikis_controller_spec.rb @@ -213,6 +213,7 @@ describe Projects::WikisController do describe 'PATCH #update' do let(:new_title) { 'New title' } let(:new_content) { 'New content' } + subject do patch(:update, params: { diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index bbbb9691f53..597d2a185b5 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -81,6 +81,7 @@ describe UsersController do context 'json with events' do let(:project) { create(:project) } + before do project.add_developer(user) Gitlab::DataBuilder::Push.build_sample(project, user) diff --git a/spec/features/dashboard/snippets_spec.rb b/spec/features/dashboard/snippets_spec.rb index ff3eb58931d..94dc8601abb 100644 --- a/spec/features/dashboard/snippets_spec.rb +++ b/spec/features/dashboard/snippets_spec.rb @@ -91,6 +91,7 @@ describe 'Dashboard snippets' do context 'as an external user' do let(:user) { create(:user, :external) } + before do sign_in(user) visit dashboard_snippets_path diff --git a/spec/frontend/vue_shared/components/loading_button_spec.js b/spec/frontend/vue_shared/components/loading_button_spec.js new file mode 100644 index 00000000000..34fac37c250 --- /dev/null +++ b/spec/frontend/vue_shared/components/loading_button_spec.js @@ -0,0 +1,96 @@ +import { shallowMount } from '@vue/test-utils'; +import LoadingButton from '~/vue_shared/components/loading_button.vue'; + +const LABEL = 'Hello'; + +describe('LoadingButton', () => { + let wrapper; + + afterEach(() => { + wrapper.destroy(); + }); + + const buildWrapper = (propsData = {}) => { + wrapper = shallowMount(LoadingButton, { + propsData, + }); + }; + const findButtonLabel = () => wrapper.find('.js-loading-button-label'); + const findButtonIcon = () => wrapper.find('.js-loading-button-icon'); + + describe('loading spinner', () => { + it('shown when loading', () => { + buildWrapper({ loading: true }); + + expect(findButtonIcon().exists()).toBe(true); + }); + }); + + describe('disabled state', () => { + it('disabled when loading', () => { + buildWrapper({ loading: true }); + expect(wrapper.attributes('disabled')).toBe('disabled'); + }); + + it('not disabled when normal', () => { + buildWrapper({ loading: false }); + + expect(wrapper.attributes('disabled')).toBe(undefined); + }); + }); + + describe('label', () => { + it('shown when normal', () => { + buildWrapper({ loading: false, label: LABEL }); + expect(findButtonLabel().text()).toBe(LABEL); + }); + + it('shown when loading', () => { + buildWrapper({ loading: false, label: LABEL }); + expect(findButtonLabel().text()).toBe(LABEL); + }); + }); + + describe('container class', () => { + it('should default to btn btn-align-content', () => { + buildWrapper(); + + expect(wrapper.classes()).toContain('btn'); + expect(wrapper.classes()).toContain('btn-align-content'); + }); + + it('should be configurable through props', () => { + const containerClass = 'test-class'; + + buildWrapper({ + containerClass, + }); + + expect(wrapper.classes()).not.toContain('btn'); + expect(wrapper.classes()).not.toContain('btn-align-content'); + expect(wrapper.classes()).toContain(containerClass); + }); + }); + + describe('click callback prop', () => { + it('calls given callback when normal', () => { + buildWrapper({ + loading: false, + }); + + wrapper.trigger('click'); + + expect(wrapper.emitted('click')).toBeTruthy(); + }); + + it('does not call given callback when disabled because of loading', () => { + buildWrapper({ + loading: true, + }); + + wrapper.trigger('click'); + + expect(wrapper.emitted('click')).toBeFalsy(); + }); + }); +}); diff --git a/spec/javascripts/ide/stores/actions_spec.js b/spec/javascripts/ide/stores/actions_spec.js index 0ee114cb70d..cb03204d337 100644 --- a/spec/javascripts/ide/stores/actions_spec.js +++ b/spec/javascripts/ide/stores/actions_spec.js @@ -319,7 +319,7 @@ describe('Multi-file store actions', () => { { type: types.TOGGLE_FILE_OPEN, payload: 'test' }, { type: types.ADD_FILE_TO_CHANGED, payload: 'test' }, ], - [ + jasmine.arrayContaining([ { type: 'setFileActive', payload: 'test', @@ -327,7 +327,7 @@ describe('Multi-file store actions', () => { { type: 'triggerFilesChange', }, - ], + ]), done, ); }); @@ -350,6 +350,21 @@ describe('Multi-file store actions', () => { }) .catch(done.fail); }); + + it('bursts unused seal', done => { + store + .dispatch('createTempEntry', { + name: 'test', + branchId: 'mybranch', + type: 'blob', + }) + .then(() => { + expect(store.state.unusedSeal).toBe(false); + + done(); + }) + .catch(done.fail); + }); }); }); @@ -648,6 +663,19 @@ describe('Multi-file store actions', () => { ], ); }); + + it('bursts unused seal', done => { + store.state.entries.test = file('test'); + + store + .dispatch('deleteEntry', 'test') + .then(() => { + expect(store.state.unusedSeal).toBe(false); + + done(); + }) + .catch(done.fail); + }); }); describe('renameEntry', () => { @@ -747,7 +775,7 @@ describe('Multi-file store actions', () => { payload: 'renamed', }, ], - [{ type: 'triggerFilesChange' }], + [{ type: 'burstUnusedSeal' }, { type: 'triggerFilesChange' }], done, ); }); @@ -807,6 +835,20 @@ describe('Multi-file store actions', () => { .then(done) .catch(done.fail); }); + + it('bursts unused seal', done => { + store + .dispatch('renameEntry', { + path: 'orig', + name: 'renamed', + }) + .then(() => { + expect(store.state.unusedSeal).toBe(false); + + done(); + }) + .catch(done.fail); + }); }); describe('folder', () => { diff --git a/spec/javascripts/vue_shared/components/loading_button_spec.js b/spec/javascripts/vue_shared/components/loading_button_spec.js deleted file mode 100644 index 6b03c354e01..00000000000 --- a/spec/javascripts/vue_shared/components/loading_button_spec.js +++ /dev/null @@ -1,111 +0,0 @@ -import Vue from 'vue'; -import mountComponent from 'spec/helpers/vue_mount_component_helper'; -import loadingButton from '~/vue_shared/components/loading_button.vue'; - -const LABEL = 'Hello'; - -describe('LoadingButton', function() { - let vm; - let LoadingButton; - - beforeEach(() => { - LoadingButton = Vue.extend(loadingButton); - }); - - afterEach(() => { - vm.$destroy(); - }); - - describe('loading spinner', () => { - it('shown when loading', () => { - vm = mountComponent(LoadingButton, { - loading: true, - }); - - expect(vm.$el.querySelector('.js-loading-button-icon')).toBeDefined(); - }); - }); - - describe('disabled state', () => { - it('disabled when loading', () => { - vm = mountComponent(LoadingButton, { - loading: true, - }); - - expect(vm.$el.disabled).toEqual(true); - }); - - it('not disabled when normal', () => { - vm = mountComponent(LoadingButton, { - loading: false, - }); - - expect(vm.$el.disabled).toEqual(false); - }); - }); - - describe('label', () => { - it('shown when normal', () => { - vm = mountComponent(LoadingButton, { - loading: false, - label: LABEL, - }); - const label = vm.$el.querySelector('.js-loading-button-label'); - - expect(label.textContent.trim()).toEqual(LABEL); - }); - - it('shown when loading', () => { - vm = mountComponent(LoadingButton, { - loading: true, - label: LABEL, - }); - const label = vm.$el.querySelector('.js-loading-button-label'); - - expect(label.textContent.trim()).toEqual(LABEL); - }); - }); - - describe('container class', () => { - it('should default to btn btn-align-content', () => { - vm = mountComponent(LoadingButton, {}); - - expect(vm.$el.classList.contains('btn')).toEqual(true); - expect(vm.$el.classList.contains('btn-align-content')).toEqual(true); - }); - - it('should be configurable through props', () => { - vm = mountComponent(LoadingButton, { - containerClass: 'test-class', - }); - - expect(vm.$el.classList.contains('btn')).toEqual(false); - expect(vm.$el.classList.contains('btn-align-content')).toEqual(false); - expect(vm.$el.classList.contains('test-class')).toEqual(true); - }); - }); - - describe('click callback prop', () => { - it('calls given callback when normal', () => { - vm = mountComponent(LoadingButton, { - loading: false, - }); - spyOn(vm, '$emit'); - - vm.$el.click(); - - expect(vm.$emit).toHaveBeenCalledWith('click', jasmine.any(Object)); - }); - - it('does not call given callback when disabled because of loading', () => { - vm = mountComponent(LoadingButton, { - loading: true, - }); - spyOn(vm, '$emit'); - - vm.$el.click(); - - expect(vm.$emit).not.toHaveBeenCalled(); - }); - }); -}); diff --git a/spec/models/blob_spec.rb b/spec/models/blob_spec.rb index 2c141cae98d..c7ca0625b77 100644 --- a/spec/models/blob_spec.rb +++ b/spec/models/blob_spec.rb @@ -424,6 +424,7 @@ describe Blob do describe 'policy' do let(:project) { build(:project) } + subject { described_class.new(fake_blob(path: 'foo'), project) } it 'works with policy' do diff --git a/spec/models/blob_viewer/changelog_spec.rb b/spec/models/blob_viewer/changelog_spec.rb index 0fcc94182af..b71531ff3c2 100644 --- a/spec/models/blob_viewer/changelog_spec.rb +++ b/spec/models/blob_viewer/changelog_spec.rb @@ -7,6 +7,7 @@ describe BlobViewer::Changelog do let(:project) { create(:project, :repository) } let(:blob) { fake_blob(path: 'CHANGELOG') } + subject { described_class.new(blob) } describe '#render_error' do diff --git a/spec/models/blob_viewer/composer_json_spec.rb b/spec/models/blob_viewer/composer_json_spec.rb index eda34779679..a6bb64ba121 100644 --- a/spec/models/blob_viewer/composer_json_spec.rb +++ b/spec/models/blob_viewer/composer_json_spec.rb @@ -15,6 +15,7 @@ describe BlobViewer::ComposerJson do SPEC end let(:blob) { fake_blob(path: 'composer.json', data: data) } + subject { described_class.new(blob) } describe '#package_name' do diff --git a/spec/models/blob_viewer/gemspec_spec.rb b/spec/models/blob_viewer/gemspec_spec.rb index b6cc82c03ba..291d14e2d72 100644 --- a/spec/models/blob_viewer/gemspec_spec.rb +++ b/spec/models/blob_viewer/gemspec_spec.rb @@ -15,6 +15,7 @@ describe BlobViewer::Gemspec do SPEC end let(:blob) { fake_blob(path: 'activerecord.gemspec', data: data) } + subject { described_class.new(blob) } describe '#package_name' do diff --git a/spec/models/blob_viewer/gitlab_ci_yml_spec.rb b/spec/models/blob_viewer/gitlab_ci_yml_spec.rb index db405ceb4f1..02993052124 100644 --- a/spec/models/blob_viewer/gitlab_ci_yml_spec.rb +++ b/spec/models/blob_viewer/gitlab_ci_yml_spec.rb @@ -12,6 +12,7 @@ describe BlobViewer::GitlabCiYml do let(:data) { File.read(Rails.root.join('spec/support/gitlab_stubs/gitlab_ci.yml')) } let(:blob) { fake_blob(path: '.gitlab-ci.yml', data: data) } let(:sha) { sample_commit.id } + subject { described_class.new(blob) } describe '#validation_message' do diff --git a/spec/models/blob_viewer/license_spec.rb b/spec/models/blob_viewer/license_spec.rb index e02bfae3829..b0426401932 100644 --- a/spec/models/blob_viewer/license_spec.rb +++ b/spec/models/blob_viewer/license_spec.rb @@ -7,6 +7,7 @@ describe BlobViewer::License do let(:project) { create(:project, :repository) } let(:blob) { fake_blob(path: 'LICENSE') } + subject { described_class.new(blob) } describe '#license' do diff --git a/spec/models/blob_viewer/package_json_spec.rb b/spec/models/blob_viewer/package_json_spec.rb index b317278f3c8..7f7b1dcfcb3 100644 --- a/spec/models/blob_viewer/package_json_spec.rb +++ b/spec/models/blob_viewer/package_json_spec.rb @@ -15,6 +15,7 @@ describe BlobViewer::PackageJson do SPEC end let(:blob) { fake_blob(path: 'package.json', data: data) } + subject { described_class.new(blob) } describe '#package_name' do @@ -54,6 +55,7 @@ describe BlobViewer::PackageJson do SPEC end let(:blob) { fake_blob(path: 'package.json', data: data) } + subject { described_class.new(blob) } describe '#package_url' do diff --git a/spec/models/blob_viewer/podspec_json_spec.rb b/spec/models/blob_viewer/podspec_json_spec.rb index 7f1fb8666fd..dd5ed03b77d 100644 --- a/spec/models/blob_viewer/podspec_json_spec.rb +++ b/spec/models/blob_viewer/podspec_json_spec.rb @@ -15,6 +15,7 @@ describe BlobViewer::PodspecJson do SPEC end let(:blob) { fake_blob(path: 'AFNetworking.podspec.json', data: data) } + subject { described_class.new(blob) } describe '#package_name' do diff --git a/spec/models/blob_viewer/podspec_spec.rb b/spec/models/blob_viewer/podspec_spec.rb index 527ae79d766..2d9b184c5cb 100644 --- a/spec/models/blob_viewer/podspec_spec.rb +++ b/spec/models/blob_viewer/podspec_spec.rb @@ -15,6 +15,7 @@ describe BlobViewer::Podspec do SPEC end let(:blob) { fake_blob(path: 'Reachability.podspec', data: data) } + subject { described_class.new(blob) } describe '#package_name' do diff --git a/spec/models/blob_viewer/readme_spec.rb b/spec/models/blob_viewer/readme_spec.rb index 958927bddb4..6586adbc373 100644 --- a/spec/models/blob_viewer/readme_spec.rb +++ b/spec/models/blob_viewer/readme_spec.rb @@ -7,6 +7,7 @@ describe BlobViewer::Readme do let(:project) { create(:project, :repository, :wiki_repo) } let(:blob) { fake_blob(path: 'README.md') } + subject { described_class.new(blob) } describe '#render_error' do diff --git a/spec/models/blob_viewer/route_map_spec.rb b/spec/models/blob_viewer/route_map_spec.rb index f7ce873c9d1..6c703df5c4c 100644 --- a/spec/models/blob_viewer/route_map_spec.rb +++ b/spec/models/blob_viewer/route_map_spec.rb @@ -14,6 +14,7 @@ describe BlobViewer::RouteMap do MAP end let(:blob) { fake_blob(path: '.gitlab/route-map.yml', data: data) } + subject { described_class.new(blob) } describe '#validation_message' do diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 20915e6d3b1..d813a88c809 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -610,6 +610,7 @@ describe Ci::Build do context 'artifacts archive is a zip file and metadata exists' do let(:build) { create(:ci_build, :artifacts) } + it { is_expected.to be_truthy } end end @@ -1408,6 +1409,7 @@ describe Ci::Build do describe '#erased?' do let!(:build) { create(:ci_build, :trace_artifact, :success, :artifacts) } + subject { build.erased? } context 'job has not been erased' do @@ -1469,6 +1471,7 @@ describe Ci::Build do describe '#first_pending' do let!(:first) { create(:ci_build, pipeline: pipeline, status: 'pending', created_at: Date.yesterday) } let!(:second) { create(:ci_build, pipeline: pipeline, status: 'pending') } + subject { described_class.first_pending } it { is_expected.to be_a(described_class) } diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index ac438f7d473..5c9a03a26ec 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -755,11 +755,13 @@ describe Ci::Runner do context 'when group runner' do let(:runner) { create(:ci_runner, :group, description: 'Group runner', groups: [group]) } let(:group) { create(:group) } + it { is_expected.to be_falsey } end context 'when shared runner' do let(:runner) { create(:ci_runner, :instance, description: 'Shared runner') } + it { is_expected.to be_falsey } end diff --git a/spec/models/ci/stage_spec.rb b/spec/models/ci/stage_spec.rb index c997f1ef405..b65c11f837c 100644 --- a/spec/models/ci/stage_spec.rb +++ b/spec/models/ci/stage_spec.rb @@ -146,6 +146,7 @@ describe Ci::Stage, :models do let(:user) { create(:user) } let(:stage) { create(:ci_stage_entity, status: :created) } + subject { stage.detailed_status(user) } where(:statuses, :label) do diff --git a/spec/models/clusters/applications/elastic_stack_spec.rb b/spec/models/clusters/applications/elastic_stack_spec.rb index d0e0dd5ad57..2179e930691 100644 --- a/spec/models/clusters/applications/elastic_stack_spec.rb +++ b/spec/models/clusters/applications/elastic_stack_spec.rb @@ -123,6 +123,7 @@ describe Clusters::Applications::ElasticStack do context "cluster doesn't have kubeclient" do let(:cluster) { create(:cluster) } + subject { create(:clusters_applications_elastic_stack, cluster: cluster) } it 'returns nil' do diff --git a/spec/models/clusters/applications/helm_spec.rb b/spec/models/clusters/applications/helm_spec.rb index 64f58155a66..87454e1d3e2 100644 --- a/spec/models/clusters/applications/helm_spec.rb +++ b/spec/models/clusters/applications/helm_spec.rb @@ -52,6 +52,7 @@ describe Clusters::Applications::Helm do describe '#issue_client_cert' do let(:application) { create(:clusters_applications_helm) } + subject { application.issue_client_cert } it 'returns a new cert' do diff --git a/spec/models/clusters/applications/knative_spec.rb b/spec/models/clusters/applications/knative_spec.rb index c1057af5f80..68ac3f0d483 100644 --- a/spec/models/clusters/applications/knative_spec.rb +++ b/spec/models/clusters/applications/knative_spec.rb @@ -131,6 +131,7 @@ describe Clusters::Applications::Knative do describe '#update_command' do let!(:current_installed_version) { knative.version = '0.1.0' } + subject { knative.update_command } it 'is initialized with current version' do diff --git a/spec/models/clusters/applications/prometheus_spec.rb b/spec/models/clusters/applications/prometheus_spec.rb index d588ce3bc38..0f829e138d5 100644 --- a/spec/models/clusters/applications/prometheus_spec.rb +++ b/spec/models/clusters/applications/prometheus_spec.rb @@ -66,6 +66,7 @@ describe Clusters::Applications::Prometheus do context "cluster doesn't have kubeclient" do let(:cluster) { create(:cluster) } + subject { create(:clusters_applications_prometheus, cluster: cluster) } it 'returns nil' do diff --git a/spec/models/concerns/cache_markdown_field_spec.rb b/spec/models/concerns/cache_markdown_field_spec.rb index 9a12c3d6965..06d12c14793 100644 --- a/spec/models/concerns/cache_markdown_field_spec.rb +++ b/spec/models/concerns/cache_markdown_field_spec.rb @@ -92,6 +92,7 @@ describe CacheMarkdownField, :clean_gitlab_redis_cache do describe '#latest_cached_markdown_version' do let(:thing) { klass.new } + subject { thing.latest_cached_markdown_version } it 'returns default version' do @@ -151,6 +152,7 @@ describe CacheMarkdownField, :clean_gitlab_redis_cache do describe '#banzai_render_context' do let(:thing) { klass.new(title: markdown, title_html: html, cached_markdown_version: cache_version) } + subject(:context) { thing.banzai_render_context(:title) } it 'sets project to nil if the object lacks a project' do diff --git a/spec/models/concerns/ignorable_columns_spec.rb b/spec/models/concerns/ignorable_columns_spec.rb index 55efa1b5fda..018b1296c62 100644 --- a/spec/models/concerns/ignorable_columns_spec.rb +++ b/spec/models/concerns/ignorable_columns_spec.rb @@ -49,11 +49,13 @@ describe IgnorableColumns do context 'with single column' do let(:columns) { :name } + it_behaves_like 'storing removal information' end context 'with array column' do let(:columns) { %i[name created_at] } + it_behaves_like 'storing removal information' end diff --git a/spec/models/concerns/loaded_in_group_list_spec.rb b/spec/models/concerns/loaded_in_group_list_spec.rb index 7c97b580779..509811822e0 100644 --- a/spec/models/concerns/loaded_in_group_list_spec.rb +++ b/spec/models/concerns/loaded_in_group_list_spec.rb @@ -4,6 +4,7 @@ require 'spec_helper' describe LoadedInGroupList do let(:parent) { create(:group) } + subject(:found_group) { Group.with_selects_for_list.find_by(id: parent.id) } describe '.with_selects_for_list' do diff --git a/spec/models/concerns/prometheus_adapter_spec.rb b/spec/models/concerns/prometheus_adapter_spec.rb index 3d26ba95192..3ac96b308ed 100644 --- a/spec/models/concerns/prometheus_adapter_spec.rb +++ b/spec/models/concerns/prometheus_adapter_spec.rb @@ -103,6 +103,7 @@ describe PrometheusAdapter, :use_clean_rails_memory_store_caching do describe '#calculate_reactive_cache' do let(:environment) { create(:environment, slug: 'env-slug') } + before do service.manual_configuration = true service.active = true diff --git a/spec/models/concerns/resolvable_note_spec.rb b/spec/models/concerns/resolvable_note_spec.rb index 4f46252a044..12e50ac807e 100644 --- a/spec/models/concerns/resolvable_note_spec.rb +++ b/spec/models/concerns/resolvable_note_spec.rb @@ -5,6 +5,7 @@ require 'spec_helper' describe Note, ResolvableNote do let(:project) { create(:project, :repository) } let(:merge_request) { create(:merge_request, source_project: project) } + subject { create(:discussion_note_on_merge_request, noteable: merge_request, project: project) } context 'resolvability scopes' do diff --git a/spec/models/concerns/token_authenticatable_spec.rb b/spec/models/concerns/token_authenticatable_spec.rb index 43b894b5957..36eb8fdaba4 100644 --- a/spec/models/concerns/token_authenticatable_spec.rb +++ b/spec/models/concerns/token_authenticatable_spec.rb @@ -13,6 +13,7 @@ end describe User, 'TokenAuthenticatable' do let(:token_field) { :feed_token } + it_behaves_like 'TokenAuthenticatable' describe 'ensures authentication token' do diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index b82a9e9aa9d..e37a7e85b75 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -8,6 +8,7 @@ describe Environment, :use_clean_rails_memory_store_caching do include RepoHelpers let(:project) { create(:project, :stubbed_repository) } + subject(:environment) { create(:environment, project: project) } it { is_expected.to be_kind_of(ReactiveCaching) } @@ -676,6 +677,7 @@ describe Environment, :use_clean_rails_memory_store_caching do context 'with deployment' do let!(:deployment) { create(:deployment, :success, environment: environment) } + it { is_expected.to be_truthy } end @@ -832,6 +834,7 @@ describe Environment, :use_clean_rails_memory_store_caching do context 'and a deployment' do let!(:deployment) { create(:deployment, environment: environment) } + it { is_expected.to be_truthy } end @@ -866,6 +869,7 @@ describe Environment, :use_clean_rails_memory_store_caching do describe '#metrics' do let(:project) { create(:prometheus_project) } + subject { environment.metrics } context 'when the environment has metrics' do @@ -947,6 +951,7 @@ describe Environment, :use_clean_rails_memory_store_caching do describe '#additional_metrics' do let(:project) { create(:prometheus_project) } let(:metric_params) { [] } + subject { environment.additional_metrics(*metric_params) } context 'when the environment has additional metrics' do diff --git a/spec/models/external_issue_spec.rb b/spec/models/external_issue_spec.rb index 9d064d458f0..b8d85d49b07 100644 --- a/spec/models/external_issue_spec.rb +++ b/spec/models/external_issue_spec.rb @@ -33,6 +33,7 @@ describe ExternalIssue do context 'if issue id is a number' do let(:issue) { described_class.new('1234', project) } + it 'returns the issue ID prefixed by #' do expect(issue.reference_link_text).to eq '#1234' end diff --git a/spec/models/global_milestone_spec.rb b/spec/models/global_milestone_spec.rb index 9d901d01a52..34dbdfec60d 100644 --- a/spec/models/global_milestone_spec.rb +++ b/spec/models/global_milestone_spec.rb @@ -162,6 +162,7 @@ describe GlobalMilestone do describe '#initialize' do let(:milestone1_project1) { create(:milestone, title: "Milestone v1.2", project: project1) } + subject(:global_milestone) { described_class.new(milestone1_project1) } it 'has exactly one group milestone' do diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 3fa9d71cc7d..842acf92c2a 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -28,6 +28,7 @@ describe Group do describe '#members & #requesters' do let(:requester) { create(:user) } let(:developer) { create(:user) } + before do group.request_access(requester) group.add_developer(developer) diff --git a/spec/models/hooks/web_hook_log_spec.rb b/spec/models/hooks/web_hook_log_spec.rb index 22aad2fab0a..3520720d9a4 100644 --- a/spec/models/hooks/web_hook_log_spec.rb +++ b/spec/models/hooks/web_hook_log_spec.rb @@ -53,16 +53,19 @@ describe WebHookLog do describe '2xx' do let(:status) { '200' } + it { expect(web_hook_log.success?).to be_truthy } end describe 'not 2xx' do let(:status) { '500' } + it { expect(web_hook_log.success?).to be_falsey } end describe 'internal erorr' do let(:status) { 'internal error' } + it { expect(web_hook_log.success?).to be_falsey } end end diff --git a/spec/models/instance_configuration_spec.rb b/spec/models/instance_configuration_spec.rb index 43954511858..3e0181b8846 100644 --- a/spec/models/instance_configuration_spec.rb +++ b/spec/models/instance_configuration_spec.rb @@ -48,6 +48,7 @@ describe InstanceConfiguration do describe '#gitlab_pages' do let(:gitlab_pages) { subject.settings[:gitlab_pages] } + it 'returns Settings.pages' do gitlab_pages.delete(:ip_address) @@ -73,6 +74,7 @@ describe InstanceConfiguration do describe '#gitlab_ci' do let(:gitlab_ci) { subject.settings[:gitlab_ci] } + it 'returns Settings.gitalb_ci' do gitlab_ci.delete(:artifacts_max_size) diff --git a/spec/models/internal_id_spec.rb b/spec/models/internal_id_spec.rb index c73ade3f896..33d03bfc0f5 100644 --- a/spec/models/internal_id_spec.rb +++ b/spec/models/internal_id_spec.rb @@ -170,6 +170,7 @@ describe InternalId do describe '.track_greatest' do let(:value) { 9001 } + subject { described_class.track_greatest(issue, scope, usage, value, init) } context 'in the absence of a record' do @@ -210,6 +211,7 @@ describe InternalId do describe '#increment_and_save!' do let(:id) { create(:internal_id) } + subject { id.increment_and_save! } it 'returns incremented iid' do @@ -236,6 +238,7 @@ describe InternalId do describe '#track_greatest_and_save!' do let(:id) { create(:internal_id) } let(:new_last_value) { 9001 } + subject { id.track_greatest_and_save!(new_last_value) } it 'returns new last value' do diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index d1ed06dd04d..5c3f7c09e22 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -259,6 +259,7 @@ describe Issue do describe '#can_move?' do let(:user) { create(:user) } let(:issue) { create(:issue) } + subject { issue.can_move?(user) } context 'user is not a member of project issue belongs to' do @@ -277,6 +278,7 @@ describe Issue do context 'issue not persisted' do let(:issue) { build(:issue, project: project) } + it { is_expected.to eq false } end @@ -306,6 +308,7 @@ describe Issue do describe '#moved?' do let(:issue) { create(:issue) } + subject { issue.moved? } context 'issue not moved' do @@ -322,6 +325,7 @@ describe Issue do describe '#duplicated?' do let(:issue) { create(:issue) } + subject { issue.duplicated? } context 'issue not duplicated' do @@ -380,6 +384,7 @@ describe Issue do describe '#has_related_branch?' do let(:issue) { create(:issue, title: "Blue Bell Knoll") } + subject { issue.has_related_branch? } context 'branch found' do @@ -442,6 +447,7 @@ describe Issue do describe '#can_be_worked_on?' do let(:project) { build(:project) } + subject { build(:issue, :opened, project: project) } context 'is closed' do diff --git a/spec/models/key_spec.rb b/spec/models/key_spec.rb index 2dd9583087f..1ae90cae4b1 100644 --- a/spec/models/key_spec.rb +++ b/spec/models/key_spec.rb @@ -25,6 +25,7 @@ describe Key, :mailer do describe "Methods" do let(:user) { create(:user) } + it { is_expected.to respond_to :projects } it { is_expected.to respond_to :publishable_key } diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index bf90fa53aba..c98d123ff52 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -110,6 +110,7 @@ describe MergeRequest do describe '#squash?' do let(:merge_request) { build(:merge_request, squash: squash) } + subject { merge_request.squash? } context 'disabled in database' do @@ -851,6 +852,7 @@ describe MergeRequest do describe '#modified_paths' do let(:paths) { double(:paths) } + subject(:merge_request) { build(:merge_request) } before do @@ -879,6 +881,7 @@ describe MergeRequest do context 'when no arguments provided' do let(:diff) { merge_request.merge_request_diff } + subject(:merge_request) { create(:merge_request, source_branch: 'feature', target_branch: 'master') } it 'returns affected file paths for merge_request_diff' do @@ -1554,6 +1557,7 @@ describe MergeRequest do describe '#calculate_reactive_cache' do let(:project) { create(:project, :repository) } let(:merge_request) { create(:merge_request, source_project: project) } + subject { merge_request.calculate_reactive_cache(service_class_name) } context 'when given an unknown service class name' do @@ -3044,6 +3048,7 @@ describe MergeRequest do describe 'transition to cannot_be_merged' do let(:notification_service) { double(:notification_service) } let(:todo_service) { double(:todo_service) } + subject { create(:merge_request, state, merge_status: :unchecked) } before do @@ -3253,6 +3258,7 @@ describe MergeRequest do describe 'when merge_when_pipeline_succeeds? is true' do describe 'when merge user is author' do let(:user) { create(:user) } + subject do create(:merge_request, merge_when_pipeline_succeeds: true, @@ -3267,6 +3273,7 @@ describe MergeRequest do describe 'when merge user and author are different users' do let(:merge_user) { create(:user) } + subject do create(:merge_request, merge_when_pipeline_succeeds: true, diff --git a/spec/models/project_deploy_token_spec.rb b/spec/models/project_deploy_token_spec.rb index 8c8924762bd..0543bbdf2a8 100644 --- a/spec/models/project_deploy_token_spec.rb +++ b/spec/models/project_deploy_token_spec.rb @@ -5,6 +5,7 @@ require 'spec_helper' RSpec.describe ProjectDeployToken, type: :model do let(:project) { create(:project) } let(:deploy_token) { create(:deploy_token) } + subject(:project_deploy_token) { create(:project_deploy_token, project: project, deploy_token: deploy_token) } it { is_expected.to belong_to :project } diff --git a/spec/models/project_services/microsoft_teams_service_spec.rb b/spec/models/project_services/microsoft_teams_service_spec.rb index 275244fa5fd..83d3c8b3a70 100644 --- a/spec/models/project_services/microsoft_teams_service_spec.rb +++ b/spec/models/project_services/microsoft_teams_service_spec.rb @@ -38,6 +38,7 @@ describe MicrosoftTeamsService do describe "#execute" do let(:user) { create(:user) } + set(:project) { create(:project, :repository, :wiki_repo) } before do diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 99d7e4d156f..e66f37f2eec 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -165,6 +165,7 @@ describe Project do let(:project) { create(:project, :public) } let(:requester) { create(:user) } let(:developer) { create(:user) } + before do project.request_access(requester) project.add_developer(developer) @@ -815,6 +816,7 @@ describe Project do context 'with external issues tracker' do let!(:internal_issue) { create(:issue, project: project) } + before do allow(project).to receive(:external_issue_tracker).and_return(true) end @@ -2334,6 +2336,7 @@ describe Project do describe '#has_remote_mirror?' do let(:project) { create(:project, :remote_mirror, :import_started) } + subject { project.has_remote_mirror? } before do @@ -2353,6 +2356,7 @@ describe Project do describe '#update_remote_mirrors' do let(:project) { create(:project, :remote_mirror, :import_started) } + delegate :update_remote_mirrors, to: :project before do @@ -3460,6 +3464,7 @@ describe Project do describe '#pipeline_status' do let(:project) { create(:project, :repository) } + it 'builds a pipeline status' do expect(project.pipeline_status).to be_a(Gitlab::Cache::Ci::ProjectPipelineStatus) end @@ -4638,6 +4643,7 @@ describe Project do describe '#execute_hooks' do let(:data) { { ref: 'refs/heads/master', data: 'data' } } + it 'executes active projects hooks with the specified scope' do hook = create(:project_hook, merge_requests_events: false, push_events: true) expect(ProjectHook).to receive(:select_active) @@ -4968,6 +4974,7 @@ describe Project do context 'when there is a gitlab deploy token associated but is has been revoked' do let!(:deploy_token) { create(:deploy_token, :gitlab_deploy_token, :revoked, projects: [project]) } + it { is_expected.to be_nil } end @@ -5011,6 +5018,7 @@ describe Project do context '#members_among' do let(:users) { create_list(:user, 3) } + set(:group) { create(:group) } set(:project) { create(:project, namespace: group) } diff --git a/spec/models/readme_blob_spec.rb b/spec/models/readme_blob_spec.rb index f07713bd908..34182fa413f 100644 --- a/spec/models/readme_blob_spec.rb +++ b/spec/models/readme_blob_spec.rb @@ -7,6 +7,7 @@ describe ReadmeBlob do describe 'policy' do let(:project) { build(:project, :repository) } + subject { described_class.new(fake_blob(path: 'README.md'), project.repository) } it 'works with policy' do diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index c0245dfdf1a..661cbd14427 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -717,6 +717,7 @@ describe Repository do describe "search_files_by_content" do let(:results) { repository.search_files_by_content('feature', 'master') } + subject { results } it { is_expected.to be_an Array } diff --git a/spec/models/sent_notification_spec.rb b/spec/models/sent_notification_spec.rb index 09be90b82ed..7539bf1e957 100644 --- a/spec/models/sent_notification_spec.rb +++ b/spec/models/sent_notification_spec.rb @@ -18,6 +18,7 @@ describe SentNotification do context "when the project doesn't match the discussion project" do let(:discussion_id) { create(:note).discussion_id } + subject { build(:sent_notification, in_reply_to_discussion_id: discussion_id) } it "is invalid" do @@ -29,6 +30,7 @@ describe SentNotification do let(:project) { create(:project, :repository) } let(:issue) { create(:issue, project: project) } let(:discussion_id) { create(:note, project: project, noteable: issue).discussion_id } + subject { build(:sent_notification, project: project, noteable: issue, in_reply_to_discussion_id: discussion_id) } it "is valid" do @@ -196,6 +198,7 @@ describe SentNotification do describe '#create_reply' do context 'for issue' do let(:issue) { create(:issue) } + subject { described_class.record(issue, issue.author.id) } it 'creates a comment on the issue' do @@ -206,6 +209,7 @@ describe SentNotification do context 'for issue comment' do let(:note) { create(:note_on_issue) } + subject { described_class.record_note(note, note.author.id) } it 'creates a comment on the issue' do @@ -217,6 +221,7 @@ describe SentNotification do context 'for issue discussion' do let(:note) { create(:discussion_note_on_issue) } + subject { described_class.record_note(note, note.author.id) } it 'creates a reply on the discussion' do @@ -228,6 +233,7 @@ describe SentNotification do context 'for merge request' do let(:merge_request) { create(:merge_request) } + subject { described_class.record(merge_request, merge_request.author.id) } it 'creates a comment on the merge_request' do @@ -238,6 +244,7 @@ describe SentNotification do context 'for merge request comment' do let(:note) { create(:note_on_merge_request) } + subject { described_class.record_note(note, note.author.id) } it 'creates a comment on the merge request' do @@ -249,6 +256,7 @@ describe SentNotification do context 'for merge request diff discussion' do let(:note) { create(:diff_note_on_merge_request) } + subject { described_class.record_note(note, note.author.id) } it 'creates a reply on the discussion' do @@ -260,6 +268,7 @@ describe SentNotification do context 'for merge request non-diff discussion' do let(:note) { create(:discussion_note_on_merge_request) } + subject { described_class.record_note(note, note.author.id) } it 'creates a reply on the discussion' do @@ -272,6 +281,7 @@ describe SentNotification do context 'for commit' do let(:project) { create(:project, :repository) } let(:commit) { project.commit } + subject { described_class.record(commit, project.creator.id) } it 'creates a comment on the commit' do @@ -282,6 +292,7 @@ describe SentNotification do context 'for commit comment' do let(:note) { create(:note_on_commit) } + subject { described_class.record_note(note, note.author.id) } it 'creates a comment on the commit' do @@ -293,6 +304,7 @@ describe SentNotification do context 'for commit diff discussion' do let(:note) { create(:diff_note_on_commit) } + subject { described_class.record_note(note, note.author.id) } it 'creates a reply on the discussion' do @@ -304,6 +316,7 @@ describe SentNotification do context 'for commit non-diff discussion' do let(:note) { create(:discussion_note_on_commit) } + subject { described_class.record_note(note, note.author.id) } it 'creates a reply on the discussion' do diff --git a/spec/models/snippet_spec.rb b/spec/models/snippet_spec.rb index 9c549a6d56d..ae43c0d585a 100644 --- a/spec/models/snippet_spec.rb +++ b/spec/models/snippet_spec.rb @@ -141,6 +141,7 @@ describe Snippet do describe "#content_html_invalidated?" do let(:snippet) { create(:snippet, content: "md", content_html: "html", file_name: "foo.md") } + it "invalidates the HTML cache of content when the filename changes" do expect { snippet.file_name = "foo.rb" }.to change { snippet.content_html_invalidated? }.from(false).to(true) end diff --git a/spec/models/uploads/fog_spec.rb b/spec/models/uploads/fog_spec.rb index b93d9449da9..72a169280af 100644 --- a/spec/models/uploads/fog_spec.rb +++ b/spec/models/uploads/fog_spec.rb @@ -31,6 +31,7 @@ describe Uploads::Fog do describe '#keys' do let!(:uploads) { create_list(:upload, 2, :object_storage, uploader: FileUploader, model: project) } + subject { data_store.keys(relation) } it 'returns keys' do @@ -41,6 +42,7 @@ describe Uploads::Fog do describe '#delete_keys' do let(:keys) { data_store.keys(relation) } let!(:uploads) { create_list(:upload, 2, :with_file, :issuable_upload, model: project) } + subject { data_store.delete_keys(keys) } before do diff --git a/spec/models/uploads/local_spec.rb b/spec/models/uploads/local_spec.rb index 3468399f370..374c3019edc 100644 --- a/spec/models/uploads/local_spec.rb +++ b/spec/models/uploads/local_spec.rb @@ -15,6 +15,7 @@ describe Uploads::Local do describe '#keys' do let!(:uploads) { create_list(:upload, 2, uploader: FileUploader, model: project) } + subject { data_store.keys(relation) } it 'returns keys' do @@ -25,6 +26,7 @@ describe Uploads::Local do describe '#delete_keys' do let(:keys) { data_store.keys(relation) } let!(:uploads) { create_list(:upload, 2, :with_file, :issuable_upload, model: project) } + subject { data_store.delete_keys(keys) } it 'deletes multiple data' do diff --git a/spec/models/user_interacted_project_spec.rb b/spec/models/user_interacted_project_spec.rb index b96ff08e22d..e2c485343ae 100644 --- a/spec/models/user_interacted_project_spec.rb +++ b/spec/models/user_interacted_project_spec.rb @@ -11,6 +11,7 @@ describe UserInteractedProject do Event::ACTIONS.each do |action| context "for all actions (event types)" do let(:event) { build(:event, action: action) } + it 'creates a record' do expect { subject }.to change { described_class.count }.from(0).to(1) end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 749d80ebfc2..314cc4e1d3c 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -2390,6 +2390,7 @@ describe User, :do_not_mock_admin_mode do describe '#authorizations_for_projects' do let!(:user) { create(:user) } + subject { Project.where("EXISTS (?)", user.authorizations_for_projects) } it 'includes projects that belong to a user, but no other projects' do @@ -3686,6 +3687,7 @@ describe User, :do_not_mock_admin_mode do describe '#required_terms_not_accepted?' do let(:user) { build(:user) } + subject { user.required_terms_not_accepted? } context "when terms are not enforced" do diff --git a/spec/rubocop/cop/migration/add_column_with_default_spec.rb b/spec/rubocop/cop/migration/add_column_with_default_spec.rb new file mode 100644 index 00000000000..3d2315ddfa1 --- /dev/null +++ b/spec/rubocop/cop/migration/add_column_with_default_spec.rb @@ -0,0 +1,65 @@ +# frozen_string_literal: true + +require 'spec_helper' + +require 'rubocop' +require 'rubocop/rspec/support' + +require_relative '../../../../rubocop/cop/migration/add_column_with_default' + +describe RuboCop::Cop::Migration::AddColumnWithDefault do + include CopHelper + + let(:cop) { described_class.new } + + context 'outside of a migration' do + it 'does not register any offenses' do + expect_no_offenses(<<~RUBY) + def up + add_reference(:projects, :users) + end + RUBY + end + end + + context 'in a migration' do + before do + allow(cop).to receive(:in_migration?).and_return(true) + end + + let(:offense) { '`add_column_with_default` with `allow_null: false` may cause prolonged lock situations and downtime, see https://gitlab.com/gitlab-org/gitlab/issues/38060' } + + it 'registers an offense when specifying allow_null: false' do + expect_offense(<<~RUBY) + def up + add_column_with_default(:ci_build_needs, :artifacts, :boolean, default: true, allow_null: false) + ^^^^^^^^^^^^^^^^^^^^^^^ #{offense} + end + RUBY + end + + it 'registers no offense when specifying allow_null: true' do + expect_no_offenses(<<~RUBY) + def up + add_column_with_default(:ci_build_needs, :artifacts, :boolean, default: true, allow_null: true) + end + RUBY + end + + it 'registers no offense when allow_null is not specified' do + expect_no_offenses(<<~RUBY) + def up + add_column_with_default(:ci_build_needs, :artifacts, :boolean, default: true) + end + RUBY + end + + it 'registers no offense for application_settings (whitelisted table)' do + expect_no_offenses(<<~RUBY) + def up + add_column_with_default(:application_settings, :another_column, :boolean, default: true, allow_null: false) + end + RUBY + end + end +end |