diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2023-12-15 09:13:20 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2023-12-15 09:13:20 +0300 |
commit | 5e9a1717166c6b9bf0a6cbd00137d5c0043ba442 (patch) | |
tree | 30e54c2e3c41ad4caf51b0a60b46549d3feb3a7c /spec | |
parent | 00880613328c66f85aee755fcd1e70ce4cb9fcdf (diff) |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'spec')
-rw-r--r-- | spec/features/issues/discussion_lock_spec.rb | 254 | ||||
-rw-r--r-- | spec/lib/backup/database_configuration_spec.rb | 239 | ||||
-rw-r--r-- | spec/lib/backup/database_connection_spec.rb | 103 | ||||
-rw-r--r-- | spec/lib/backup/database_model_spec.rb | 186 | ||||
-rw-r--r-- | spec/lib/backup/database_spec.rb | 99 | ||||
-rw-r--r-- | spec/lib/backup/dump/postgres_spec.rb | 68 | ||||
-rw-r--r-- | spec/models/repository_spec.rb | 94 | ||||
-rw-r--r-- | spec/views/profiles/keys/_form.html.haml_spec.rb | 4 | ||||
-rw-r--r-- | spec/workers/process_commit_worker_spec.rb | 254 |
9 files changed, 794 insertions, 507 deletions
diff --git a/spec/features/issues/discussion_lock_spec.rb b/spec/features/issues/discussion_lock_spec.rb index 1b7357a774a..2ef912061e6 100644 --- a/spec/features/issues/discussion_lock_spec.rb +++ b/spec/features/issues/discussion_lock_spec.rb @@ -6,104 +6,240 @@ RSpec.describe 'Discussion Lock', :js, feature_category: :team_planning do let(:user) { create(:user) } let(:issue) { create(:issue, project: project, author: user) } let(:project) { create(:project, :public) } + let(:more_dropdown) { find_by_testid('desktop-dropdown') } + let(:issuable_lock) { find_by_testid('issuable-lock') } + let(:locked_badge) { '[data-testid="locked-badge"]' } + let(:issuable_note_warning) { '[data-testid="issuable-note-warning"]' } - before do - sign_in(user) - stub_feature_flags(moved_mr_sidebar: false) - end - - context 'when a user is a team member' do + context 'when feature flag is disabled' do before do - project.add_developer(user) + sign_in(user) + stub_feature_flags(moved_mr_sidebar: false) end - context 'when the discussion is unlocked' do - it 'the user can lock the issue' do - visit project_issue_path(project, issue) + context 'when a user is a team member' do + before do + project.add_developer(user) + end - expect(find('.issuable-sidebar')).to have_content('Unlocked') + context 'when the discussion is unlocked' do + it 'the user can lock the issue' do + visit project_issue_path(project, issue) - page.within('.issuable-sidebar') do - find('.lock-edit').click - click_button('Lock') - end + expect(find('.issuable-sidebar')).to have_content('Unlocked') + + page.within('.issuable-sidebar') do + find('.lock-edit').click + click_button('Lock') + end - expect(find('#notes')).to have_content('locked the discussion in this issue') + expect(find('#notes')).to have_content('locked the discussion in this issue') + end end - end - context 'when the discussion is locked' do - before do - issue.update_attribute(:discussion_locked, true) - visit project_issue_path(project, issue) + context 'when the discussion is locked' do + before do + issue.update_attribute(:discussion_locked, true) + visit project_issue_path(project, issue) + end + + it 'the user can unlock the issue' do + expect(find('.issuable-sidebar')).to have_content('Locked') + + page.within('.issuable-sidebar') do + find('.lock-edit').click + click_button('Unlock') + end + + expect(find('#notes')).to have_content('unlocked the discussion in this issue') + expect(find('.issuable-sidebar')).to have_content('Unlocked') + end + + it 'the user can create a comment' do + page.within('#notes .js-main-target-form') do + fill_in 'note[note]', with: 'Some new comment' + click_button 'Comment' + end + + wait_for_requests + + expect(find('div#notes')).to have_content('Some new comment') + end end + end - it 'the user can unlock the issue' do - expect(find('.issuable-sidebar')).to have_content('Locked') + context 'when a user is not a team member' do + context 'when the discussion is unlocked' do + before do + visit project_issue_path(project, issue) + end - page.within('.issuable-sidebar') do - find('.lock-edit').click - click_button('Unlock') + it 'the user can not lock the issue' do + expect(find('.issuable-sidebar')).to have_content('Unlocked') + expect(find('.issuable-sidebar')).not_to have_selector('.lock-edit') end - expect(find('#notes')).to have_content('unlocked the discussion in this issue') - expect(find('.issuable-sidebar')).to have_content('Unlocked') + it 'the user can create a comment' do + page.within('#notes .js-main-target-form') do + fill_in 'note[note]', with: 'Some new comment' + click_button 'Comment' + end + + wait_for_requests + + expect(find('div#notes')).to have_content('Some new comment') + end end - it 'the user can create a comment' do - page.within('#notes .js-main-target-form') do - fill_in 'note[note]', with: 'Some new comment' - click_button 'Comment' + context 'when the discussion is locked' do + before do + issue.update_attribute(:discussion_locked, true) + visit project_issue_path(project, issue) end - wait_for_requests + it 'the user can not unlock the issue' do + expect(find('.issuable-sidebar')).to have_content('Locked') + expect(find('.issuable-sidebar')).not_to have_selector('.lock-edit') + end - expect(find('div#notes')).to have_content('Some new comment') + it 'the user can not create a comment' do + page.within('#notes') do + expect(page).not_to have_selector('.js-main-target-form') + expect(find_by_testid('disabled-comments')) + .to have_content('The discussion in this issue is locked. Only project members can comment.') + end + end end end - end - context 'when a user is not a team member' do - context 'when the discussion is unlocked' do + context 'for axe automated accessibility testing' do before do + project.add_developer(user) + issue.update_attribute(:discussion_locked, true) visit project_issue_path(project, issue) end - it 'the user can not lock the issue' do - expect(find('.issuable-sidebar')).to have_content('Unlocked') - expect(find('.issuable-sidebar')).not_to have_selector('.lock-edit') + it 'passes tests' do + # rubocop:disable Capybara/TestidFinders -- within_testid does not work here + expect(page).to be_axe_clean.within(locked_badge) + expect(page).to be_axe_clean.within(issuable_note_warning) + # rubocop:enable Capybara/TestidFinders + page.within('.issuable-sidebar') do + find('.lock-edit').click + expect(page).to be_axe_clean.within('.lock-edit') + end end + end + end + + context 'when feature flag is enabled' do + before do + sign_in(user) + stub_feature_flags(moved_mr_sidebar: true) + end + + context 'when a user is a team member' do + before do + project.add_developer(user) + end + + context 'when the discussion is unlocked' do + it 'the user can lock the issue' do + visit project_issue_path(project, issue) + + more_dropdown.click + expect(issuable_lock).to have_content('Lock discussion') - it 'the user can create a comment' do - page.within('#notes .js-main-target-form') do - fill_in 'note[note]', with: 'Some new comment' - click_button 'Comment' + issuable_lock.click + expect(find('#notes')).to have_content('locked the discussion in this issue') + end + end + + context 'when the discussion is locked' do + before do + issue.update_attribute(:discussion_locked, true) + visit project_issue_path(project, issue) end - wait_for_requests + it 'the user can unlock the issue' do + more_dropdown.click + expect(issuable_lock).to have_content('Unlock discussion') + + issuable_lock.click + expect(find('#notes')).to have_content('unlocked the discussion in this issue') + expect(issuable_lock).to have_content('Lock discussion') + end - expect(find('div#notes')).to have_content('Some new comment') + it 'the user can create a comment' do + page.within('#notes .js-main-target-form') do + fill_in 'note[note]', with: 'Some new comment' + click_button 'Comment' + end + + wait_for_requests + + expect(find('div#notes')).to have_content('Some new comment') + end end end - context 'when the discussion is locked' do - before do - issue.update_attribute(:discussion_locked, true) - visit project_issue_path(project, issue) - end + context 'when a user is not a team member' do + context 'when the discussion is unlocked' do + before do + visit project_issue_path(project, issue) + end - it 'the user can not unlock the issue' do - expect(find('.issuable-sidebar')).to have_content('Locked') - expect(find('.issuable-sidebar')).not_to have_selector('.lock-edit') + it 'the user can not lock the issue' do + more_dropdown.click + expect(issuable_lock).to have_content('Lock discussion') + end + + it 'the user can create a comment' do + page.within('#notes .js-main-target-form') do + fill_in 'note[note]', with: 'Some new comment' + click_button 'Comment' + end + + wait_for_requests + + expect(find('div#notes')).to have_content('Some new comment') + end end - it 'the user can not create a comment' do - page.within('#notes') do - expect(page).not_to have_selector('js-main-target-form') - expect(find_by_testid('disabled-comments')) - .to have_content('The discussion in this issue is locked. Only project members can comment.') + context 'when the discussion is locked' do + before do + issue.update_attribute(:discussion_locked, true) + visit project_issue_path(project, issue) + end + + it 'the user can not unlock the issue' do + more_dropdown.click + expect(issuable_lock).to have_content('Unlock discussion') + end + + it 'the user can not create a comment' do + page.within('#notes') do + expect(page).not_to have_selector('js-main-target-form') + expect(find_by_testid('disabled-comments')) + .to have_content('The discussion in this issue is locked. Only project members can comment.') + end end end end + + it 'passes axe automated accessibility testing' do + project.add_developer(user) + issue.update_attribute(:discussion_locked, true) + visit project_issue_path(project, issue) + wait_for_all_requests + + # rubocop:disable Capybara/TestidFinders -- within_testid does not work here + expect(page).to be_axe_clean.within(locked_badge) + expect(page).to be_axe_clean.within(issuable_note_warning) + + more_dropdown.click + expect(page).to be_axe_clean.within('[data-testid="lock-issue-toggle"] button') + # rubocop:enable Capybara/TestidFinders + end end end diff --git a/spec/lib/backup/database_configuration_spec.rb b/spec/lib/backup/database_configuration_spec.rb new file mode 100644 index 00000000000..b7fa9f161c1 --- /dev/null +++ b/spec/lib/backup/database_configuration_spec.rb @@ -0,0 +1,239 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Backup::DatabaseConfiguration, :reestablished_active_record_base, feature_category: :backup_restore do + using RSpec::Parameterized::TableSyntax + + let(:connection_name) { 'main' } + + subject(:config) { described_class.new(connection_name) } + + describe '#initialize' do + it 'initializes with the provided connection_name' do + expect_next_instance_of(described_class) do |config| + expect(config.connection_name).to eq(connection_name) + end + + config + end + end + + describe '#activerecord_configuration' do + it 'returns a ActiveRecord::DatabaseConfigurations::HashConfig' do + expect(config.activerecord_configuration).to be_a ActiveRecord::DatabaseConfigurations::HashConfig + end + end + + context 'with configuration override feature' do + let(:application_config) do + { + adapter: 'postgresql', + host: 'some_host', + port: '5432' + } + end + + let(:active_record_key) { described_class::SUPPORTED_OVERRIDES.invert[pg_env] } + + before do + allow(config).to receive(:original_activerecord_config).and_return(application_config) + end + + shared_context 'with generic database with overridden values' do + where(:env_variable, :overridden_value) do + 'GITLAB_BACKUP_PGHOST' | 'test.invalid.' + 'GITLAB_BACKUP_PGUSER' | 'some_user' + 'GITLAB_BACKUP_PGPORT' | '1543' + 'GITLAB_BACKUP_PGPASSWORD' | 'secret' + 'GITLAB_BACKUP_PGSSLMODE' | 'allow' + 'GITLAB_BACKUP_PGSSLKEY' | 'some_key' + 'GITLAB_BACKUP_PGSSLCERT' | '/path/to/cert' + 'GITLAB_BACKUP_PGSSLROOTCERT' | '/path/to/root/cert' + 'GITLAB_BACKUP_PGSSLCRL' | '/path/to/crl' + 'GITLAB_BACKUP_PGSSLCOMPRESSION' | '1' + 'GITLAB_OVERRIDE_PGHOST' | 'test.invalid.' + 'GITLAB_OVERRIDE_PGUSER' | 'some_user' + 'GITLAB_OVERRIDE_PGPORT' | '1543' + 'GITLAB_OVERRIDE_PGPASSWORD' | 'secret' + 'GITLAB_OVERRIDE_PGSSLMODE' | 'allow' + 'GITLAB_OVERRIDE_PGSSLKEY' | 'some_key' + 'GITLAB_OVERRIDE_PGSSLCERT' | '/path/to/cert' + 'GITLAB_OVERRIDE_PGSSLROOTCERT' | '/path/to/root/cert' + 'GITLAB_OVERRIDE_PGSSLCRL' | '/path/to/crl' + 'GITLAB_OVERRIDE_PGSSLCOMPRESSION' | '1' + end + end + + shared_context 'with generic database with overridden values using current database prefix' do + where(:env_variable, :overridden_value) do + 'GITLAB_BACKUP_MAIN_PGHOST' | 'test.invalid.' + 'GITLAB_BACKUP_MAIN_PGUSER' | 'some_user' + 'GITLAB_BACKUP_MAIN_PGPORT' | '1543' + 'GITLAB_BACKUP_MAIN_PGPASSWORD' | 'secret' + 'GITLAB_BACKUP_MAIN_PGSSLMODE' | 'allow' + 'GITLAB_BACKUP_MAIN_PGSSLKEY' | 'some_key' + 'GITLAB_BACKUP_MAIN_PGSSLCERT' | '/path/to/cert' + 'GITLAB_BACKUP_MAIN_PGSSLROOTCERT' | '/path/to/root/cert' + 'GITLAB_BACKUP_MAIN_PGSSLCRL' | '/path/to/crl' + 'GITLAB_BACKUP_MAIN_PGSSLCOMPRESSION' | '1' + 'GITLAB_OVERRIDE_MAIN_PGHOST' | 'test.invalid.' + 'GITLAB_OVERRIDE_MAIN_PGUSER' | 'some_user' + 'GITLAB_OVERRIDE_MAIN_PGPORT' | '1543' + 'GITLAB_OVERRIDE_MAIN_PGPASSWORD' | 'secret' + 'GITLAB_OVERRIDE_MAIN_PGSSLMODE' | 'allow' + 'GITLAB_OVERRIDE_MAIN_PGSSLKEY' | 'some_key' + 'GITLAB_OVERRIDE_MAIN_PGSSLCERT' | '/path/to/cert' + 'GITLAB_OVERRIDE_MAIN_PGSSLROOTCERT' | '/path/to/root/cert' + 'GITLAB_OVERRIDE_MAIN_PGSSLCRL' | '/path/to/crl' + 'GITLAB_OVERRIDE_MAIN_PGSSLCOMPRESSION' | '1' + end + end + + shared_context 'with generic database with overridden values for a different database prefix' do + where(:env_variable, :overridden_value) do + 'GITLAB_BACKUP_CI_PGHOST' | 'test.invalid.' + 'GITLAB_BACKUP_CI_PGUSER' | 'some_user' + 'GITLAB_BACKUP_CI_PGPORT' | '1543' + 'GITLAB_BACKUP_CI_PGPASSWORD' | 'secret' + 'GITLAB_BACKUP_CI_PGSSLMODE' | 'allow' + 'GITLAB_BACKUP_CI_PGSSLKEY' | 'some_key' + 'GITLAB_BACKUP_CI_PGSSLCERT' | '/path/to/cert' + 'GITLAB_BACKUP_CI_PGSSLROOTCERT' | '/path/to/root/cert' + 'GITLAB_BACKUP_CI_PGSSLCRL' | '/path/to/crl' + 'GITLAB_BACKUP_CI_PGSSLCOMPRESSION' | '1' + 'GITLAB_OVERRIDE_CI_PGHOST' | 'test.invalid.' + 'GITLAB_OVERRIDE_CI_PGUSER' | 'some_user' + 'GITLAB_OVERRIDE_CI_PGPORT' | '1543' + 'GITLAB_OVERRIDE_CI_PGPASSWORD' | 'secret' + 'GITLAB_OVERRIDE_CI_PGSSLMODE' | 'allow' + 'GITLAB_OVERRIDE_CI_PGSSLKEY' | 'some_key' + 'GITLAB_OVERRIDE_CI_PGSSLCERT' | '/path/to/cert' + 'GITLAB_OVERRIDE_CI_PGSSLROOTCERT' | '/path/to/root/cert' + 'GITLAB_OVERRIDE_CI_PGSSLCRL' | '/path/to/crl' + 'GITLAB_OVERRIDE_CI_PGSSLCOMPRESSION' | '1' + end + end + + describe('#pg_env_variables') do + context 'with provided ENV variables' do + before do + stub_env(env_variable, overridden_value) + end + + context 'when generic database configuration is overridden' do + include_context "with generic database with overridden values" + + with_them do + let(:pg_env) { env_variable[/GITLAB_(BACKUP|OVERRIDE)_(\w+)/, 2] } + + it 'PostgreSQL ENV overrides application configuration' do + expect(config.pg_env_variables).to include({ pg_env => overridden_value }) + end + end + end + + context 'when specific database configuration is overridden' do + context 'and environment variables are for the current database name' do + include_context 'with generic database with overridden values using current database prefix' + + with_them do + let(:pg_env) { env_variable[/GITLAB_(BACKUP|OVERRIDE)_MAIN_(\w+)/, 2] } + + it 'PostgreSQL ENV overrides application configuration' do + expect(config.pg_env_variables).to include({ pg_env => overridden_value }) + end + end + end + + context 'and environment variables are for another database' do + include_context 'with generic database with overridden values for a different database prefix' + + with_them do + let(:pg_env) { env_variable[/GITLAB_(BACKUP|OVERRIDE)_CI_(\w+)/, 1] } + + it 'PostgreSQL ENV is expected to equal application configuration' do + expect(config.pg_env_variables).to eq( + { + 'PGHOST' => application_config[:host], + 'PGPORT' => application_config[:port] + } + ) + end + end + end + end + end + + context 'when both GITLAB_BACKUP_PGUSER and GITLAB_BACKUP_MAIN_PGUSER variable are present' do + it 'prefers more specific GITLAB_BACKUP_MAIN_PGUSER' do + stub_env('GITLAB_BACKUP_PGUSER', 'generic_user') + stub_env('GITLAB_BACKUP_MAIN_PGUSER', 'specific_user') + + expect(config.pg_env_variables['PGUSER']).to eq('specific_user') + end + end + end + + describe('#activerecord_variables') do + context 'with provided ENV variables' do + before do + stub_env(env_variable, overridden_value) + end + + context 'when generic database configuration is overridden' do + include_context "with generic database with overridden values" + + with_them do + let(:pg_env) { env_variable[/GITLAB_(BACKUP|OVERRIDE)_(\w+)/, 2] } + + it 'ActiveRecord backup configuration overrides application configuration' do + expect(config.activerecord_variables).to eq( + application_config.merge(active_record_key => overridden_value) + ) + end + end + end + + context 'when specific database configuration is overridden' do + context 'and environment variables are for the current database name' do + include_context 'with generic database with overridden values using current database prefix' + + with_them do + let(:pg_env) { env_variable[/GITLAB_(BACKUP|OVERRIDE)_MAIN_(\w+)/, 2] } + + it 'ActiveRecord backup configuration overrides application configuration' do + expect(config.activerecord_variables).to eq( + application_config.merge(active_record_key => overridden_value) + ) + end + end + end + + context 'and environment variables are for another database' do + include_context 'with generic database with overridden values for a different database prefix' + + with_them do + let(:pg_env) { env_variable[/GITLAB_(BACKUP|OVERRIDE)_CI_(\w+)/, 1] } + + it 'ActiveRecord backup configuration is expected to equal application configuration' do + expect(config.activerecord_variables).to eq(application_config) + end + end + end + end + end + + context 'when both GITLAB_BACKUP_PGUSER and GITLAB_BACKUP_MAIN_PGUSER variable are present' do + with_them do + it 'prefers more specific GITLAB_BACKUP_MAIN_PGUSER' do + stub_env('GITLAB_BACKUP_PGUSER', 'generic_user') + stub_env('GITLAB_BACKUP_MAIN_PGUSER', 'specific_user') + + expect(config.activerecord_variables[:username]).to eq('specific_user') + end + end + end + end + end +end diff --git a/spec/lib/backup/database_connection_spec.rb b/spec/lib/backup/database_connection_spec.rb new file mode 100644 index 00000000000..b56da3d99f7 --- /dev/null +++ b/spec/lib/backup/database_connection_spec.rb @@ -0,0 +1,103 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Backup::DatabaseConnection, :reestablished_active_record_base, feature_category: :backup_restore do + let(:connection_name) { 'main' } + let(:snapshot_id_pattern) { /[A-Z0-9]{8}-[A-Z0-9]{8}-[0-9]/ } + + subject(:backup_connection) { described_class.new(connection_name) } + + describe '#initialize' do + it 'initializes database_configuration with the provided connection_name' do + expect(Backup::DatabaseConfiguration).to receive(:new).with(connection_name).and_call_original + + backup_connection + end + end + + describe '#connection_name' do + it 'returns the same connection name used during initialization' do + expect(backup_connection.connection_name).to eq(connection_name) + end + end + + describe '#connection' do + it 'is an instance of a ActiveRecord::Base.connection' do + backup_connection.connection.is_a? Gitlab::Database::LoadBalancing::ConnectionProxy + end + end + + describe '#database_configuration' do + it 'returns database configuration' do + expect(backup_connection.database_configuration).to be_a(Backup::DatabaseConfiguration) + end + end + + describe '#snapshot_id' do + it "returns nil when snapshot has not been triggered" do + expect(backup_connection.snapshot_id).to be_nil + end + + context 'when a snapshot transaction is open', :delete do + let!(:snapshot_id) { backup_connection.export_snapshot! } + + it 'returns the snapshot_id in the expected format' do + expect(backup_connection.snapshot_id).to match(snapshot_id_pattern) + end + + it 'returns the snapshot_id equal to the one returned by #export_snapshot!' do + expect(backup_connection.snapshot_id).to eq(snapshot_id) + end + + it "returns nil after a snapshot is released" do + backup_connection.release_snapshot! + + expect(backup_connection.snapshot_id).to be_nil + end + end + end + + describe '#export_snapshot!', :delete do + it 'returns a snapshot_id in the expected format' do + expect(backup_connection.export_snapshot!).to match(snapshot_id_pattern) + end + + it 'opens a transaction with correct isolation format and triggers a snapshot generation' do + expect(backup_connection.connection).to receive(:begin_transaction).with( + isolation: :repeatable_read + ).and_call_original + + expect(backup_connection.connection).to receive(:select_value).with( + "SELECT pg_export_snapshot()" + ).and_call_original + + backup_connection.export_snapshot! + end + + it 'disables transaction time out' do + expect_next_instance_of(Gitlab::Database::TransactionTimeoutSettings) do |transaction_settings| + expect(transaction_settings).to receive(:disable_timeouts).and_call_original + end + + backup_connection.export_snapshot! + end + end + + describe '#release_snapshot!', :delete do + it 'clears out existing snapshot_id' do + snapshot_id = backup_connection.export_snapshot! + + expect { backup_connection.release_snapshot! }.to change { backup_connection.snapshot_id } + .from(snapshot_id).to(nil) + end + + it 'executes a transaction rollback' do + backup_connection.export_snapshot! + + expect(backup_connection.connection).to receive(:rollback_transaction).and_call_original + + backup_connection.release_snapshot! + end + end +end diff --git a/spec/lib/backup/database_model_spec.rb b/spec/lib/backup/database_model_spec.rb deleted file mode 100644 index 522bba80972..00000000000 --- a/spec/lib/backup/database_model_spec.rb +++ /dev/null @@ -1,186 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Backup::DatabaseModel, :reestablished_active_record_base, feature_category: :backup_restore do - using RSpec::Parameterized::TableSyntax - - let(:gitlab_database_name) { 'main' } - - describe '#connection' do - subject { described_class.new(gitlab_database_name).connection } - - it 'an instance of a ActiveRecord::Base.connection' do - subject.is_a? ActiveRecord::Base.connection.class # rubocop:disable Database/MultipleDatabases - end - end - - describe '#config' do - let(:application_config) do - { - adapter: 'postgresql', - host: 'some_host', - port: '5432' - } - end - - subject { described_class.new(gitlab_database_name).config } - - before do - allow( - Gitlab::Database.database_base_models_with_gitlab_shared[gitlab_database_name].connection_db_config - ).to receive(:configuration_hash).and_return(application_config) - end - - shared_examples 'no configuration is overridden' do - it 'ActiveRecord backup configuration is expected to equal application configuration' do - expect(subject[:activerecord]).to eq(application_config) - end - - it 'PostgreSQL ENV is expected to equal application configuration' do - expect(subject[:pg_env]).to eq( - { - 'PGHOST' => application_config[:host], - 'PGPORT' => application_config[:port] - } - ) - end - end - - shared_examples 'environment variables override application configuration' do - let(:active_record_key) { described_class::SUPPORTED_OVERRIDES.invert[pg_env] } - - it 'ActiveRecord backup configuration overrides application configuration' do - expect(subject[:activerecord]).to eq(application_config.merge(active_record_key => overridden_value)) - end - - it 'PostgreSQL ENV overrides application configuration' do - expect(subject[:pg_env]).to include({ pg_env => overridden_value }) - end - end - - context 'when no GITLAB_BACKUP_PG* variables are set' do - it_behaves_like 'no configuration is overridden' - end - - context 'when generic database configuration is overridden' do - where(:env_variable, :overridden_value) do - 'GITLAB_BACKUP_PGHOST' | 'test.invalid.' - 'GITLAB_BACKUP_PGUSER' | 'some_user' - 'GITLAB_BACKUP_PGPORT' | '1543' - 'GITLAB_BACKUP_PGPASSWORD' | 'secret' - 'GITLAB_BACKUP_PGSSLMODE' | 'allow' - 'GITLAB_BACKUP_PGSSLKEY' | 'some_key' - 'GITLAB_BACKUP_PGSSLCERT' | '/path/to/cert' - 'GITLAB_BACKUP_PGSSLROOTCERT' | '/path/to/root/cert' - 'GITLAB_BACKUP_PGSSLCRL' | '/path/to/crl' - 'GITLAB_BACKUP_PGSSLCOMPRESSION' | '1' - 'GITLAB_OVERRIDE_PGHOST' | 'test.invalid.' - 'GITLAB_OVERRIDE_PGUSER' | 'some_user' - 'GITLAB_OVERRIDE_PGPORT' | '1543' - 'GITLAB_OVERRIDE_PGPASSWORD' | 'secret' - 'GITLAB_OVERRIDE_PGSSLMODE' | 'allow' - 'GITLAB_OVERRIDE_PGSSLKEY' | 'some_key' - 'GITLAB_OVERRIDE_PGSSLCERT' | '/path/to/cert' - 'GITLAB_OVERRIDE_PGSSLROOTCERT' | '/path/to/root/cert' - 'GITLAB_OVERRIDE_PGSSLCRL' | '/path/to/crl' - 'GITLAB_OVERRIDE_PGSSLCOMPRESSION' | '1' - end - - with_them do - let(:pg_env) { env_variable[/GITLAB_(BACKUP|OVERRIDE)_(\w+)/, 2] } - - before do - stub_env(env_variable, overridden_value) - end - - it_behaves_like 'environment variables override application configuration' - end - end - - context 'when specific database configuration is overridden' do - context 'and environment variables are for the current database name' do - where(:env_variable, :overridden_value) do - 'GITLAB_BACKUP_MAIN_PGHOST' | 'test.invalid.' - 'GITLAB_BACKUP_MAIN_PGUSER' | 'some_user' - 'GITLAB_BACKUP_MAIN_PGPORT' | '1543' - 'GITLAB_BACKUP_MAIN_PGPASSWORD' | 'secret' - 'GITLAB_BACKUP_MAIN_PGSSLMODE' | 'allow' - 'GITLAB_BACKUP_MAIN_PGSSLKEY' | 'some_key' - 'GITLAB_BACKUP_MAIN_PGSSLCERT' | '/path/to/cert' - 'GITLAB_BACKUP_MAIN_PGSSLROOTCERT' | '/path/to/root/cert' - 'GITLAB_BACKUP_MAIN_PGSSLCRL' | '/path/to/crl' - 'GITLAB_BACKUP_MAIN_PGSSLCOMPRESSION' | '1' - 'GITLAB_OVERRIDE_MAIN_PGHOST' | 'test.invalid.' - 'GITLAB_OVERRIDE_MAIN_PGUSER' | 'some_user' - 'GITLAB_OVERRIDE_MAIN_PGPORT' | '1543' - 'GITLAB_OVERRIDE_MAIN_PGPASSWORD' | 'secret' - 'GITLAB_OVERRIDE_MAIN_PGSSLMODE' | 'allow' - 'GITLAB_OVERRIDE_MAIN_PGSSLKEY' | 'some_key' - 'GITLAB_OVERRIDE_MAIN_PGSSLCERT' | '/path/to/cert' - 'GITLAB_OVERRIDE_MAIN_PGSSLROOTCERT' | '/path/to/root/cert' - 'GITLAB_OVERRIDE_MAIN_PGSSLCRL' | '/path/to/crl' - 'GITLAB_OVERRIDE_MAIN_PGSSLCOMPRESSION' | '1' - end - - with_them do - let(:pg_env) { env_variable[/GITLAB_(BACKUP|OVERRIDE)_MAIN_(\w+)/, 2] } - - before do - stub_env(env_variable, overridden_value) - end - - it_behaves_like 'environment variables override application configuration' - end - end - - context 'and environment variables are for another database' do - where(:env_variable, :overridden_value) do - 'GITLAB_BACKUP_CI_PGHOST' | 'test.invalid.' - 'GITLAB_BACKUP_CI_PGUSER' | 'some_user' - 'GITLAB_BACKUP_CI_PGPORT' | '1543' - 'GITLAB_BACKUP_CI_PGPASSWORD' | 'secret' - 'GITLAB_BACKUP_CI_PGSSLMODE' | 'allow' - 'GITLAB_BACKUP_CI_PGSSLKEY' | 'some_key' - 'GITLAB_BACKUP_CI_PGSSLCERT' | '/path/to/cert' - 'GITLAB_BACKUP_CI_PGSSLROOTCERT' | '/path/to/root/cert' - 'GITLAB_BACKUP_CI_PGSSLCRL' | '/path/to/crl' - 'GITLAB_BACKUP_CI_PGSSLCOMPRESSION' | '1' - 'GITLAB_OVERRIDE_CI_PGHOST' | 'test.invalid.' - 'GITLAB_OVERRIDE_CI_PGUSER' | 'some_user' - 'GITLAB_OVERRIDE_CI_PGPORT' | '1543' - 'GITLAB_OVERRIDE_CI_PGPASSWORD' | 'secret' - 'GITLAB_OVERRIDE_CI_PGSSLMODE' | 'allow' - 'GITLAB_OVERRIDE_CI_PGSSLKEY' | 'some_key' - 'GITLAB_OVERRIDE_CI_PGSSLCERT' | '/path/to/cert' - 'GITLAB_OVERRIDE_CI_PGSSLROOTCERT' | '/path/to/root/cert' - 'GITLAB_OVERRIDE_CI_PGSSLCRL' | '/path/to/crl' - 'GITLAB_OVERRIDE_CI_PGSSLCOMPRESSION' | '1' - end - - with_them do - let(:pg_env) { env_variable[/GITLAB_(BACKUP|OVERRIDE)_CI_(\w+)/, 1] } - - before do - stub_env(env_variable, overridden_value) - end - - it_behaves_like 'no configuration is overridden' - end - end - - context 'when both GITLAB_BACKUP_PGUSER and GITLAB_BACKUP_MAIN_PGUSER variable are present' do - before do - stub_env('GITLAB_BACKUP_PGUSER', 'generic_user') - stub_env('GITLAB_BACKUP_MAIN_PGUSER', 'specfic_user') - end - - it 'prefers more specific GITLAB_BACKUP_MAIN_PGUSER' do - config = subject - expect(config.dig(:activerecord, :username)).to eq('specfic_user') - expect(config.dig(:pg_env, 'PGUSER')).to eq('specfic_user') - end - end - end - end -end diff --git a/spec/lib/backup/database_spec.rb b/spec/lib/backup/database_spec.rb index 26d09c275c1..86468689f76 100644 --- a/spec/lib/backup/database_spec.rb +++ b/spec/lib/backup/database_spec.rb @@ -48,28 +48,16 @@ RSpec.describe Backup::Database, :reestablished_active_record_base, feature_cate it 'uses snapshots' do Dir.mktmpdir do |dir| - expect_next_instances_of(Backup::DatabaseModel, 2) do |adapter| - expect(adapter.connection).to receive(:begin_transaction).with( - isolation: :repeatable_read - ).and_call_original - expect(adapter.connection).to receive(:select_value).with( - "SELECT pg_export_snapshot()" - ).and_call_original - expect(adapter.connection).to receive(:rollback_transaction).and_call_original - end + expect_next_instances_of(Backup::DatabaseConnection, 2) do |backup_connection| + expect(backup_connection).to receive(:export_snapshot!).and_call_original - subject.dump(dir, backup_id) - end - end + expect_next_instance_of(::Gitlab::Backup::Cli::Utils::PgDump) do |pgdump| + expect(pgdump.snapshot_id).to eq(backup_connection.snapshot_id) + end - it 'disables transaction time out' do - number_of_databases = base_models_for_backup.count - expect(Gitlab::Database::TransactionTimeoutSettings) - .to receive(:new).exactly(2 * number_of_databases).times.and_return(timeout_service) - expect(timeout_service).to receive(:disable_timeouts).exactly(number_of_databases).times - expect(timeout_service).to receive(:restore_timeouts).exactly(number_of_databases).times + expect(backup_connection).to receive(:release_snapshot!).and_call_original + end - Dir.mktmpdir do |dir| subject.dump(dir, backup_id) end end @@ -82,79 +70,18 @@ RSpec.describe Backup::Database, :reestablished_active_record_base, feature_cate it 'does not use snapshots' do Dir.mktmpdir do |dir| - base_model = Backup::DatabaseModel.new('main') - expect(base_model.connection).not_to receive(:begin_transaction).with( - isolation: :repeatable_read - ).and_call_original - expect(base_model.connection).not_to receive(:select_value).with( - "SELECT pg_export_snapshot()" - ).and_call_original - expect(base_model.connection).not_to receive(:rollback_transaction).and_call_original - - subject.dump(dir, backup_id) - end - end - end - - describe 'pg_dump arguments' do - let(:snapshot_id) { 'fake_id' } - let(:default_pg_args) do - args = [ - '--clean', - '--if-exists' - ] - - if Gitlab::Database.database_mode == Gitlab::Database::MODE_MULTIPLE_DATABASES - args + ["--snapshot=#{snapshot_id}"] - else - args - end - end + expect_next_instance_of(Backup::DatabaseConnection) do |backup_connection| + expect(backup_connection).not_to receive(:export_snapshot!) - let(:dumper) { double } - let(:destination_dir) { 'tmp' } - - before do - allow(Backup::Dump::Postgres).to receive(:new).and_return(dumper) - allow(dumper).to receive(:dump).with(any_args).and_return(true) - end - - shared_examples 'pg_dump arguments' do - it 'calls Backup::Dump::Postgres with correct pg_dump arguments' do - number_of_databases = base_models_for_backup.count - if number_of_databases > 1 - expect_next_instances_of(Backup::DatabaseModel, number_of_databases) do |model| - expect(model.connection).to receive(:select_value).with( - "SELECT pg_export_snapshot()" - ).and_return(snapshot_id) + expect_next_instance_of(::Gitlab::Backup::Cli::Utils::PgDump) do |pgdump| + expect(pgdump.snapshot_id).to be_nil end - end - - expect(dumper).to receive(:dump).with(anything, anything, expected_pg_args) - subject.dump(destination_dir, backup_id) - end - end - - context 'when no PostgreSQL schemas are specified' do - let(:expected_pg_args) { default_pg_args } - - include_examples 'pg_dump arguments' - end - - context 'when a PostgreSQL schema is used' do - let(:schema) { 'gitlab' } - let(:expected_pg_args) do - default_pg_args + ['-n', schema] + Gitlab::Database::EXTRA_SCHEMAS.flat_map do |schema| - ['-n', schema.to_s] + expect(backup_connection).not_to receive(:release_snapshot!) end - end - before do - allow(Gitlab.config.backup).to receive(:pg_schema).and_return(schema) + subject.dump(dir, backup_id) end - - include_examples 'pg_dump arguments' end end diff --git a/spec/lib/backup/dump/postgres_spec.rb b/spec/lib/backup/dump/postgres_spec.rb index f7020c78c6c..1da2ee950db 100644 --- a/spec/lib/backup/dump/postgres_spec.rb +++ b/spec/lib/backup/dump/postgres_spec.rb @@ -3,17 +3,22 @@ require 'spec_helper' RSpec.describe Backup::Dump::Postgres, feature_category: :backup_restore do - describe '#dump' do - let(:pg_database) { 'gitlabhq_test' } - let(:destination_dir) { Dir.mktmpdir } - let(:db_file_name) { File.join(destination_dir, 'output.gz') } + let(:pg_database) { 'gitlabhq_test' } + let(:pg_dump) { ::Gitlab::Backup::Cli::Utils::PgDump.new(database_name: pg_database) } + let(:default_compression_cmd) { 'gzip -c -1' } - let(:pipes) { IO.pipe } - let(:gzip_pid) { spawn('gzip -c -1', in: pipes[0], out: [db_file_name, 'w', 0o600]) } - let(:pg_dump_pid) { Process.spawn('pg_dump', *args, pg_database, out: pipes[1]) } - let(:args) { ['--help'] } + subject(:postgres) { described_class.new } - subject { described_class.new } + describe '#compress_cmd' do + it 'returns default compression command' do + expect(postgres.compress_cmd).to eq(default_compression_cmd) + end + end + + describe '#dump' do + let(:pipes) { IO.pipe } + let(:destination_dir) { Dir.mktmpdir } + let(:dump_file_name) { File.join(destination_dir, 'output.gz') } before do allow(IO).to receive(:pipe).and_return(pipes) @@ -23,33 +28,54 @@ RSpec.describe Backup::Dump::Postgres, feature_category: :backup_restore do FileUtils.remove_entry destination_dir end - it 'creates gzipped dump using supplied arguments' do - expect(subject).to receive(:spawn).with('gzip -c -1', in: pipes.first, - out: [db_file_name, 'w', 0o600]).and_return(gzip_pid) - expect(Process).to receive(:spawn).with('pg_dump', *args, pg_database, out: pipes[1]).and_return(pg_dump_pid) + context 'with default compression method' do + it 'creates a dump file' do + postgres.dump(dump_file_name, pg_dump) - subject.dump(pg_database, db_file_name, args) + expect(File.exist?(dump_file_name)).to eq(true) + end + + it 'default compression command is used' do + compressor_pid = spawn(default_compression_cmd, in: pipes[0], out: [dump_file_name, 'w', 0o600]) + + expect(postgres).to receive(:spawn).with( + default_compression_cmd, + in: pipes.first, + out: [dump_file_name, 'w', 0o600]).and_return(compressor_pid) - expect(File.exist?(db_file_name)).to eq(true) + postgres.dump(dump_file_name, pg_dump) + + expect(File.exist?(dump_file_name)).to eq(true) + end end context 'when COMPRESS_CMD is set to tee' do - let(:tee_pid) { spawn('tee', in: pipes[0], out: [db_file_name, 'w', 0o600]) } + let(:tee_pid) { spawn('tee', in: pipes[0], out: [dump_file_name, 'w', 0o600]) } before do stub_env('COMPRESS_CMD', 'tee') end + it 'creates a dump file' do + postgres.dump(dump_file_name, pg_dump) + + expect(File.exist?(dump_file_name)).to eq(true) + end + it 'passes through tee instead of gzip' do - expect(subject).to receive(:spawn).with('tee', in: pipes.first, - out: [db_file_name, 'w', 0o600]).and_return(tee_pid) - expect(Process).to receive(:spawn).with('pg_dump', *args, pg_database, out: pipes[1]).and_return(pg_dump_pid) + custom_compression_command = 'tee' + compressor_pid = spawn(custom_compression_command, in: pipes[0], out: [dump_file_name, 'w', 0o600]) + + expect(postgres).to receive(:spawn).with( + custom_compression_command, + in: pipes.first, + out: [dump_file_name, 'w', 0o600]).and_return(compressor_pid) expect do - subject.dump(pg_database, db_file_name, args) + postgres.dump(dump_file_name, pg_dump) end.to output(/Using custom COMPRESS_CMD 'tee'/).to_stdout - expect(File.exist?(db_file_name)).to eq(true) + expect(File.exist?(dump_file_name)).to eq(true) end end end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 08679626c21..eeb0bbb8e7d 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -2224,46 +2224,82 @@ RSpec.describe Repository, feature_category: :source_code_management do describe 'rolling back the `rebase_commit_sha`' do let(:new_sha) { Digest::SHA1.hexdigest('foo') } - it 'does not rollback when there are no errors' do - second_response = double(pre_receive_error: nil, git_error: nil) - mock_gitaly(second_response) + context 'when there are no errors' do + before do + responses = [ + double(rebase_sha: new_sha).as_null_object, + double + ] + + expect_any_instance_of( + Gitaly::OperationService::Stub + ).to receive(:user_rebase_confirmable).and_return(responses.each) + end - repository.rebase(user, merge_request) + it 'does not rollback when there are no errors' do + repository.rebase(user, merge_request) - expect(merge_request.reload.rebase_commit_sha).to eq(new_sha) + expect(merge_request.reload.rebase_commit_sha).to eq(new_sha) + end end - it 'does rollback when a PreReceiveError is encountered in the second step' do - second_response = double(pre_receive_error: 'my_error', git_error: nil) - mock_gitaly(second_response) + context 'when there was an error' do + let(:first_response) do + double(rebase_sha: new_sha).as_null_object + end - expect do - repository.rebase(user, merge_request) - end.to raise_error(Gitlab::Git::PreReceiveError) + before do + request_enum = double(push: nil).as_null_object + allow(Gitlab::GitalyClient::QueueEnumerator).to receive(:new).and_return(request_enum) - expect(merge_request.reload.rebase_commit_sha).to be_nil - end + expect_any_instance_of( + Gitaly::OperationService::Stub + ).to receive(:user_rebase_confirmable).and_return(first_response) - it 'does rollback when a GitError is encountered in the second step' do - second_response = double(pre_receive_error: nil, git_error: 'git error') - mock_gitaly(second_response) + # Faking second request failure + allow(request_enum).to receive(:push) + .with(Gitaly::UserRebaseConfirmableRequest.new(apply: true)) { raise(error) } + end - expect do - repository.rebase(user, merge_request) - end.to raise_error(Gitlab::Git::Repository::GitError) + context 'when a PreReceiveError is encountered in the second step' do + let(:error) do + new_detailed_error( + GRPC::Core::StatusCodes::INTERNAL, + 'something failed', + Gitaly::UserRebaseConfirmableError.new( + access_check: Gitaly::AccessCheckError.new( + error_message: 'something went wrong' + ))) + end - expect(merge_request.reload.rebase_commit_sha).to be_nil - end + it 'does rollback' do + expect do + repository.rebase(user, merge_request) + end.to raise_error(Gitlab::Git::PreReceiveError) - def mock_gitaly(second_response) - responses = [ - double(rebase_sha: new_sha).as_null_object, - second_response - ] + expect(merge_request.reload.rebase_commit_sha).to be_nil + end + end - expect_any_instance_of( - Gitaly::OperationService::Stub - ).to receive(:user_rebase_confirmable).and_return(responses.each) + context 'when a when a GitError is encountered in the second step' do + let(:error) do + new_detailed_error( + GRPC::Core::StatusCodes::INTERNAL, + 'something failed', + Gitaly::UserSquashError.new( + rebase_conflict: Gitaly::MergeConflictError.new( + conflicting_files: ['conflicting-file'] + ))) + end + + it 'does rollback' do + expect do + repository.rebase(user, merge_request) + end.to raise_error(Gitlab::Git::Repository::GitError) + + expect(merge_request.reload.rebase_commit_sha).to be_nil + end + end end end end diff --git a/spec/views/profiles/keys/_form.html.haml_spec.rb b/spec/views/profiles/keys/_form.html.haml_spec.rb index dd8af14100a..f427804d005 100644 --- a/spec/views/profiles/keys/_form.html.haml_spec.rb +++ b/spec/views/profiles/keys/_form.html.haml_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'profiles/keys/_form.html.haml' do +RSpec.describe 'profiles/keys/_form.html.haml', feature_category: :system_access do include SshKeysHelper let_it_be(:key) { Key.new } @@ -44,7 +44,7 @@ RSpec.describe 'profiles/keys/_form.html.haml' do end it 'has the validation warning', :aggregate_failures do - expect(rendered).to have_text("Oops, are you sure? Publicly visible private SSH keys can compromise your system.") + expect(rendered).to have_text("Are you sure? Publicly visible private SSH keys can compromise your system.") expect(rendered).to have_button('Yes, add it') end diff --git a/spec/workers/process_commit_worker_spec.rb b/spec/workers/process_commit_worker_spec.rb index 02221285ad3..956e29ec7f4 100644 --- a/spec/workers/process_commit_worker_spec.rb +++ b/spec/workers/process_commit_worker_spec.rb @@ -3,180 +3,186 @@ require 'spec_helper' RSpec.describe ProcessCommitWorker, feature_category: :source_code_management do - let(:worker) { described_class.new } - let(:user) { create(:user) } + let_it_be(:user) { create(:user) } + let(:project) { create(:project, :public, :repository) } let(:issue) { create(:issue, project: project, author: user) } let(:commit) { project.commit } + let(:worker) { described_class.new } + it "is deduplicated" do expect(described_class.get_deduplicate_strategy).to eq(:until_executed) expect(described_class.get_deduplication_options).to include(feature_flag: :deduplicate_process_commit_worker) end describe '#perform' do - it 'does not process the commit when the project does not exist' do - expect(worker).not_to receive(:close_issues) + subject(:perform) { worker.perform(project_id, user_id, commit.to_hash, default) } - worker.perform(-1, user.id, commit.to_hash) - end - - it 'does not process the commit when the user does not exist' do - expect(worker).not_to receive(:close_issues) + let(:project_id) { project.id } + let(:user_id) { user.id } - worker.perform(project.id, -1, commit.to_hash) + before do + allow(Commit).to receive(:build_from_sidekiq_hash).and_return(commit) end - include_examples 'an idempotent worker' do - subject do - perform_multiple([project.id, user.id, commit.to_hash], worker: worker) - end - - it 'processes the commit message' do - expect(worker).to receive(:process_commit_message) - .exactly(IdempotentWorkerHelper::WORKER_EXEC_TIMES) - .and_call_original + context 'when pushing to the default branch' do + let(:default) { true } - subject - end + context 'when project does not exist' do + let(:project_id) { -1 } - it 'updates the issue metrics' do - expect(worker).to receive(:update_issue_metrics) - .exactly(IdempotentWorkerHelper::WORKER_EXEC_TIMES) - .and_call_original + it 'does not close related issues' do + expect { perform }.to change { Issues::CloseWorker.jobs.size }.by(0) - subject + perform + end end - end - end - describe '#process_commit_message' do - context 'when pushing to the default branch' do - before do - allow(commit).to receive(:safe_message).and_return("Closes #{issue.to_reference}") - end + context 'when user does not exist' do + let(:user_id) { -1 } - it 'closes issues that should be closed per the commit message' do - expect(worker).to receive(:close_issues).with(project, user, user, commit, [issue]) + it 'does not close related issues' do + expect { perform }.not_to change { Issues::CloseWorker.jobs.size } - worker.process_commit_message(project, commit, user, user, true) + perform + end end - it 'creates cross references' do - expect(commit).to receive(:create_cross_references!).with(user, [issue]) - - worker.process_commit_message(project, commit, user, user, true) - end - end + include_examples 'an idempotent worker' do + before do + allow(commit).to receive(:safe_message).and_return("Closes #{issue.to_reference}") + issue.metrics.update!(first_mentioned_in_commit_at: nil) + end - context 'when pushing to a non-default branch' do - it 'does not close any issues' do - allow(commit).to receive(:safe_message).and_return("Closes #{issue.to_reference}") + subject do + perform_multiple([project.id, user.id, commit.to_hash], worker: worker) + end - expect(worker).not_to receive(:close_issues) + it 'closes related issues' do + expect { perform }.to change { Issues::CloseWorker.jobs.size }.by(1) - worker.process_commit_message(project, commit, user, user, false) + subject + end end - it 'does not create cross references' do - expect(commit).to receive(:create_cross_references!).with(user, []) + context 'when commit is not a merge request merge commit' do + context 'when commit has issue reference' do + before do + allow(commit).to receive(:safe_message).and_return("Closes #{issue.to_reference}") + end + + it 'closes issues that should be closed per the commit message' do + expect { perform }.to change { Issues::CloseWorker.jobs.size }.by(1) + end + + it 'creates cross references' do + expect(commit).to receive(:create_cross_references!).with(user, [issue]) + + perform + end + + describe 'issue metrics', :clean_gitlab_redis_cache do + context 'when issue has no first_mentioned_in_commit_at set' do + before do + issue.metrics.update!(first_mentioned_in_commit_at: nil) + end + + it 'updates issue metrics' do + expect { perform }.to change { issue.metrics.reload.first_mentioned_in_commit_at } + .to(commit.committed_date) + end + end + + context 'when issue has first_mentioned_in_commit_at earlier than given committed_date' do + before do + issue.metrics.update!(first_mentioned_in_commit_at: commit.committed_date - 1.day) + end + + it "doesn't update issue metrics" do + expect { perform }.not_to change { issue.metrics.reload.first_mentioned_in_commit_at } + end + end + + context 'when issue has first_mentioned_in_commit_at later than given committed_date' do + before do + issue.metrics.update!(first_mentioned_in_commit_at: commit.committed_date + 1.day) + end + + it 'updates issue metrics' do + expect { perform }.to change { issue.metrics.reload.first_mentioned_in_commit_at } + .to(commit.committed_date) + end + end + end + end - worker.process_commit_message(project, commit, user, user, false) - end - end + context 'when commit has no issue references' do + before do + allow(commit).to receive(:safe_message).and_return("Lorem Ipsum") + end - context 'when commit is a merge request merge commit to the default branch' do - let(:merge_request) do - create( - :merge_request, - description: "Closes #{issue.to_reference}", - source_branch: 'feature-merged', - target_branch: 'master', - source_project: project - ) + describe 'issue metrics' do + it "doesn't execute any queries with false conditions" do + expect { perform }.not_to make_queries_matching(/WHERE (?:1=0|0=1)/) + end + end + end end - let(:commit) do - project.repository.create_branch('feature-merged', 'feature') - project.repository.after_create_branch + context 'when commit is a merge request merge commit' do + let(:merge_request) do + create( + :merge_request, + description: "Closes #{issue.to_reference}", + source_branch: 'feature-merged', + target_branch: 'master', + source_project: project + ) + end - MergeRequests::MergeService - .new(project: project, current_user: merge_request.author, params: { sha: merge_request.diff_head_sha }) - .execute(merge_request) + let(:commit) do + project.repository.create_branch('feature-merged', 'feature') + project.repository.after_create_branch - merge_request.reload.merge_commit - end + MergeRequests::MergeService + .new(project: project, current_user: merge_request.author, params: { sha: merge_request.diff_head_sha }) + .execute(merge_request) - it 'does not close any issues from the commit message' do - expect(worker).not_to receive(:close_issues) + merge_request.reload.merge_commit + end - worker.process_commit_message(project, commit, user, user, true) - end + it 'does not close any issues from the commit message' do + expect { perform }.not_to change { Issues::CloseWorker.jobs.size } - it 'still creates cross references' do - expect(commit).to receive(:create_cross_references!).with(user, []) + perform + end - worker.process_commit_message(project, commit, user, user, true) - end - end - end + it 'still creates cross references' do + expect(commit).to receive(:create_cross_references!).with(commit.author, []) - describe '#close_issues' do - it 'creates Issue::CloseWorker jobs' do - expect do - worker.close_issues(project, user, user, commit, [issue]) - end.to change { Issues::CloseWorker.jobs.size }.by(1) + perform + end + end end - end - describe '#update_issue_metrics', :clean_gitlab_redis_cache do - context 'when commit has issue reference' do - subject(:update_metrics_and_reload) do - -> { - worker.update_issue_metrics(commit, user) - issue.metrics.reload - } - end + context 'when pushing to a non-default branch' do + let(:default) { false } before do allow(commit).to receive(:safe_message).and_return("Closes #{issue.to_reference}") end - context 'when issue has no first_mentioned_in_commit_at set' do - it 'updates issue metrics' do - expect { update_metrics_and_reload.call } - .to change { issue.metrics.first_mentioned_in_commit_at }.to(commit.committed_date) - end - end - - context 'when issue has first_mentioned_in_commit_at earlier than given committed_date' do - before do - issue.metrics.update!(first_mentioned_in_commit_at: commit.committed_date - 1.day) - end - - it "doesn't update issue metrics" do - expect { update_metrics_and_reload.call }.not_to change { issue.metrics.first_mentioned_in_commit_at } - end - end - - context 'when issue has first_mentioned_in_commit_at later than given committed_date' do - before do - issue.metrics.update!(first_mentioned_in_commit_at: commit.committed_date + 1.day) - end + it 'does not close any issues from the commit message' do + expect { perform }.not_to change { Issues::CloseWorker.jobs.size } - it "doesn't update issue metrics" do - expect { update_metrics_and_reload.call } - .to change { issue.metrics.first_mentioned_in_commit_at }.to(commit.committed_date) - end + perform end - end - context 'when commit has no issue references' do - it "doesn't execute any queries with false conditions" do - allow(commit).to receive(:safe_message).and_return("Lorem Ipsum") + it 'still creates cross references' do + expect(commit).to receive(:create_cross_references!).with(user, []) - expect { worker.update_issue_metrics(commit, user) } - .not_to make_queries_matching(/WHERE (?:1=0|0=1)/) + perform end end end |