From c6fa8c3fbaab2d7bcde5cecead129b085d5a2dae Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Mon, 12 Feb 2018 15:38:10 +0000 Subject: Create an empty wiki when there is no wiki in the gitlab export bundle --- .../lib/gitlab/import_export/wiki_restorer_spec.rb | 45 ++++++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 spec/lib/gitlab/import_export/wiki_restorer_spec.rb (limited to 'spec/lib') diff --git a/spec/lib/gitlab/import_export/wiki_restorer_spec.rb b/spec/lib/gitlab/import_export/wiki_restorer_spec.rb new file mode 100644 index 00000000000..81b654e9c5f --- /dev/null +++ b/spec/lib/gitlab/import_export/wiki_restorer_spec.rb @@ -0,0 +1,45 @@ +require 'spec_helper' + +describe Gitlab::ImportExport::WikiRestorer do + describe 'restore a wiki Git repo' do + let!(:project_with_wiki) { create(:project, :wiki_repo) } + let!(:project_without_wiki) { create(:project) } + let!(:project) { create(:project) } + let(:export_path) { "#{Dir.tmpdir}/project_tree_saver_spec" } + let(:shared) { Gitlab::ImportExport::Shared.new(relative_path: project.full_path) } + let(:bundler) { Gitlab::ImportExport::WikiRepoSaver.new(project: project_with_wiki, shared: shared) } + let(:bundle_path) { File.join(shared.export_path, Gitlab::ImportExport.project_bundle_filename) } + let(:restorer) do + described_class.new(path_to_bundle: bundle_path, + shared: shared, + project: project.wiki, + wiki_enabled: true) + end + + before do + allow(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path) + + bundler.save + end + + after do + FileUtils.rm_rf(export_path) + Gitlab::Shell.new.remove_repository(project_with_wiki.wiki.repository_storage_path, project_with_wiki.wiki.disk_path) + Gitlab::Shell.new.remove_repository(project.wiki.repository_storage_path, project.wiki.disk_path) + end + + it 'restores the wiki repo successfully' do + expect(restorer.restore).to be true + end + + describe "no wiki in the bundle" do + let(:bundler) { Gitlab::ImportExport::WikiRepoSaver.new(project: project_without_wiki, shared: shared) } + + it 'creates an empty wiki' do + expect(restorer.restore).to be true + + expect(project.wiki_repository_exists?).to be true + end + end + end +end -- cgit v1.2.1 From 4af914c25c44e88556b93ac5b08d63ee22e27c5c Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Tue, 6 Feb 2018 23:44:45 -0800 Subject: Disable caching of tables for migration spec that drops a temporary table This is to fix job failures, such as https://gitlab.com/gitlab-org/gitlab-ee/-/jobs/51409392. --- spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb b/spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb index 8590522f3ef..5caf7e6cc4c 100644 --- a/spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb +++ b/spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::BackgroundMigration::PopulateUntrackedUploads, :sidekiq do +describe Gitlab::BackgroundMigration::PopulateUntrackedUploads, :sidekiq, :delete_no_cache do include TrackUntrackedUploadsHelpers subject { described_class.new } -- cgit v1.2.1 From d1eac99b92635429eb0ea4e11c6f61e0494f9eb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Wed, 7 Feb 2018 10:31:56 +0100 Subject: Use the :migration metadata in migration specs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- .../gitlab/background_migration/populate_untracked_uploads_spec.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb b/spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb index 5caf7e6cc4c..fb3f29ff4c9 100644 --- a/spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb +++ b/spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb @@ -1,6 +1,11 @@ require 'spec_helper' -describe Gitlab::BackgroundMigration::PopulateUntrackedUploads, :sidekiq, :delete_no_cache do +# This migration is using UploadService, which sets uploads.secret that is only +# added to the DB schema in 20180129193323. Since the test isn't isolated, we +# just use the latest schema when testing this migration. +# Ideally, the test should not use factories nor UploadService, and rely on the +# `table` helper instead. +describe Gitlab::BackgroundMigration::PopulateUntrackedUploads, :sidekiq, :migration, schema: 20180129193323 do include TrackUntrackedUploadsHelpers subject { described_class.new } -- cgit v1.2.1 From 34c2a59c5794179d04c96a2df53a6ed3bd72d6b5 Mon Sep 17 00:00:00 2001 From: Alessio Caiazza Date: Mon, 12 Feb 2018 17:19:25 +0100 Subject: Honour workhorse provided file name In the attempt to unify file uploading at workhorse level gitlab-org/gitlab-workhorse!230 we moved to a prefix-based tempfile creation in order to avoid upload collisions. Artifacts and LFS uploads already set original_filename to workhorse provided filename This commit add the same feature to `Gitlab::Middleware::Multipart` --- spec/lib/gitlab/middleware/multipart_spec.rb | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/middleware/multipart_spec.rb b/spec/lib/gitlab/middleware/multipart_spec.rb index 8d925460f01..a2ba91dae80 100644 --- a/spec/lib/gitlab/middleware/multipart_spec.rb +++ b/spec/lib/gitlab/middleware/multipart_spec.rb @@ -5,15 +5,17 @@ require 'tempfile' describe Gitlab::Middleware::Multipart do let(:app) { double(:app) } let(:middleware) { described_class.new(app) } + let(:original_filename) { 'filename' } it 'opens top-level files' do Tempfile.open('top-level') do |tempfile| - env = post_env({ 'file' => tempfile.path }, { 'file.name' => 'filename' }, Gitlab::Workhorse.secret, 'gitlab-workhorse') + env = post_env({ 'file' => tempfile.path }, { 'file.name' => original_filename }, Gitlab::Workhorse.secret, 'gitlab-workhorse') expect(app).to receive(:call) do |env| file = Rack::Request.new(env).params['file'] expect(file).to be_a(::UploadedFile) expect(file.path).to eq(tempfile.path) + expect(file.original_filename).to eq(original_filename) end middleware.call(env) @@ -34,13 +36,14 @@ describe Gitlab::Middleware::Multipart do it 'opens files one level deep' do Tempfile.open('one-level') do |tempfile| - in_params = { 'user' => { 'avatar' => { '.name' => 'filename' } } } + in_params = { 'user' => { 'avatar' => { '.name' => original_filename } } } env = post_env({ 'user[avatar]' => tempfile.path }, in_params, Gitlab::Workhorse.secret, 'gitlab-workhorse') expect(app).to receive(:call) do |env| file = Rack::Request.new(env).params['user']['avatar'] expect(file).to be_a(::UploadedFile) expect(file.path).to eq(tempfile.path) + expect(file.original_filename).to eq(original_filename) end middleware.call(env) @@ -49,13 +52,14 @@ describe Gitlab::Middleware::Multipart do it 'opens files two levels deep' do Tempfile.open('two-levels') do |tempfile| - in_params = { 'project' => { 'milestone' => { 'themesong' => { '.name' => 'filename' } } } } + in_params = { 'project' => { 'milestone' => { 'themesong' => { '.name' => original_filename } } } } env = post_env({ 'project[milestone][themesong]' => tempfile.path }, in_params, Gitlab::Workhorse.secret, 'gitlab-workhorse') expect(app).to receive(:call) do |env| file = Rack::Request.new(env).params['project']['milestone']['themesong'] expect(file).to be_a(::UploadedFile) expect(file.path).to eq(tempfile.path) + expect(file.original_filename).to eq(original_filename) end middleware.call(env) -- cgit v1.2.1 From 740499bab5524d490218fd36033402cf67ec49f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= Date: Mon, 12 Feb 2018 12:39:47 -0500 Subject: Revert "Merge branch 'rd-40552-gitlab-should-check-if-keys-are-valid-before-saving' into 'master'" This reverts commit a58f8c32c62bcf5824d1fe1d0de53e9bda974d65, reversing changes made to cd5d75c362cdf06efb8174eddfbd0f4b65687dec. --- spec/lib/gitlab/ssh_public_key_spec.rb | 35 ---------------------------------- 1 file changed, 35 deletions(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/ssh_public_key_spec.rb b/spec/lib/gitlab/ssh_public_key_spec.rb index c15e29774b6..93d538141ce 100644 --- a/spec/lib/gitlab/ssh_public_key_spec.rb +++ b/spec/lib/gitlab/ssh_public_key_spec.rb @@ -37,41 +37,6 @@ describe Gitlab::SSHPublicKey, lib: true do end end - describe '.sanitize(key_content)' do - let(:content) { build(:key).key } - - context 'when key has blank space characters' do - it 'removes the extra blank space characters' do - unsanitized = content.insert(100, "\n") - .insert(40, "\r\n") - .insert(30, ' ') - - sanitized = described_class.sanitize(unsanitized) - _, body = sanitized.split - - expect(sanitized).not_to eq(unsanitized) - expect(body).not_to match(/\s/) - end - end - - context "when key doesn't have blank space characters" do - it "doesn't modify the content" do - sanitized = described_class.sanitize(content) - - expect(sanitized).to eq(content) - end - end - - context "when key is invalid" do - it 'returns the original content' do - unsanitized = "ssh-foo any content==" - sanitized = described_class.sanitize(unsanitized) - - expect(sanitized).to eq(unsanitized) - end - end - end - describe '#valid?' do subject { public_key } -- cgit v1.2.1 From f917fc5d76d39fb17fda2678d5270407595f0128 Mon Sep 17 00:00:00 2001 From: Michael Kozono Date: Thu, 8 Feb 2018 10:08:55 -0800 Subject: Fix last batch size equals max batch size error --- .../prepare_untracked_uploads_spec.rb | 24 ++++++++++++++++++++++ 1 file changed, 24 insertions(+) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb b/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb index 48204114ae8..8189e74646a 100644 --- a/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb +++ b/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb @@ -133,6 +133,18 @@ describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :sidekiq do context 'when there are files in /uploads/tmp' do it_behaves_like 'does not add files in /uploads/tmp' end + + context 'when the last batch size exactly matches the max batch size' do + it 'does not raise error' do + stub_const("#{described_class}::FIND_BATCH_SIZE", 5) + + expect do + described_class.new.perform + end.not_to raise_error + + expect(untracked_files_for_uploads.count).to eq(5) + end + end end end @@ -205,6 +217,18 @@ describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :sidekiq do context 'when there are files in /uploads/tmp' do it_behaves_like 'does not add files in /uploads/tmp' end + + context 'when the last batch size exactly matches the max batch size' do + it 'does not raise error' do + stub_const("#{described_class}::FIND_BATCH_SIZE", 5) + + expect do + described_class.new.perform + end.not_to raise_error + + expect(untracked_files_for_uploads.count).to eq(5) + end + end end end -- cgit v1.2.1 From 67d310a1a640088d497843ec8dce3f16eb285d2d Mon Sep 17 00:00:00 2001 From: Michael Kozono Date: Thu, 8 Feb 2018 10:17:00 -0800 Subject: Fix orphan temp table untracked_files_for_uploads --- .../prepare_untracked_uploads_spec.rb | 42 +++++++++------------- 1 file changed, 17 insertions(+), 25 deletions(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb b/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb index 8189e74646a..6a19e8fdcb8 100644 --- a/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb +++ b/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb @@ -8,8 +8,6 @@ describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :sidekiq do before do DatabaseCleaner.clean - - drop_temp_table_if_exists end after do @@ -44,27 +42,6 @@ describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :sidekiq do end end - it 'ensures the untracked_files_for_uploads table exists' do - expect do - described_class.new.perform - end.to change { ActiveRecord::Base.connection.table_exists?(:untracked_files_for_uploads) }.from(false).to(true) - end - - it 'has a path field long enough for really long paths' do - described_class.new.perform - - component = 'a' * 255 - - long_path = [ - 'uploads', - component, # project.full_path - component # filename - ].flatten.join('/') - - record = untracked_files_for_uploads.create!(path: long_path) - expect(record.reload.path.size).to eq(519) - end - context "test bulk insert with ON CONFLICT DO NOTHING or IGNORE" do around do |example| # If this is CI, we use Postgres 9.2 so this whole context should be @@ -90,6 +67,21 @@ describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :sidekiq do UploadService.new(project2, uploaded_file, FileUploader).execute end + it 'has a path field long enough for really long paths' do + described_class.new.perform + + component = 'a' * 255 + + long_path = [ + 'uploads', + component, # project.full_path + component # filename + ].flatten.join('/') + + record = untracked_files_for_uploads.create!(path: long_path) + expect(record.reload.path.size).to eq(519) + end + it 'adds unhashed files to the untracked_files_for_uploads table' do described_class.new.perform @@ -235,10 +227,10 @@ describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :sidekiq do # Very new or lightly-used installations that are running this migration # may not have an upload directory because they have no uploads. context 'when no files were ever uploaded' do - it 'does not add to the untracked_files_for_uploads table (and does not raise error)' do + it 'deletes the `untracked_files_for_uploads` table (and does not raise error)' do described_class.new.perform - expect(untracked_files_for_uploads.count).to eq(0) + expect(untracked_files_for_uploads.connection.table_exists?(:untracked_files_for_uploads)).to be_falsey end end end -- cgit v1.2.1 From 4e6a8eaab6004d789cab08e625478fe9d5ef2bb7 Mon Sep 17 00:00:00 2001 From: Michael Kozono Date: Fri, 9 Feb 2018 17:01:36 -0800 Subject: Dry up spec --- .../prepare_untracked_uploads_spec.rb | 136 +++++---------------- 1 file changed, 29 insertions(+), 107 deletions(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb b/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb index 6a19e8fdcb8..43f3548eadc 100644 --- a/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb +++ b/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb @@ -21,36 +21,7 @@ describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :sidekiq do end end - # E.g. The installation is in use at the time of migration, and someone has - # just uploaded a file - shared_examples 'does not add files in /uploads/tmp' do - let(:tmp_file) { Rails.root.join(described_class::ABSOLUTE_UPLOAD_DIR, 'tmp', 'some_file.jpg') } - - before do - FileUtils.mkdir(File.dirname(tmp_file)) - FileUtils.touch(tmp_file) - end - - after do - FileUtils.rm(tmp_file) - end - - it 'does not add files from /uploads/tmp' do - described_class.new.perform - - expect(untracked_files_for_uploads.count).to eq(5) - end - end - - context "test bulk insert with ON CONFLICT DO NOTHING or IGNORE" do - around do |example| - # If this is CI, we use Postgres 9.2 so this whole context should be - # skipped since we're unable to use ON CONFLICT DO NOTHING or IGNORE. - if described_class.new.send(:can_bulk_insert_and_ignore_duplicates?) - example.run - end - end - + shared_examples 'prepares the untracked_files_for_uploads table' do context 'when files were uploaded before and after hashed storage was enabled' do let!(:appearance) { create_or_update_appearance(logo: uploaded_file, header_logo: uploaded_file) } let!(:user) { create(:user, :with_avatar) } @@ -122,94 +93,27 @@ describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :sidekiq do end end + # E.g. The installation is in use at the time of migration, and someone has + # just uploaded a file context 'when there are files in /uploads/tmp' do - it_behaves_like 'does not add files in /uploads/tmp' - end - - context 'when the last batch size exactly matches the max batch size' do - it 'does not raise error' do - stub_const("#{described_class}::FIND_BATCH_SIZE", 5) - - expect do - described_class.new.perform - end.not_to raise_error + let(:tmp_file) { Rails.root.join(described_class::ABSOLUTE_UPLOAD_DIR, 'tmp', 'some_file.jpg') } - expect(untracked_files_for_uploads.count).to eq(5) + before do + FileUtils.mkdir(File.dirname(tmp_file)) + FileUtils.touch(tmp_file) end - end - end - end - - context 'test bulk insert without ON CONFLICT DO NOTHING or IGNORE' do - before do - # If this is CI, we use Postgres 9.2 so this stub has no effect. - # - # If this is being run on Postgres 9.5+ or MySQL, then this stub allows us - # to test the bulk insert functionality without ON CONFLICT DO NOTHING or - # IGNORE. - allow_any_instance_of(described_class).to receive(:postgresql_pre_9_5?).and_return(true) - end - - context 'when files were uploaded before and after hashed storage was enabled' do - let!(:appearance) { create_or_update_appearance(logo: uploaded_file, header_logo: uploaded_file) } - let!(:user) { create(:user, :with_avatar) } - let!(:project1) { create(:project, :with_avatar, :legacy_storage) } - let(:project2) { create(:project) } # instantiate after enabling hashed_storage - - before do - # Markdown upload before enabling hashed_storage - UploadService.new(project1, uploaded_file, FileUploader).execute - - stub_application_setting(hashed_storage_enabled: true) - # Markdown upload after enabling hashed_storage - UploadService.new(project2, uploaded_file, FileUploader).execute - end - - it 'adds unhashed files to the untracked_files_for_uploads table' do - described_class.new.perform - - expect(untracked_files_for_uploads.count).to eq(5) - end - - it 'adds files with paths relative to CarrierWave.root' do - described_class.new.perform - untracked_files_for_uploads.all.each do |file| - expect(file.path.start_with?('uploads/')).to be_truthy + after do + FileUtils.rm(tmp_file) end - end - it 'does not add hashed files to the untracked_files_for_uploads table' do - described_class.new.perform - - hashed_file_path = project2.uploads.where(uploader: 'FileUploader').first.path - expect(untracked_files_for_uploads.where("path like '%#{hashed_file_path}%'").exists?).to be_falsey - end - - it 'correctly schedules the follow-up background migration jobs' do - described_class.new.perform - - expect(described_class::FOLLOW_UP_MIGRATION).to be_scheduled_migration(1, 5) - expect(BackgroundMigrationWorker.jobs.size).to eq(1) - end - - # E.g. from a previous failed run of this background migration - context 'when there is existing data in untracked_files_for_uploads' do - before do + it 'does not add files from /uploads/tmp' do described_class.new.perform - end - it 'does not error or produce duplicates of existing data' do - expect do - described_class.new.perform - end.not_to change { untracked_files_for_uploads.count }.from(5) + expect(untracked_files_for_uploads.count).to eq(5) end end - context 'when there are files in /uploads/tmp' do - it_behaves_like 'does not add files in /uploads/tmp' - end - context 'when the last batch size exactly matches the max batch size' do it 'does not raise error' do stub_const("#{described_class}::FIND_BATCH_SIZE", 5) @@ -224,6 +128,24 @@ describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :sidekiq do end end + # If running on Postgres 9.2 (like on CI), this whole context is skipped + # since we're unable to use ON CONFLICT DO NOTHING or IGNORE. + context "test bulk insert with ON CONFLICT DO NOTHING or IGNORE", if: described_class.new.send(:can_bulk_insert_and_ignore_duplicates?) do + it_behaves_like 'prepares the untracked_files_for_uploads table' + end + + # If running on Postgres 9.2 (like on CI), the stubbed method has no effect. + # + # If running on Postgres 9.5+ or MySQL, then this context effectively tests + # the bulk insert functionality without ON CONFLICT DO NOTHING or IGNORE. + context 'test bulk insert without ON CONFLICT DO NOTHING or IGNORE' do + before do + allow_any_instance_of(described_class).to receive(:postgresql_pre_9_5?).and_return(true) + end + + it_behaves_like 'prepares the untracked_files_for_uploads table' + end + # Very new or lightly-used installations that are running this migration # may not have an upload directory because they have no uploads. context 'when no files were ever uploaded' do -- cgit v1.2.1 From 04b642889a0459ff38cca043c5898ba4647d2fc6 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Mon, 12 Feb 2018 11:15:26 -0800 Subject: Disable query limiting warnings for now on GitLab.com --- spec/lib/gitlab/query_limiting_spec.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/query_limiting_spec.rb b/spec/lib/gitlab/query_limiting_spec.rb index 2eddab0b8c3..42877b1e2dd 100644 --- a/spec/lib/gitlab/query_limiting_spec.rb +++ b/spec/lib/gitlab/query_limiting_spec.rb @@ -12,14 +12,16 @@ describe Gitlab::QueryLimiting do expect(described_class.enable?).to eq(true) end - it 'returns true on GitLab.com' do + it 'returns false on GitLab.com' do + expect(Rails.env).to receive(:development?).and_return(false) + expect(Rails.env).to receive(:test?).and_return(false) allow(Gitlab).to receive(:com?).and_return(true) - expect(described_class.enable?).to eq(true) + expect(described_class.enable?).to eq(false) end - it 'returns true in a non GitLab.com' do - expect(Gitlab).to receive(:com?).and_return(false) + it 'returns false in a non GitLab.com' do + allow(Gitlab).to receive(:com?).and_return(false) expect(Rails.env).to receive(:development?).and_return(false) expect(Rails.env).to receive(:test?).and_return(false) -- cgit v1.2.1 From 06c111ca8f4373069ad0d3bbcf9e1ce2f5fd6d97 Mon Sep 17 00:00:00 2001 From: Dylan Griffith Date: Sun, 19 Nov 2017 14:01:01 +1100 Subject: Ensure users can't create environments with leading or trailing slashes (Fixes #39885) --- spec/lib/gitlab/regex_spec.rb | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/regex_spec.rb b/spec/lib/gitlab/regex_spec.rb index 8b54d72d6f7..a1079e54975 100644 --- a/spec/lib/gitlab/regex_spec.rb +++ b/spec/lib/gitlab/regex_spec.rb @@ -18,6 +18,7 @@ describe Gitlab::Regex do subject { described_class.environment_name_regex } it { is_expected.to match('foo') } + it { is_expected.to match('a') } it { is_expected.to match('foo-1') } it { is_expected.to match('FOO') } it { is_expected.to match('foo/1') } @@ -25,6 +26,10 @@ describe Gitlab::Regex do it { is_expected.not_to match('9&foo') } it { is_expected.not_to match('foo-^') } it { is_expected.not_to match('!!()()') } + it { is_expected.not_to match('/foo') } + it { is_expected.not_to match('foo/') } + it { is_expected.not_to match('/foo/') } + it { is_expected.not_to match('/') } end describe '.environment_slug_regex' do -- cgit v1.2.1 From b45c7dd5fecd33d1fbc9053cb353d7ed0c6d57dd Mon Sep 17 00:00:00 2001 From: Drew Blessing Date: Thu, 25 Jan 2018 16:18:15 -0600 Subject: Revert problematic LDAP person validation that threw exceptions Constructors shouldn't throw exceptions. We also learned that different LDAP servers behave a bit unexpectedly sometimes - returning attributes we didn't ask for, or returned attributes with language subtypes. --- spec/lib/gitlab/ldap/person_spec.rb | 9 --------- 1 file changed, 9 deletions(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/ldap/person_spec.rb b/spec/lib/gitlab/ldap/person_spec.rb index ff29d9aa5be..381bf2bc0e0 100644 --- a/spec/lib/gitlab/ldap/person_spec.rb +++ b/spec/lib/gitlab/ldap/person_spec.rb @@ -66,15 +66,6 @@ describe Gitlab::LDAP::Person do end end - describe '.validate_entry' do - it 'raises InvalidEntryError' do - entry['foo'] = 'bar' - - expect { described_class.new(entry, 'ldapmain') } - .to raise_error(Gitlab::LDAP::Person::InvalidEntryError) - end - end - describe '#name' do it 'uses the configured name attribute and handles values as an array' do name = 'John Doe' -- cgit v1.2.1 From e3bd674e81d565ef3bd399a70b3039316b518693 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Tue, 13 Feb 2018 16:40:23 +0100 Subject: Remove Sentry reporting for query limiting Using Sentry, while useful, poses two problems you have to choose from: 1. All errors are reported separately, making it easy to create issues but also making it next to impossible to see other errors (due to the sheer volume of threshold errors). 2. Errors can be grouped or merged together, reducing the noise. This however also means it's (as far as I can tell) much harder to automatically create GitLab issues from Sentry for the offending controllers. Since both solutions are terrible I decided to go with a third option: not using Sentry for this at all. Instead we'll investigate using Prometheus alerts and Grafana dashboards for this, which has the added benefit of being able to more accurately measure the behaviour over time. Note that throwing errors in test environments is still enabled, and whitelisting is still necessary to prevent that from happening (and that in turn still requires that developers create issues). --- spec/lib/gitlab/query_limiting/transaction_spec.rb | 12 ------------ 1 file changed, 12 deletions(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/query_limiting/transaction_spec.rb b/spec/lib/gitlab/query_limiting/transaction_spec.rb index b4231fcd0fa..b72b8574174 100644 --- a/spec/lib/gitlab/query_limiting/transaction_spec.rb +++ b/spec/lib/gitlab/query_limiting/transaction_spec.rb @@ -59,18 +59,6 @@ describe Gitlab::QueryLimiting::Transaction do expect { transaction.act_upon_results } .to raise_error(described_class::ThresholdExceededError) end - - it 'reports the error in Sentry if raising an error is disabled' do - expect(transaction) - .to receive(:raise_error?) - .and_return(false) - - expect(Raven) - .to receive(:capture_exception) - .with(an_instance_of(described_class::ThresholdExceededError)) - - transaction.act_upon_results - end end end -- cgit v1.2.1 From 080dba4a7e036e4bcb9cae69c5de6bfa33ff8b2e Mon Sep 17 00:00:00 2001 From: Michael Kozono Date: Tue, 13 Feb 2018 12:31:30 -0800 Subject: Avoid dropping tables in test And use :migration tag to use deletion strategy, and to avoid caching tables, and to lock into a particular schema. Attempting to fix intermittent spec errors `PG::UndefinedTable: ERROR: relation "public.untracked_files_for_uploads" does not exist`. --- .../populate_untracked_uploads_spec.rb | 14 ++------------ .../prepare_untracked_uploads_spec.rb | 19 +++++++------------ 2 files changed, 9 insertions(+), 24 deletions(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb b/spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb index fb3f29ff4c9..c8fa252439a 100644 --- a/spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb +++ b/spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb @@ -14,16 +14,10 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads, :sidekiq, :migra let!(:uploads) { described_class::Upload } before do - DatabaseCleaner.clean - drop_temp_table_if_exists ensure_temporary_tracking_table_exists uploads.delete_all end - after(:all) do - drop_temp_table_if_exists - end - context 'with untracked files and tracked files in untracked_files_for_uploads' do let!(:appearance) { create_or_update_appearance(logo: uploaded_file, header_logo: uploaded_file) } let!(:user1) { create(:user, :with_avatar) } @@ -122,9 +116,9 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads, :sidekiq, :migra end it 'drops the temporary tracking table after processing the batch, if there are no untracked rows left' do - subject.perform(1, untracked_files_for_uploads.last.id) + expect(subject).to receive(:drop_temp_table_if_finished) - expect(ActiveRecord::Base.connection.table_exists?(:untracked_files_for_uploads)).to be_falsey + subject.perform(1, untracked_files_for_uploads.last.id) end it 'does not block a whole batch because of one bad path' do @@ -260,10 +254,6 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do ensure_temporary_tracking_table_exists end - after(:all) do - drop_temp_table_if_exists - end - describe '#upload_path' do def assert_upload_path(file_path, expected_upload_path) untracked_file = create_untracked_file(file_path) diff --git a/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb b/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb index 43f3548eadc..ca77e64ae40 100644 --- a/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb +++ b/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb @@ -1,19 +1,11 @@ require 'spec_helper' -describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :sidekiq do +describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :sidekiq, :migration, schema: 20180129193323 do include TrackUntrackedUploadsHelpers include MigrationsHelpers let!(:untracked_files_for_uploads) { described_class::UntrackedFile } - before do - DatabaseCleaner.clean - end - - after do - drop_temp_table_if_exists - end - around do |example| # Especially important so the follow-up migration does not get run Sidekiq::Testing.fake! do @@ -76,7 +68,8 @@ describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :sidekiq do it 'correctly schedules the follow-up background migration jobs' do described_class.new.perform - expect(described_class::FOLLOW_UP_MIGRATION).to be_scheduled_migration(1, 5) + ids = described_class::UntrackedFile.all.order(:id).pluck(:id) + expect(described_class::FOLLOW_UP_MIGRATION).to be_scheduled_migration(ids.first, ids.last) expect(BackgroundMigrationWorker.jobs.size).to eq(1) end @@ -150,9 +143,11 @@ describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :sidekiq do # may not have an upload directory because they have no uploads. context 'when no files were ever uploaded' do it 'deletes the `untracked_files_for_uploads` table (and does not raise error)' do - described_class.new.perform + background_migration = described_class.new + + expect(background_migration).to receive(:drop_temp_table) - expect(untracked_files_for_uploads.connection.table_exists?(:untracked_files_for_uploads)).to be_falsey + background_migration.perform end end end -- cgit v1.2.1