diff options
Diffstat (limited to 'spec/lib')
45 files changed, 998 insertions, 246 deletions
diff --git a/spec/lib/banzai/filter/markdown_filter_spec.rb b/spec/lib/banzai/filter/markdown_filter_spec.rb index 8d01a651651..c5e84a0c1e7 100644 --- a/spec/lib/banzai/filter/markdown_filter_spec.rb +++ b/spec/lib/banzai/filter/markdown_filter_spec.rb @@ -46,6 +46,12 @@ RSpec.describe Banzai::Filter::MarkdownFilter do expect(result).to start_with('<pre><code lang="æ—¥">') end + + it 'works with additional language parameters' do + result = filter("```ruby:red gem\nsome code\n```", no_sourcepos: true) + + expect(result).to start_with('<pre><code lang="ruby:red gem">') + end end end diff --git a/spec/lib/bulk_imports/common/extractors/graphql_extractor_spec.rb b/spec/lib/bulk_imports/common/extractors/graphql_extractor_spec.rb index cde8e2d5c18..a7a19fb73fc 100644 --- a/spec/lib/bulk_imports/common/extractors/graphql_extractor_spec.rb +++ b/spec/lib/bulk_imports/common/extractors/graphql_extractor_spec.rb @@ -41,12 +41,11 @@ RSpec.describe BulkImports::Common::Extractors::GraphqlExtractor do end context 'when variables are present' do - let(:query) { { query: double(to_s: 'test', variables: { full_path: :source_full_path }) } } + let(:variables) { { foo: :bar } } + let(:query) { { query: double(to_s: 'test', variables: variables) } } it 'builds graphql query variables for import entity' do - expected_variables = { full_path: import_entity.source_full_path } - - expect(graphql_client).to receive(:execute).with(anything, expected_variables) + expect(graphql_client).to receive(:execute).with(anything, variables) subject.extract(context).first end diff --git a/spec/lib/bulk_imports/common/transformers/graphql_cleaner_transformer_spec.rb b/spec/lib/bulk_imports/common/transformers/graphql_cleaner_transformer_spec.rb deleted file mode 100644 index 8f39b6e7c93..00000000000 --- a/spec/lib/bulk_imports/common/transformers/graphql_cleaner_transformer_spec.rb +++ /dev/null @@ -1,88 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe BulkImports::Common::Transformers::GraphqlCleanerTransformer do - describe '#transform' do - let_it_be(:expected_output) do - { - 'name' => 'test', - 'fullName' => 'test', - 'description' => 'test', - 'labels' => [ - { 'title' => 'label1' }, - { 'title' => 'label2' }, - { 'title' => 'label3' } - ] - } - end - - it 'deep cleans hash from GraphQL keys' do - data = { - 'data' => { - 'group' => { - 'name' => 'test', - 'fullName' => 'test', - 'description' => 'test', - 'labels' => { - 'edges' => [ - { 'node' => { 'title' => 'label1' } }, - { 'node' => { 'title' => 'label2' } }, - { 'node' => { 'title' => 'label3' } } - ] - } - } - } - } - - transformed_data = described_class.new.transform(nil, data) - - expect(transformed_data).to eq(expected_output) - end - - context 'when data does not have data/group nesting' do - it 'deep cleans hash from GraphQL keys' do - data = { - 'name' => 'test', - 'fullName' => 'test', - 'description' => 'test', - 'labels' => { - 'edges' => [ - { 'node' => { 'title' => 'label1' } }, - { 'node' => { 'title' => 'label2' } }, - { 'node' => { 'title' => 'label3' } } - ] - } - } - - transformed_data = described_class.new.transform(nil, data) - - expect(transformed_data).to eq(expected_output) - end - end - - context 'when data is not a hash' do - it 'does not perform transformation' do - data = 'test' - - transformed_data = described_class.new.transform(nil, data) - - expect(transformed_data).to eq(data) - end - end - - context 'when nested data is not an array or hash' do - it 'only removes top level data/group keys' do - data = { - 'data' => { - 'group' => 'test' - } - } - - transformed_data = described_class.new.transform(nil, data) - - expect(transformed_data).to eq('test') - end - end - end -end diff --git a/spec/lib/bulk_imports/common/transformers/hash_key_digger_spec.rb b/spec/lib/bulk_imports/common/transformers/hash_key_digger_spec.rb new file mode 100644 index 00000000000..2b33701653e --- /dev/null +++ b/spec/lib/bulk_imports/common/transformers/hash_key_digger_spec.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe BulkImports::Common::Transformers::HashKeyDigger do + describe '#transform' do + it 'when the key_path is an array' do + data = { foo: { bar: :value } } + key_path = %i[foo bar] + transformed = described_class.new(key_path: key_path).transform(nil, data) + + expect(transformed).to eq(:value) + end + + it 'when the key_path is not an array' do + data = { foo: { bar: :value } } + key_path = :foo + transformed = described_class.new(key_path: key_path).transform(nil, data) + + expect(transformed).to eq({ bar: :value }) + end + + it "when the data is not a hash" do + expect { described_class.new(key_path: nil).transform(nil, nil) } + .to raise_error(ArgumentError, "Given data must be a Hash") + end + end +end diff --git a/spec/lib/bulk_imports/groups/pipelines/group_pipeline_spec.rb b/spec/lib/bulk_imports/groups/pipelines/group_pipeline_spec.rb index 3949dd23b49..a132e964141 100644 --- a/spec/lib/bulk_imports/groups/pipelines/group_pipeline_spec.rb +++ b/spec/lib/bulk_imports/groups/pipelines/group_pipeline_spec.rb @@ -72,7 +72,6 @@ RSpec.describe BulkImports::Groups::Pipelines::GroupPipeline do describe 'pipeline parts' do it { expect(described_class).to include_module(BulkImports::Pipeline) } - it { expect(described_class).to include_module(BulkImports::Pipeline::Attributes) } it { expect(described_class).to include_module(BulkImports::Pipeline::Runner) } it 'has extractors' do @@ -90,13 +89,16 @@ RSpec.describe BulkImports::Groups::Pipelines::GroupPipeline do it 'has transformers' do expect(described_class.transformers) .to contain_exactly( - { klass: BulkImports::Common::Transformers::GraphqlCleanerTransformer, options: nil }, + { klass: BulkImports::Common::Transformers::HashKeyDigger, options: { key_path: %w[data group] } }, { klass: BulkImports::Common::Transformers::UnderscorifyKeysTransformer, options: nil }, - { klass: BulkImports::Groups::Transformers::GroupAttributesTransformer, options: nil }) + { klass: BulkImports::Groups::Transformers::GroupAttributesTransformer, options: nil } + ) end it 'has loaders' do - expect(described_class.loaders).to contain_exactly({ klass: BulkImports::Groups::Loaders::GroupLoader, options: nil }) + expect(described_class.loaders).to contain_exactly({ + klass: BulkImports::Groups::Loaders::GroupLoader, options: nil + }) end end end diff --git a/spec/lib/bulk_imports/groups/pipelines/subgroup_entities_pipeline_spec.rb b/spec/lib/bulk_imports/groups/pipelines/subgroup_entities_pipeline_spec.rb index 60a4a796682..ace8eb4d9f6 100644 --- a/spec/lib/bulk_imports/groups/pipelines/subgroup_entities_pipeline_spec.rb +++ b/spec/lib/bulk_imports/groups/pipelines/subgroup_entities_pipeline_spec.rb @@ -55,7 +55,6 @@ RSpec.describe BulkImports::Groups::Pipelines::SubgroupEntitiesPipeline do describe 'pipeline parts' do it { expect(described_class).to include_module(BulkImports::Pipeline) } - it { expect(described_class).to include_module(BulkImports::Pipeline::Attributes) } it { expect(described_class).to include_module(BulkImports::Pipeline::Runner) } it 'has extractors' do diff --git a/spec/lib/bulk_imports/importers/group_importer_spec.rb b/spec/lib/bulk_imports/importers/group_importer_spec.rb index 95ac5925c97..7fec3b79179 100644 --- a/spec/lib/bulk_imports/importers/group_importer_spec.rb +++ b/spec/lib/bulk_imports/importers/group_importer_spec.rb @@ -18,8 +18,8 @@ RSpec.describe BulkImports::Importers::GroupImporter do subject { described_class.new(bulk_import_entity) } before do + allow(Gitlab).to receive(:ee?).and_return(false) allow(BulkImports::Pipeline::Context).to receive(:new).and_return(context) - stub_http_requests end describe '#execute' do @@ -39,18 +39,4 @@ RSpec.describe BulkImports::Importers::GroupImporter do expect(pipeline).to receive(:run).with(context) end end - - def stub_http_requests - double_response = double( - code: 200, - success?: true, - parsed_response: {}, - headers: {} - ) - - allow_next_instance_of(BulkImports::Clients::Http) do |client| - allow(client).to receive(:get).and_return(double_response) - allow(client).to receive(:post).and_return(double_response) - end - end end diff --git a/spec/lib/bulk_imports/pipeline/attributes_spec.rb b/spec/lib/bulk_imports/pipeline_spec.rb index 54c5dbd4cae..ba7bcf8788f 100644 --- a/spec/lib/bulk_imports/pipeline/attributes_spec.rb +++ b/spec/lib/bulk_imports/pipeline_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe BulkImports::Pipeline::Attributes do +RSpec.describe BulkImports::Pipeline do describe 'pipeline attributes' do before do stub_const('BulkImports::Extractor', Class.new) @@ -10,7 +10,7 @@ RSpec.describe BulkImports::Pipeline::Attributes do stub_const('BulkImports::Loader', Class.new) klass = Class.new do - include BulkImports::Pipeline::Attributes + include BulkImports::Pipeline extractor BulkImports::Extractor, { foo: :bar } transformer BulkImports::Transformer, { foo: :bar } diff --git a/spec/lib/feature/definition_spec.rb b/spec/lib/feature/definition_spec.rb index fa0207d829a..974310d8ea9 100644 --- a/spec/lib/feature/definition_spec.rb +++ b/spec/lib/feature/definition_spec.rb @@ -75,7 +75,7 @@ RSpec.describe Feature::Definition do describe '.load_from_file' do it 'properly loads a definition from file' do - expect(File).to receive(:read).with(path) { yaml_content } + expect_file_read(path, content: yaml_content) expect(described_class.send(:load_from_file, path).attributes) .to eq(definition.attributes) @@ -93,7 +93,7 @@ RSpec.describe Feature::Definition do context 'for invalid definition' do it 'raises exception' do - expect(File).to receive(:read).with(path) { '{}' } + expect_file_read(path, content: '{}') expect do described_class.send(:load_from_file, path) diff --git a/spec/lib/gitlab/background_migration/update_existing_users_that_require_two_factor_auth_spec.rb b/spec/lib/gitlab/background_migration/update_existing_users_that_require_two_factor_auth_spec.rb new file mode 100644 index 00000000000..bebb398413b --- /dev/null +++ b/spec/lib/gitlab/background_migration/update_existing_users_that_require_two_factor_auth_spec.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::BackgroundMigration::UpdateExistingUsersThatRequireTwoFactorAuth, schema: 20201030121314 do + include MigrationHelpers::NamespacesHelpers + + let(:group_with_2fa_parent) { create_namespace('parent', Gitlab::VisibilityLevel::PRIVATE) } + let(:group_with_2fa_child) { create_namespace('child', Gitlab::VisibilityLevel::PRIVATE, parent_id: group_with_2fa_parent.id) } + let(:members_table) { table(:members) } + let(:users_table) { table(:users) } + + subject { described_class.new } + + describe '#perform' do + context 'with group members' do + let(:user_1) { create_user('user@example.com') } + let!(:member) { create_group_member(user_1, group_with_2fa_parent) } + let!(:user_without_group) { create_user('user_without@example.com') } + let(:user_other) { create_user('user_other@example.com') } + let!(:member_other) { create_group_member(user_other, group_with_2fa_parent) } + + it 'updates user when user should not be required to establish two factor authentication' do + subject.perform(user_1.id, user_without_group.id) + + expect(user_1.reload.require_two_factor_authentication_from_group).to eq(false) + end + + it 'does not update user when user is member of group that requires two factor authentication' do + group = create_namespace('other', Gitlab::VisibilityLevel::PRIVATE, require_two_factor_authentication: true) + create_group_member(user_1, group) + + subject.perform(user_1.id, user_without_group.id) + + expect(user_1.reload.require_two_factor_authentication_from_group).to eq(true) + end + + it 'does not update user who is not in current batch' do + subject.perform(user_1.id, user_without_group.id) + + expect(user_other.reload.require_two_factor_authentication_from_group).to eq(true) + end + + it 'updates all users in current batch' do + subject.perform(user_1.id, user_other.id) + + expect(user_other.reload.require_two_factor_authentication_from_group).to eq(false) + end + + it 'does not update user when user is member of group which parent group requires two factor authentication' do + group_with_2fa_parent.update!(require_two_factor_authentication: true) + subject.perform(user_1.id, user_other.id) + + expect(user_1.reload.require_two_factor_authentication_from_group).to eq(true) + end + + it 'does not update user when user is member of group which has subgroup that requires two factor authentication' do + create_namespace('subgroup', Gitlab::VisibilityLevel::PRIVATE, require_two_factor_authentication: true, parent_id: group_with_2fa_child.id) + + subject.perform(user_1.id, user_other.id) + + expect(user_1.reload.require_two_factor_authentication_from_group).to eq(true) + end + end + end + + def create_user(email, require_2fa: true) + users_table.create!(email: email, projects_limit: 10, require_two_factor_authentication_from_group: require_2fa) + end + + def create_group_member(user, group) + members_table.create!(user_id: user.id, source_id: group.id, access_level: GroupMember::MAINTAINER, source_type: "Namespace", type: "GroupMember", notification_level: 3) + end +end diff --git a/spec/lib/gitlab/ci/parsers/codequality/code_climate_spec.rb b/spec/lib/gitlab/ci/parsers/codequality/code_climate_spec.rb new file mode 100644 index 00000000000..c6b8cf2a985 --- /dev/null +++ b/spec/lib/gitlab/ci/parsers/codequality/code_climate_spec.rb @@ -0,0 +1,138 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Ci::Parsers::Codequality::CodeClimate do + describe '#parse!' do + subject(:parse) { described_class.new.parse!(code_climate, codequality_report) } + + let(:codequality_report) { Gitlab::Ci::Reports::CodequalityReports.new } + let(:code_climate) do + [ + { + "categories": [ + "Complexity" + ], + "check_name": "argument_count", + "content": { + "body": "" + }, + "description": "Method `new_array` has 12 arguments (exceeds 4 allowed). Consider refactoring.", + "fingerprint": "15cdb5c53afd42bc22f8ca366a08d547", + "location": { + "path": "foo.rb", + "lines": { + "begin": 10, + "end": 10 + } + }, + "other_locations": [], + "remediation_points": 900000, + "severity": "major", + "type": "issue", + "engine_name": "structure" + } + ].to_json + end + + context "when data is code_climate style JSON" do + context "when there are no degradations" do + let(:code_climate) { [].to_json } + + it "returns a codequality report" do + expect { parse }.not_to raise_error + + expect(codequality_report.degradations_count).to eq(0) + end + end + + context "when there are degradations" do + it "returns a codequality report" do + expect { parse }.not_to raise_error + + expect(codequality_report.degradations_count).to eq(1) + end + end + end + + context "when data is not a valid JSON string" do + let(:code_climate) do + [ + { + "categories": [ + "Complexity" + ], + "check_name": "argument_count", + "content": { + "body": "" + }, + "description": "Method `new_array` has 12 arguments (exceeds 4 allowed). Consider refactoring.", + "fingerprint": "15cdb5c53afd42bc22f8ca366a08d547", + "location": { + "path": "foo.rb", + "lines": { + "begin": 10, + "end": 10 + } + }, + "other_locations": [], + "remediation_points": 900000, + "severity": "major", + "type": "issue", + "engine_name": "structure" + } + ] + end + + it "sets error_message" do + expect { parse }.not_to raise_error + + expect(codequality_report.error_message).to include('JSON parsing failed') + end + end + + context 'when degradations contain an invalid one' do + let(:code_climate) do + [ + { + "type": "Issue", + "check_name": "Rubocop/Metrics/ParameterLists", + "description": "Avoid parameter lists longer than 5 parameters. [12/5]", + "fingerprint": "ab5f8b935886b942d621399aefkaehfiaehf", + "severity": "minor" + }, + { + "categories": [ + "Complexity" + ], + "check_name": "argument_count", + "content": { + "body": "" + }, + "description": "Method `new_array` has 12 arguments (exceeds 4 allowed). Consider refactoring.", + "fingerprint": "15cdb5c53afd42bc22f8ca366a08d547", + "location": { + "path": "foo.rb", + "lines": { + "begin": 10, + "end": 10 + } + }, + "other_locations": [], + "remediation_points": 900000, + "severity": "major", + "type": "issue", + "engine_name": "structure" + } + ].to_json + end + + it 'stops parsing the report' do + expect { parse }.not_to raise_error + + expect(codequality_report.degradations_count).to eq(0) + expect(codequality_report.error_message).to eq("Invalid degradation format: The property '#/' did not contain a required property of 'location'") + end + end + end +end diff --git a/spec/lib/gitlab/ci/parsers_spec.rb b/spec/lib/gitlab/ci/parsers_spec.rb index db9a5775d9f..b932cd81272 100644 --- a/spec/lib/gitlab/ci/parsers_spec.rb +++ b/spec/lib/gitlab/ci/parsers_spec.rb @@ -30,6 +30,14 @@ RSpec.describe Gitlab::Ci::Parsers do end end + context 'when file_type is codequality' do + let(:file_type) { 'codequality' } + + it 'fabricates the class' do + is_expected.to be_a(described_class::Codequality::CodeClimate) + end + end + context 'when file_type is terraform' do let(:file_type) { 'terraform' } diff --git a/spec/lib/gitlab/ci/reports/codequality_reports_spec.rb b/spec/lib/gitlab/ci/reports/codequality_reports_spec.rb new file mode 100644 index 00000000000..44e67259369 --- /dev/null +++ b/spec/lib/gitlab/ci/reports/codequality_reports_spec.rb @@ -0,0 +1,136 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Ci::Reports::CodequalityReports do + let(:codequality_report) { described_class.new } + let(:degradation_1) do + { + "categories": [ + "Complexity" + ], + "check_name": "argument_count", + "content": { + "body": "" + }, + "description": "Method `new_array` has 12 arguments (exceeds 4 allowed). Consider refactoring.", + "fingerprint": "15cdb5c53afd42bc22f8ca366a08d547", + "location": { + "path": "foo.rb", + "lines": { + "begin": 10, + "end": 10 + } + }, + "other_locations": [], + "remediation_points": 900000, + "severity": "major", + "type": "issue", + "engine_name": "structure" + }.with_indifferent_access + end + + let(:degradation_2) do + { + "type": "Issue", + "check_name": "Rubocop/Metrics/ParameterLists", + "description": "Avoid parameter lists longer than 5 parameters. [12/5]", + "categories": [ + "Complexity" + ], + "remediation_points": 550000, + "location": { + "path": "foo.rb", + "positions": { + "begin": { + "column": 14, + "line": 10 + }, + "end": { + "column": 39, + "line": 10 + } + } + }, + "content": { + "body": "This cop checks for methods with too many parameters.\nThe maximum number of parameters is configurable.\nKeyword arguments can optionally be excluded from the total count." + }, + "engine_name": "rubocop", + "fingerprint": "ab5f8b935886b942d621399f5a2ca16e", + "severity": "minor" + }.with_indifferent_access + end + + it { expect(codequality_report.degradations).to eq({}) } + + describe '#add_degradation' do + context 'when there is a degradation' do + before do + codequality_report.add_degradation(degradation_1) + end + + it 'adds degradation to codequality report' do + expect(codequality_report.degradations.keys).to eq([degradation_1[:fingerprint]]) + expect(codequality_report.degradations.values.size).to eq(1) + end + end + + context 'when a required property is missing in the degradation' do + let(:invalid_degradation) do + { + "type": "Issue", + "check_name": "Rubocop/Metrics/ParameterLists", + "description": "Avoid parameter lists longer than 5 parameters. [12/5]", + "fingerprint": "ab5f8b935886b942d621399aefkaehfiaehf", + "severity": "minor" + }.with_indifferent_access + end + + it 'sets location as an error' do + codequality_report.add_degradation(invalid_degradation) + + expect(codequality_report.error_message).to eq("Invalid degradation format: The property '#/' did not contain a required property of 'location'") + end + end + end + + describe '#set_error_message' do + context 'when there is an error' do + it 'sets errors' do + codequality_report.set_error_message("error") + + expect(codequality_report.error_message).to eq("error") + end + end + end + + describe '#degradations_count' do + subject(:degradations_count) { codequality_report.degradations_count } + + context 'when there are many degradations' do + before do + codequality_report.add_degradation(degradation_1) + codequality_report.add_degradation(degradation_2) + end + + it 'returns the number of degradations' do + expect(degradations_count).to eq(2) + end + end + end + + describe '#all_degradations' do + subject(:all_degradations) { codequality_report.all_degradations } + + context 'when there are many degradations' do + before do + codequality_report.add_degradation(degradation_1) + codequality_report.add_degradation(degradation_2) + end + + it 'returns all degradations' do + expect(all_degradations).to contain_exactly(degradation_1, degradation_2) + end + end + end +end diff --git a/spec/lib/gitlab/cleanup/project_uploads_spec.rb b/spec/lib/gitlab/cleanup/project_uploads_spec.rb index 05d744d95e2..a99bdcc9a0f 100644 --- a/spec/lib/gitlab/cleanup/project_uploads_spec.rb +++ b/spec/lib/gitlab/cleanup/project_uploads_spec.rb @@ -15,10 +15,10 @@ RSpec.describe Gitlab::Cleanup::ProjectUploads do describe '#run!' do shared_examples_for 'moves the file' do shared_examples_for 'a real run' do - let(:args) { [dry_run: false] } + let(:args) { { dry_run: false } } it 'moves the file to its proper location' do - subject.run!(*args) + subject.run!(**args) expect(File.exist?(path)).to be_falsey expect(File.exist?(new_path)).to be_truthy @@ -28,13 +28,13 @@ RSpec.describe Gitlab::Cleanup::ProjectUploads do expect(logger).to receive(:info).with("Looking for orphaned project uploads to clean up...") expect(logger).to receive(:info).with("Did #{action}") - subject.run!(*args) + subject.run!(**args) end end shared_examples_for 'a dry run' do it 'does not move the file' do - subject.run!(*args) + subject.run!(**args) expect(File.exist?(path)).to be_truthy expect(File.exist?(new_path)).to be_falsey @@ -44,30 +44,30 @@ RSpec.describe Gitlab::Cleanup::ProjectUploads do expect(logger).to receive(:info).with("Looking for orphaned project uploads to clean up. Dry run...") expect(logger).to receive(:info).with("Can #{action}") - subject.run!(*args) + subject.run!(**args) end end context 'when dry_run is false' do - let(:args) { [dry_run: false] } + let(:args) { { dry_run: false } } it_behaves_like 'a real run' end context 'when dry_run is nil' do - let(:args) { [dry_run: nil] } + let(:args) { { dry_run: nil } } it_behaves_like 'a real run' end context 'when dry_run is true' do - let(:args) { [dry_run: true] } + let(:args) { { dry_run: true } } it_behaves_like 'a dry run' end context 'with dry_run not specified' do - let(:args) { [] } + let(:args) { {} } it_behaves_like 'a dry run' end diff --git a/spec/lib/gitlab/database/batch_count_spec.rb b/spec/lib/gitlab/database/batch_count_spec.rb index a1cc759e011..29688b18e94 100644 --- a/spec/lib/gitlab/database/batch_count_spec.rb +++ b/spec/lib/gitlab/database/batch_count_spec.rb @@ -130,6 +130,16 @@ RSpec.describe Gitlab::Database::BatchCount do expect(described_class.batch_count(model, start: model.minimum(:id), finish: model.maximum(:id))).to eq(5) end + it 'stops counting when finish value is reached' do + stub_const('Gitlab::Database::BatchCounter::MIN_REQUIRED_BATCH_SIZE', 0) + + expect(described_class.batch_count(model, + start: model.minimum(:id), + finish: model.maximum(:id) - 1, # Do not count the last record + batch_size: model.count - 2 # Ensure there are multiple batches + )).to eq(model.count - 1) + end + it "defaults the batch size to #{Gitlab::Database::BatchCounter::DEFAULT_BATCH_SIZE}" do min_id = model.minimum(:id) relation = instance_double(ActiveRecord::Relation) @@ -242,6 +252,19 @@ RSpec.describe Gitlab::Database::BatchCount do expect(described_class.batch_distinct_count(model, column, start: model.minimum(column), finish: model.maximum(column))).to eq(2) end + it 'stops counting when finish value is reached' do + # Create a new unique author that should not be counted + create(:issue) + + stub_const('Gitlab::Database::BatchCounter::MIN_REQUIRED_BATCH_SIZE', 0) + + expect(described_class.batch_distinct_count(model, column, + start: User.minimum(:id), + finish: User.maximum(:id) - 1, # Do not count the newly created issue + batch_size: model.count - 2 # Ensure there are multiple batches + )).to eq(2) + end + it 'counts with User min and max as start and finish' do expect(described_class.batch_distinct_count(model, column, start: User.minimum(:id), finish: User.maximum(:id))).to eq(2) end diff --git a/spec/lib/gitlab/database/migrations/background_migration_helpers_spec.rb b/spec/lib/gitlab/database/migrations/background_migration_helpers_spec.rb index 48132d68031..3e8563376ce 100644 --- a/spec/lib/gitlab/database/migrations/background_migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migrations/background_migration_helpers_spec.rb @@ -189,7 +189,51 @@ RSpec.describe Gitlab::Database::Migrations::BackgroundMigrationHelpers do end end - context "when the model doesn't have an ID column" do + context 'when the model specifies a primary_column_name' do + let!(:id1) { create(:container_expiration_policy).id } + let!(:id2) { create(:container_expiration_policy).id } + let!(:id3) { create(:container_expiration_policy).id } + + around do |example| + freeze_time { example.run } + end + + before do + ContainerExpirationPolicy.class_eval do + include EachBatch + end + end + + it 'returns the final expected delay', :aggregate_failures do + Sidekiq::Testing.fake! do + final_delay = model.queue_background_migration_jobs_by_range_at_intervals(ContainerExpirationPolicy, 'FooJob', 10.minutes, batch_size: 2, primary_column_name: :project_id) + + expect(final_delay.to_f).to eq(20.minutes.to_f) + expect(BackgroundMigrationWorker.jobs[0]['args']).to eq(['FooJob', [id1, id2]]) + expect(BackgroundMigrationWorker.jobs[0]['at']).to eq(10.minutes.from_now.to_f) + expect(BackgroundMigrationWorker.jobs[1]['args']).to eq(['FooJob', [id3, id3]]) + expect(BackgroundMigrationWorker.jobs[1]['at']).to eq(20.minutes.from_now.to_f) + end + end + + context "when the primary_column_name is not an integer" do + it 'raises error' do + expect do + model.queue_background_migration_jobs_by_range_at_intervals(ContainerExpirationPolicy, 'FooJob', 10.minutes, primary_column_name: :enabled) + end.to raise_error(StandardError, /is not an integer column/) + end + end + + context "when the primary_column_name does not exist" do + it 'raises error' do + expect do + model.queue_background_migration_jobs_by_range_at_intervals(ContainerExpirationPolicy, 'FooJob', 10.minutes, primary_column_name: :foo) + end.to raise_error(StandardError, /does not have an ID column of foo/) + end + end + end + + context "when the model doesn't have an ID or primary_column_name column" do it 'raises error (for now)' do expect do model.queue_background_migration_jobs_by_range_at_intervals(ProjectAuthorization, 'FooJob', 10.seconds) diff --git a/spec/lib/gitlab/database/postgres_hll_batch_distinct_counter_spec.rb b/spec/lib/gitlab/database/postgres_hll_batch_distinct_counter_spec.rb new file mode 100644 index 00000000000..5da86538297 --- /dev/null +++ b/spec/lib/gitlab/database/postgres_hll_batch_distinct_counter_spec.rb @@ -0,0 +1,132 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::PostgresHllBatchDistinctCounter do + let_it_be(:error_rate) { 4.9 } # HyperLogLog is a probabilistic algorithm, which provides estimated data, with given error margin + let_it_be(:fallback) { ::Gitlab::Database::BatchCounter::FALLBACK } + let_it_be(:small_batch_size) { calculate_batch_size(::Gitlab::Database::BatchCounter::MIN_REQUIRED_BATCH_SIZE) } + let(:model) { Issue } + let(:column) { :author_id } + + let(:in_transaction) { false } + + let_it_be(:user) { create(:user, email: 'email1@domain.com') } + let_it_be(:another_user) { create(:user, email: 'email2@domain.com') } + + def calculate_batch_size(batch_size) + zero_offset_modifier = -1 + + batch_size + zero_offset_modifier + end + + before do + allow(ActiveRecord::Base.connection).to receive(:transaction_open?).and_return(in_transaction) + end + + context 'different distribution of relation records' do + [10, 100, 100_000].each do |spread| + context "records are spread within #{spread}" do + before do + ids = (1..spread).to_a.sample(10) + create_list(:issue, 10).each_with_index do |issue, i| + issue.id = ids[i] + end + end + + it 'counts table' do + expect(described_class.new(model).estimate_distinct_count).to be_within(error_rate).percent_of(10) + end + end + end + end + + context 'unit test for different counting parameters' do + before_all do + create_list(:issue, 3, author: user) + create_list(:issue, 2, author: another_user) + end + + describe '#estimate_distinct_count' do + it 'counts table' do + expect(described_class.new(model).estimate_distinct_count).to be_within(error_rate).percent_of(5) + end + + it 'counts with column field' do + expect(described_class.new(model, column).estimate_distinct_count).to be_within(error_rate).percent_of(2) + end + + it 'counts with :id field' do + expect(described_class.new(model, :id).estimate_distinct_count).to be_within(error_rate).percent_of(5) + end + + it 'counts with "id" field' do + expect(described_class.new(model, "id").estimate_distinct_count).to be_within(error_rate).percent_of(5) + end + + it 'counts with table.column field' do + expect(described_class.new(model, "#{model.table_name}.#{column}").estimate_distinct_count).to be_within(error_rate).percent_of(2) + end + + it 'counts with Arel column' do + expect(described_class.new(model, model.arel_table[column]).estimate_distinct_count).to be_within(error_rate).percent_of(2) + end + + it 'counts over joined relations' do + expect(described_class.new(model.joins(:author), "users.email").estimate_distinct_count).to be_within(error_rate).percent_of(2) + end + + it 'counts with :column field with batch_size of 50K' do + expect(described_class.new(model, column).estimate_distinct_count(batch_size: 50_000)).to be_within(error_rate).percent_of(2) + end + + it 'will not count table with a batch size less than allowed' do + expect(described_class.new(model, column).estimate_distinct_count(batch_size: small_batch_size)).to eq(fallback) + end + + it 'counts with different number of batches and aggregates total result' do + stub_const('Gitlab::Database::PostgresHllBatchDistinctCounter::MIN_REQUIRED_BATCH_SIZE', 0) + + [1, 2, 4, 5, 6].each { |i| expect(described_class.new(model).estimate_distinct_count(batch_size: i)).to be_within(error_rate).percent_of(5) } + end + + it 'counts with a start and finish' do + expect(described_class.new(model, column).estimate_distinct_count(start: model.minimum(:id), finish: model.maximum(:id))).to be_within(error_rate).percent_of(2) + end + + it "defaults the batch size to #{Gitlab::Database::PostgresHllBatchDistinctCounter::DEFAULT_BATCH_SIZE}" do + min_id = model.minimum(:id) + batch_end_id = min_id + calculate_batch_size(Gitlab::Database::PostgresHllBatchDistinctCounter::DEFAULT_BATCH_SIZE) + + expect(model).to receive(:where).with("id" => min_id..batch_end_id).and_call_original + + described_class.new(model).estimate_distinct_count + end + + context 'when a transaction is open' do + let(:in_transaction) { true } + + it 'raises an error' do + expect { described_class.new(model, column).estimate_distinct_count }.to raise_error('BatchCount can not be run inside a transaction') + end + end + + context 'disallowed configurations' do + let(:default_batch_size) { Gitlab::Database::PostgresHllBatchDistinctCounter::DEFAULT_BATCH_SIZE } + + it 'returns fallback if start is bigger than finish' do + expect(described_class.new(model, column).estimate_distinct_count(start: 1, finish: 0)).to eq(fallback) + end + + it 'returns fallback if loops more than allowed' do + large_finish = Gitlab::Database::PostgresHllBatchDistinctCounter::MAX_ALLOWED_LOOPS * default_batch_size + 1 + expect(described_class.new(model, column).estimate_distinct_count(start: 1, finish: large_finish)).to eq(fallback) + end + + it 'returns fallback if batch size is less than min required' do + expect(described_class.new(model, column).estimate_distinct_count(batch_size: small_batch_size)).to eq(fallback) + end + end + end + end +end diff --git a/spec/lib/gitlab/email/smime/certificate_spec.rb b/spec/lib/gitlab/email/smime/certificate_spec.rb index e4a085d971b..f7bb933e348 100644 --- a/spec/lib/gitlab/email/smime/certificate_spec.rb +++ b/spec/lib/gitlab/email/smime/certificate_spec.rb @@ -69,8 +69,8 @@ RSpec.describe Gitlab::Email::Smime::Certificate do describe '.from_files' do it 'parses correctly a certificate and key' do - allow(File).to receive(:read).with('a_key').and_return(@cert[:key].to_s) - allow(File).to receive(:read).with('a_cert').and_return(@cert[:cert].to_pem) + stub_file_read('a_key', content: @cert[:key].to_s) + stub_file_read('a_cert', content: @cert[:cert].to_pem) parsed_cert = described_class.from_files('a_key', 'a_cert') @@ -79,9 +79,9 @@ RSpec.describe Gitlab::Email::Smime::Certificate do context 'with optional ca_certs' do it 'parses correctly certificate, key and ca_certs' do - allow(File).to receive(:read).with('a_key').and_return(@cert[:key].to_s) - allow(File).to receive(:read).with('a_cert').and_return(@cert[:cert].to_pem) - allow(File).to receive(:read).with('a_ca_cert').and_return(@intermediate_ca[:cert].to_pem) + stub_file_read('a_key', content: @cert[:key].to_s) + stub_file_read('a_cert', content: @cert[:cert].to_pem) + stub_file_read('a_ca_cert', content: @intermediate_ca[:cert].to_pem) parsed_cert = described_class.from_files('a_key', 'a_cert', 'a_ca_cert') @@ -94,8 +94,8 @@ RSpec.describe Gitlab::Email::Smime::Certificate do it 'parses correctly a certificate and key' do cert = generate_cert(signer_ca: @root_ca) - allow(File).to receive(:read).with('a_key').and_return(cert[:key].to_s) - allow(File).to receive(:read).with('a_cert').and_return(cert[:cert].to_pem) + stub_file_read('a_key', content: cert[:key].to_s) + stub_file_read('a_cert', content: cert[:cert].to_pem) parsed_cert = described_class.from_files('a_key', 'a_cert') diff --git a/spec/lib/gitlab/experimentation/controller_concern_spec.rb b/spec/lib/gitlab/experimentation/controller_concern_spec.rb index 2fe3d36daf7..047b4e7458e 100644 --- a/spec/lib/gitlab/experimentation/controller_concern_spec.rb +++ b/spec/lib/gitlab/experimentation/controller_concern_spec.rb @@ -6,12 +6,10 @@ RSpec.describe Gitlab::Experimentation::ControllerConcern, type: :controller do before do stub_const('Gitlab::Experimentation::EXPERIMENTS', { backwards_compatible_test_experiment: { - environment: environment, tracking_category: 'Team', use_backwards_compatible_subject_index: true }, test_experiment: { - environment: environment, tracking_category: 'Team' } } @@ -21,7 +19,6 @@ RSpec.describe Gitlab::Experimentation::ControllerConcern, type: :controller do Feature.enable_percentage_of_time(:test_experiment_experiment_percentage, enabled_percentage) end - let(:environment) { Rails.env.test? } let(:enabled_percentage) { 10 } controller(ApplicationController) do @@ -391,10 +388,8 @@ RSpec.describe Gitlab::Experimentation::ControllerConcern, type: :controller do context 'do not track' do before do + stub_experiment(test_experiment: true) allow(controller).to receive(:current_user).and_return(user) - allow_next_instance_of(described_class) do |instance| - allow(instance).to receive(:experiment_enabled?).with(:test_experiment).and_return(false) - end end context 'is disabled' do diff --git a/spec/lib/gitlab/experimentation/experiment_spec.rb b/spec/lib/gitlab/experimentation/experiment_spec.rb new file mode 100644 index 00000000000..4af76e9e920 --- /dev/null +++ b/spec/lib/gitlab/experimentation/experiment_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Experimentation::Experiment do + using RSpec::Parameterized::TableSyntax + + let(:percentage) { 50 } + let(:params) do + { + tracking_category: 'Category1', + use_backwards_compatible_subject_index: true + } + end + + before do + feature = double('FeatureFlag', percentage_of_time_value: percentage ) + expect(Feature).to receive(:get).with(:experiment_key_experiment_percentage).and_return(feature) + end + + subject(:experiment) { described_class.new(:experiment_key, **params) } + + describe '#enabled?' do + before do + allow(Gitlab).to receive(:dev_env_or_com?).and_return(on_gitlab_com) + end + + subject { experiment.enabled? } + + where(:on_gitlab_com, :percentage, :is_enabled) do + true | 0 | false + true | 10 | true + false | 0 | false + false | 10 | false + end + + with_them do + it { is_expected.to eq(is_enabled) } + end + end + + describe '#enabled_for_index?' do + subject { experiment.enabled_for_index?(index) } + + where(:index, :percentage, :is_enabled) do + 50 | 40 | false + 40 | 50 | true + nil | 50 | false + end + + with_them do + it { is_expected.to eq(is_enabled) } + end + end +end diff --git a/spec/lib/gitlab/experimentation_spec.rb b/spec/lib/gitlab/experimentation_spec.rb index ebf98a0151f..4130d5f9184 100644 --- a/spec/lib/gitlab/experimentation_spec.rb +++ b/spec/lib/gitlab/experimentation_spec.rb @@ -13,7 +13,6 @@ RSpec.describe Gitlab::Experimentation::EXPERIMENTS do :invite_members_version_a, :invite_members_version_b, :invite_members_empty_group_version_a, - :new_create_project_ui, :contact_sales_btn_in_app, :customize_homepage, :invite_email, @@ -33,27 +32,25 @@ RSpec.describe Gitlab::Experimentation, :snowplow do before do stub_const('Gitlab::Experimentation::EXPERIMENTS', { backwards_compatible_test_experiment: { - environment: environment, tracking_category: 'Team', use_backwards_compatible_subject_index: true }, test_experiment: { - environment: environment, tracking_category: 'Team' } }) Feature.enable_percentage_of_time(:backwards_compatible_test_experiment_experiment_percentage, enabled_percentage) Feature.enable_percentage_of_time(:test_experiment_experiment_percentage, enabled_percentage) + allow(Gitlab).to receive(:com?).and_return(true) end - let(:environment) { Rails.env.test? } let(:enabled_percentage) { 10 } describe '.enabled?' do subject { described_class.enabled?(:test_experiment) } - context 'feature toggle is enabled, we are on the right environment and we are selected' do + context 'feature toggle is enabled and we are selected' do it { is_expected.to be_truthy } end @@ -68,20 +65,6 @@ RSpec.describe Gitlab::Experimentation, :snowplow do it { is_expected.to be_falsey } end - - describe 'we are on the wrong environment' do - let(:environment) { ::Gitlab.com? } - - it { is_expected.to be_falsey } - - it 'ensures the typically less expensive environment is checked before the more expensive call to database for Feature' do - expect_next_instance_of(described_class::Experiment) do |experiment| - expect(experiment).not_to receive(:enabled?) - end - - subject - end - end end describe '.enabled_for_value?' do diff --git a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb index b09bd9dff1b..36f7d89dd0f 100644 --- a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb @@ -357,7 +357,7 @@ RSpec.describe Gitlab::GitalyClient::CommitService do end it 'sends an RPC request with the correct payload' do - expect(client.commits_by_message(query, options)).to match_array(wrap_commits(commits)) + expect(client.commits_by_message(query, **options)).to match_array(wrap_commits(commits)) end end diff --git a/spec/lib/gitlab/gitaly_client_spec.rb b/spec/lib/gitlab/gitaly_client_spec.rb index 16dd2bbee6d..7fcb11c4dfd 100644 --- a/spec/lib/gitlab/gitaly_client_spec.rb +++ b/spec/lib/gitlab/gitaly_client_spec.rb @@ -53,7 +53,7 @@ RSpec.describe Gitlab::GitalyClient do describe '.filesystem_id_from_disk' do it 'catches errors' do [Errno::ENOENT, Errno::EACCES, JSON::ParserError].each do |error| - allow(File).to receive(:read).with(described_class.storage_metadata_file_path('default')).and_raise(error) + stub_file_read(described_class.storage_metadata_file_path('default'), error: error) expect(described_class.filesystem_id_from_disk('default')).to be_nil end diff --git a/spec/lib/gitlab/gpg_spec.rb b/spec/lib/gitlab/gpg_spec.rb index 72c6c8efb5e..66ce2d5cb8a 100644 --- a/spec/lib/gitlab/gpg_spec.rb +++ b/spec/lib/gitlab/gpg_spec.rb @@ -143,6 +143,11 @@ RSpec.describe Gitlab::Gpg do end it 'keeps track of created and removed keychains in counters' do + # Gitlab::Gpg may be memoizing stale counters if a preceding spec resets the Prometheus registry + # https://gitlab.com/gitlab-org/gitlab/-/issues/286874 + described_class.remove_instance_variable(:@tmp_keychains_created) + described_class.remove_instance_variable(:@tmp_keychains_removed) + created = Gitlab::Metrics.counter(:gpg_tmp_keychains_created_total, 'The number of temporary GPG keychains') removed = Gitlab::Metrics.counter(:gpg_tmp_keychains_removed_total, 'The number of temporary GPG keychains') diff --git a/spec/lib/gitlab/graphql/pagination/array_connection_spec.rb b/spec/lib/gitlab/graphql/pagination/array_connection_spec.rb new file mode 100644 index 00000000000..03cf53bb990 --- /dev/null +++ b/spec/lib/gitlab/graphql/pagination/array_connection_spec.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ::Gitlab::Graphql::Pagination::ArrayConnection do + let(:nodes) { (1..10) } + + subject(:connection) { described_class.new(nodes, max_page_size: 100) } + + it_behaves_like 'a connection with collection methods' + + it_behaves_like 'a redactable connection' do + let(:unwanted) { 5 } + end +end diff --git a/spec/lib/gitlab/graphql/pagination/externally_paginated_array_connection_spec.rb b/spec/lib/gitlab/graphql/pagination/externally_paginated_array_connection_spec.rb index 932bcd8cd92..84e8f8b95e8 100644 --- a/spec/lib/gitlab/graphql/pagination/externally_paginated_array_connection_spec.rb +++ b/spec/lib/gitlab/graphql/pagination/externally_paginated_array_connection_spec.rb @@ -13,6 +13,12 @@ RSpec.describe Gitlab::Graphql::Pagination::ExternallyPaginatedArrayConnection d described_class.new(all_nodes, { max_page_size: values.size }.merge(arguments)) end + it_behaves_like 'a connection with collection methods' + + it_behaves_like 'a redactable connection' do + let(:unwanted) { 3 } + end + describe '#nodes' do let(:paged_nodes) { connection.nodes } diff --git a/spec/lib/gitlab/graphql/pagination/keyset/connection_spec.rb b/spec/lib/gitlab/graphql/pagination/keyset/connection_spec.rb index c8f368b15fc..1fee24bdc1f 100644 --- a/spec/lib/gitlab/graphql/pagination/keyset/connection_spec.rb +++ b/spec/lib/gitlab/graphql/pagination/keyset/connection_spec.rb @@ -21,6 +21,13 @@ RSpec.describe Gitlab::Graphql::Pagination::Keyset::Connection do Gitlab::Json.parse(Base64Bp.urlsafe_decode64(cursor)) end + it_behaves_like 'a connection with collection methods' + + it_behaves_like 'a redactable connection' do + let_it_be(:projects) { create_list(:project, 2) } + let(:unwanted) { projects.second } + end + describe '#cursor_for' do let(:project) { create(:project) } let(:cursor) { connection.cursor_for(project) } diff --git a/spec/lib/gitlab/graphql/pagination/offset_active_record_relation_connection_spec.rb b/spec/lib/gitlab/graphql/pagination/offset_active_record_relation_connection_spec.rb index 86f35de94ed..1ca7c1c3c69 100644 --- a/spec/lib/gitlab/graphql/pagination/offset_active_record_relation_connection_spec.rb +++ b/spec/lib/gitlab/graphql/pagination/offset_active_record_relation_connection_spec.rb @@ -6,4 +6,15 @@ RSpec.describe Gitlab::Graphql::Pagination::OffsetActiveRecordRelationConnection it 'subclasses from GraphQL::Relay::RelationConnection' do expect(described_class.superclass).to eq GraphQL::Pagination::ActiveRecordRelationConnection end + + it_behaves_like 'a connection with collection methods' do + let(:connection) { described_class.new(Project.all) } + end + + it_behaves_like 'a redactable connection' do + let_it_be(:users) { create_list(:user, 2) } + + let(:connection) { described_class.new(User.all, max_page_size: 10) } + let(:unwanted) { users.second } + end end diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 38fe2781331..c60f916add9 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -28,6 +28,7 @@ issues: - events - merge_requests_closing_issues - metrics +- metric_images - timelogs - issuable_severity - issuable_sla @@ -552,6 +553,8 @@ project: - pipeline_artifacts - terraform_states - alert_management_http_integrations +- exported_protected_branches +- incident_management_oncall_schedules award_emoji: - awardable - user diff --git a/spec/lib/gitlab/kubernetes/helm/v2/reset_command_spec.rb b/spec/lib/gitlab/kubernetes/helm/v2/reset_command_spec.rb index 9e580cea397..2a3a4cec2b0 100644 --- a/spec/lib/gitlab/kubernetes/helm/v2/reset_command_spec.rb +++ b/spec/lib/gitlab/kubernetes/helm/v2/reset_command_spec.rb @@ -12,32 +12,14 @@ RSpec.describe Gitlab::Kubernetes::Helm::V2::ResetCommand do it_behaves_like 'helm command generator' do let(:commands) do <<~EOS - helm reset - kubectl delete replicaset -n gitlab-managed-apps -l name\\=tiller - kubectl delete clusterrolebinding tiller-admin + export HELM_HOST="localhost:44134" + tiller -listen ${HELM_HOST} -alsologtostderr & + helm init --client-only + helm reset --force EOS end end - context 'when there is a ca.pem file' do - let(:files) { { 'ca.pem': 'some file content' } } - - it_behaves_like 'helm command generator' do - let(:commands) do - <<~EOS1.squish + "\n" + <<~EOS2 - helm reset - --tls - --tls-ca-cert /data/helm/helm/config/ca.pem - --tls-cert /data/helm/helm/config/cert.pem - --tls-key /data/helm/helm/config/key.pem - EOS1 - kubectl delete replicaset -n gitlab-managed-apps -l name\\=tiller - kubectl delete clusterrolebinding tiller-admin - EOS2 - end - end - end - describe '#pod_name' do subject { reset_command.pod_name } diff --git a/spec/lib/gitlab/rack_attack_spec.rb b/spec/lib/gitlab/rack_attack_spec.rb new file mode 100644 index 00000000000..ac24bdf3a62 --- /dev/null +++ b/spec/lib/gitlab/rack_attack_spec.rb @@ -0,0 +1,77 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::RackAttack, :aggregate_failures do + describe '.configure' do + let(:fake_rack_attack) { class_double("Rack::Attack") } + let(:fake_rack_attack_request) { class_double("Rack::Attack::Request") } + + let(:throttles) do + { + throttle_unauthenticated: Gitlab::Throttle.unauthenticated_options, + throttle_authenticated_api: Gitlab::Throttle.authenticated_api_options, + throttle_product_analytics_collector: { limit: 100, period: 60 }, + throttle_unauthenticated_protected_paths: Gitlab::Throttle.unauthenticated_options, + throttle_authenticated_protected_paths_api: Gitlab::Throttle.authenticated_api_options, + throttle_authenticated_protected_paths_web: Gitlab::Throttle.authenticated_web_options + } + end + + before do + stub_const("Rack::Attack", fake_rack_attack) + stub_const("Rack::Attack::Request", fake_rack_attack_request) + + allow(fake_rack_attack).to receive(:throttle) + allow(fake_rack_attack).to receive(:track) + allow(fake_rack_attack).to receive(:safelist) + allow(fake_rack_attack).to receive(:blocklist) + end + + it 'extends the request class' do + described_class.configure(fake_rack_attack) + + expect(fake_rack_attack_request).to include(described_class::Request) + end + + it 'configures the safelist' do + described_class.configure(fake_rack_attack) + + expect(fake_rack_attack).to have_received(:safelist).with('throttle_bypass_header') + end + + it 'configures throttles if no dry-run was configured' do + described_class.configure(fake_rack_attack) + + throttles.each do |throttle, options| + expect(fake_rack_attack).to have_received(:throttle).with(throttle.to_s, options) + end + end + + it 'configures tracks if dry-run was configured for all throttles' do + stub_env('GITLAB_THROTTLE_DRY_RUN', '*') + + described_class.configure(fake_rack_attack) + + throttles.each do |throttle, options| + expect(fake_rack_attack).to have_received(:track).with(throttle.to_s, options) + end + expect(fake_rack_attack).not_to have_received(:throttle) + end + + it 'configures tracks and throttles with a selected set of dry-runs' do + dry_run_throttles = throttles.each_key.first(2) + regular_throttles = throttles.keys[2..-1] + stub_env('GITLAB_THROTTLE_DRY_RUN', dry_run_throttles.join(',')) + + described_class.configure(fake_rack_attack) + + dry_run_throttles.each do |throttle| + expect(fake_rack_attack).to have_received(:track).with(throttle.to_s, throttles[throttle]) + end + regular_throttles.each do |throttle| + expect(fake_rack_attack).to have_received(:throttle).with(throttle.to_s, throttles[throttle]) + end + end + end +end diff --git a/spec/lib/gitlab/tracking/destinations/product_analytics_spec.rb b/spec/lib/gitlab/tracking/destinations/product_analytics_spec.rb new file mode 100644 index 00000000000..63e2e930acd --- /dev/null +++ b/spec/lib/gitlab/tracking/destinations/product_analytics_spec.rb @@ -0,0 +1,84 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Tracking::Destinations::ProductAnalytics do + let(:emitter) { SnowplowTracker::Emitter.new('localhost', buffer_size: 1) } + let(:tracker) { SnowplowTracker::Tracker.new(emitter, SnowplowTracker::Subject.new, 'namespace', 'app_id') } + + describe '#event' do + shared_examples 'does not send an event' do + it 'does not send an event' do + expect_any_instance_of(SnowplowTracker::Tracker).not_to receive(:track_struct_event) + + subject.event(allowed_category, allowed_action) + end + end + + let(:allowed_category) { 'epics' } + let(:allowed_action) { 'promote' } + let(:self_monitoring_project) { create(:project) } + + before do + stub_feature_flags(product_analytics_tracking: true) + stub_application_setting(self_monitoring_project_id: self_monitoring_project.id) + stub_application_setting(usage_ping_enabled: true) + end + + context 'with allowed event' do + it 'sends an event to Product Analytics snowplow collector' do + expect(SnowplowTracker::AsyncEmitter) + .to receive(:new) + .with(ProductAnalytics::Tracker::COLLECTOR_URL, protocol: Gitlab.config.gitlab.protocol) + .and_return(emitter) + + expect(SnowplowTracker::Tracker) + .to receive(:new) + .with(emitter, an_instance_of(SnowplowTracker::Subject), Gitlab::Tracking::SNOWPLOW_NAMESPACE, self_monitoring_project.id.to_s) + .and_return(tracker) + + freeze_time do + expect(tracker) + .to receive(:track_struct_event) + .with(allowed_category, allowed_action, 'label', 'property', 1.5, nil, (Time.now.to_f * 1000).to_i) + + subject.event(allowed_category, allowed_action, label: 'label', property: 'property', value: 1.5) + end + end + end + + context 'with non-allowed event' do + it 'does not send an event' do + expect_any_instance_of(SnowplowTracker::Tracker).not_to receive(:track_struct_event) + + subject.event('category', 'action') + subject.event(allowed_category, 'action') + subject.event('category', allowed_action) + end + end + + context 'when self-monitoring project does not exist' do + before do + stub_application_setting(self_monitoring_project_id: nil) + end + + include_examples 'does not send an event' + end + + context 'when product_analytics_tracking FF is disabled' do + before do + stub_feature_flags(product_analytics_tracking: false) + end + + include_examples 'does not send an event' + end + + context 'when usage ping is disabled' do + before do + stub_application_setting(usage_ping_enabled: false) + end + + include_examples 'does not send an event' + end + end +end diff --git a/spec/lib/gitlab/tracking/destinations/snowplow_spec.rb b/spec/lib/gitlab/tracking/destinations/snowplow_spec.rb index ee63eb6de04..7de23cd9621 100644 --- a/spec/lib/gitlab/tracking/destinations/snowplow_spec.rb +++ b/spec/lib/gitlab/tracking/destinations/snowplow_spec.rb @@ -46,7 +46,7 @@ RSpec.describe Gitlab::Tracking::Destinations::Snowplow do it 'sends event to tracker' do allow(tracker).to receive(:track_self_describing_event).and_call_original - subject.self_describing_event('iglu:com.gitlab/foo/jsonschema/1-0-0', foo: 'bar') + subject.self_describing_event('iglu:com.gitlab/foo/jsonschema/1-0-0', { foo: 'bar' }) expect(tracker).to have_received(:track_self_describing_event) do |event, context, timestamp| expect(event.to_json[:schema]).to eq('iglu:com.gitlab/foo/jsonschema/1-0-0') @@ -71,7 +71,7 @@ RSpec.describe Gitlab::Tracking::Destinations::Snowplow do it 'does not send event to tracker' do expect_any_instance_of(SnowplowTracker::Tracker).not_to receive(:track_self_describing_event) - subject.self_describing_event('iglu:com.gitlab/foo/jsonschema/1-0-0', foo: 'bar') + subject.self_describing_event('iglu:com.gitlab/foo/jsonschema/1-0-0', { foo: 'bar' }) end end end diff --git a/spec/lib/gitlab/tracking_spec.rb b/spec/lib/gitlab/tracking_spec.rb index 805bd92fd43..8efd4d4b848 100644 --- a/spec/lib/gitlab/tracking_spec.rb +++ b/spec/lib/gitlab/tracking_spec.rb @@ -36,6 +36,11 @@ RSpec.describe Gitlab::Tracking do end describe '.event' do + before do + allow_any_instance_of(Gitlab::Tracking::Destinations::Snowplow).to receive(:event) + allow_any_instance_of(Gitlab::Tracking::Destinations::ProductAnalytics).to receive(:event) + end + it 'delegates to snowplow destination' do expect_any_instance_of(Gitlab::Tracking::Destinations::Snowplow) .to receive(:event) @@ -43,6 +48,14 @@ RSpec.describe Gitlab::Tracking do described_class.event('category', 'action', label: 'label', property: 'property', value: 1.5) end + + it 'delegates to ProductAnalytics destination' do + expect_any_instance_of(Gitlab::Tracking::Destinations::ProductAnalytics) + .to receive(:event) + .with('category', 'action', label: 'label', property: 'property', value: 1.5, context: nil) + + described_class.event('category', 'action', label: 'label', property: 'property', value: 1.5) + end end describe '.self_describing_event' do @@ -51,7 +64,7 @@ RSpec.describe Gitlab::Tracking do .to receive(:self_describing_event) .with('iglu:com.gitlab/foo/jsonschema/1-0-0', { foo: 'bar' }, context: nil) - described_class.self_describing_event('iglu:com.gitlab/foo/jsonschema/1-0-0', foo: 'bar') + described_class.self_describing_event('iglu:com.gitlab/foo/jsonschema/1-0-0', { foo: 'bar' }) end end end diff --git a/spec/lib/gitlab/usage_data_counters/aggregated_metrics_spec.rb b/spec/lib/gitlab/usage_data_counters/aggregated_metrics_spec.rb index e9fb5346eae..c0deb2aa00c 100644 --- a/spec/lib/gitlab/usage_data_counters/aggregated_metrics_spec.rb +++ b/spec/lib/gitlab/usage_data_counters/aggregated_metrics_spec.rb @@ -8,7 +8,7 @@ RSpec.describe 'aggregated metrics' do Gitlab::UsageDataCounters::HLLRedisCounter.known_event?(event) end - failure_message do + failure_message do |event| "Event with name: `#{event}` can not be found within `#{Gitlab::UsageDataCounters::HLLRedisCounter::KNOWN_EVENTS_PATH}`" end end diff --git a/spec/lib/gitlab/usage_data_counters/base_counter_spec.rb b/spec/lib/gitlab/usage_data_counters/base_counter_spec.rb new file mode 100644 index 00000000000..4a31191d75f --- /dev/null +++ b/spec/lib/gitlab/usage_data_counters/base_counter_spec.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::UsageDataCounters::BaseCounter do + describe '.fetch_supported_event' do + subject { described_class.fetch_supported_event(event_name) } + + let(:event_name) { 'generic_event' } + let(:prefix) { 'generic' } + let(:known_events) { %w[event another_event] } + + before do + allow(described_class).to receive(:prefix) { prefix } + allow(described_class).to receive(:known_events) { known_events } + end + + it 'returns the matching event' do + is_expected.to eq 'event' + end + + context 'when event is unknown' do + let(:event_name) { 'generic_unknown_event' } + + it { is_expected.to be_nil } + end + + context 'when prefix does not match the event name' do + let(:prefix) { 'special' } + + it { is_expected.to be_nil } + end + end +end diff --git a/spec/lib/gitlab/usage_data_counters/hll_redis_counter_spec.rb b/spec/lib/gitlab/usage_data_counters/hll_redis_counter_spec.rb index 93704a39555..579fc048663 100644 --- a/spec/lib/gitlab/usage_data_counters/hll_redis_counter_spec.rb +++ b/spec/lib/gitlab/usage_data_counters/hll_redis_counter_spec.rb @@ -30,6 +30,7 @@ RSpec.describe Gitlab::UsageDataCounters::HLLRedisCounter, :clean_gitlab_redis_s 'search', 'source_code', 'incident_management', + 'incident_management_alerts', 'testing', 'issues_edit', 'ci_secrets_management', diff --git a/spec/lib/gitlab/usage_data_counters/search_counter_spec.rb b/spec/lib/gitlab/usage_data_counters/search_counter_spec.rb index b55e20ba555..17188a75ccb 100644 --- a/spec/lib/gitlab/usage_data_counters/search_counter_spec.rb +++ b/spec/lib/gitlab/usage_data_counters/search_counter_spec.rb @@ -20,4 +20,12 @@ RSpec.describe Gitlab::UsageDataCounters::SearchCounter, :clean_gitlab_redis_sha context 'navbar_searches counter' do it_behaves_like 'usage counter with totals', :navbar_searches end + + describe '.fetch_supported_event' do + subject { described_class.fetch_supported_event(event_name) } + + let(:event_name) { 'all_searches' } + + it { is_expected.to eq 'all_searches' } + end end diff --git a/spec/lib/gitlab/usage_data_counters_spec.rb b/spec/lib/gitlab/usage_data_counters_spec.rb new file mode 100644 index 00000000000..379a2cb778d --- /dev/null +++ b/spec/lib/gitlab/usage_data_counters_spec.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::UsageDataCounters do + describe '.usage_data_counters' do + subject { described_class.counters } + + it { is_expected.to all(respond_to :totals) } + it { is_expected.to all(respond_to :fallback_totals) } + end + + describe '.count' do + subject { described_class.count(event_name) } + + let(:event_name) { 'static_site_editor_views' } + + it 'increases a view counter' do + expect(Gitlab::UsageDataCounters::StaticSiteEditorCounter).to receive(:count).with('views') + + subject + end + + context 'when event_name is not defined' do + let(:event_name) { 'unknown' } + + it 'raises an exception' do + expect { subject }.to raise_error(Gitlab::UsageDataCounters::UnknownEvent) + end + end + end +end diff --git a/spec/lib/gitlab/usage_data_spec.rb b/spec/lib/gitlab/usage_data_spec.rb index d305b2c5bfe..746c2aef7e5 100644 --- a/spec/lib/gitlab/usage_data_spec.rb +++ b/spec/lib/gitlab/usage_data_spec.rb @@ -1235,7 +1235,7 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do subject { described_class.redis_hll_counters } let(:categories) { ::Gitlab::UsageDataCounters::HLLRedisCounter.categories } - let(:ineligible_total_categories) { %w[source_code testing ci_secrets_management] } + let(:ineligible_total_categories) { %w[source_code testing ci_secrets_management incident_management_alerts] } it 'has all known_events' do expect(subject).to have_key(:redis_hll_counters) diff --git a/spec/lib/gitlab/webpack/manifest_spec.rb b/spec/lib/gitlab/webpack/manifest_spec.rb index 1427bdd7d4f..08b4774dd67 100644 --- a/spec/lib/gitlab/webpack/manifest_spec.rb +++ b/spec/lib/gitlab/webpack/manifest_spec.rb @@ -97,7 +97,7 @@ RSpec.describe Gitlab::Webpack::Manifest do context "with dev server disabled" do before do allow(Gitlab.config.webpack.dev_server).to receive(:enabled).and_return(false) - allow(File).to receive(:read).with(::Rails.root.join("manifest_output/my_manifest.json")).and_return(manifest) + stub_file_read(::Rails.root.join("manifest_output/my_manifest.json"), content: manifest) end describe ".asset_paths" do @@ -105,7 +105,7 @@ RSpec.describe Gitlab::Webpack::Manifest do it "errors if we can't find the manifest" do allow(Gitlab.config.webpack).to receive(:manifest_filename).and_return('broken.json') - allow(File).to receive(:read).with(::Rails.root.join("manifest_output/broken.json")).and_raise(Errno::ENOENT) + stub_file_read(::Rails.root.join("manifest_output/broken.json"), error: Errno::ENOENT) expect { Gitlab::Webpack::Manifest.asset_paths("entry1") }.to raise_error(Gitlab::Webpack::Manifest::ManifestLoadError) end end diff --git a/spec/lib/gitlab_spec.rb b/spec/lib/gitlab_spec.rb index 7c2758bf27e..eaab39291b2 100644 --- a/spec/lib/gitlab_spec.rb +++ b/spec/lib/gitlab_spec.rb @@ -26,18 +26,17 @@ RSpec.describe Gitlab do end it 'returns the actual Git revision' do - expect(File).to receive(:read) - .with(described_class.root.join('REVISION')) - .and_return("abc123\n") + expect_file_read(described_class.root.join('REVISION'), content: "abc123\n") expect(described_class.revision).to eq('abc123') end it 'memoizes the revision' do + stub_file_read(described_class.root.join('REVISION'), content: "abc123\n") + expect(File).to receive(:read) - .once - .with(described_class.root.join('REVISION')) - .and_return("abc123\n") + .once + .with(described_class.root.join('REVISION')) 2.times { described_class.revision } end diff --git a/spec/lib/microsoft_teams/notifier_spec.rb b/spec/lib/microsoft_teams/notifier_spec.rb index c35d7e8420c..3b7892334dd 100644 --- a/spec/lib/microsoft_teams/notifier_spec.rb +++ b/spec/lib/microsoft_teams/notifier_spec.rb @@ -51,11 +51,11 @@ RSpec.describe MicrosoftTeams::Notifier do describe '#body' do it 'returns Markdown-based body when HTML was passed' do - expect(subject.send(:body, options)).to eq(body.to_json) + expect(subject.send(:body, **options)).to eq(body.to_json) end it 'fails when empty Hash was passed' do - expect { subject.send(:body, {}) }.to raise_error(ArgumentError) + expect { subject.send(:body, **{}) }.to raise_error(ArgumentError) end end end diff --git a/spec/lib/product_analytics/tracker_spec.rb b/spec/lib/product_analytics/tracker_spec.rb index 0d0660235f1..52470c9c039 100644 --- a/spec/lib/product_analytics/tracker_spec.rb +++ b/spec/lib/product_analytics/tracker_spec.rb @@ -5,53 +5,4 @@ require 'spec_helper' RSpec.describe ProductAnalytics::Tracker do it { expect(described_class::URL).to eq('http://localhost/-/sp.js') } it { expect(described_class::COLLECTOR_URL).to eq('localhost/-/collector') } - - describe '.event' do - after do - described_class.clear_memoization(:snowplow) - end - - context 'when usage ping is enabled' do - let(:tracker) { double } - let(:project_id) { 1 } - - before do - stub_application_setting(usage_ping_enabled: true, self_monitoring_project_id: project_id) - end - - it 'sends an event to Product Analytics snowplow collector' do - expect(SnowplowTracker::AsyncEmitter) - .to receive(:new) - .with(described_class::COLLECTOR_URL, { protocol: 'http' }) - .and_return('_emitter_') - - expect(SnowplowTracker::Tracker) - .to receive(:new) - .with('_emitter_', an_instance_of(SnowplowTracker::Subject), 'gl', project_id.to_s) - .and_return(tracker) - - freeze_time do - expect(tracker) - .to receive(:track_struct_event) - .with('category', 'action', '_label_', '_property_', '_value_', nil, (Time.current.to_f * 1000).to_i) - - described_class.event('category', 'action', label: '_label_', property: '_property_', - value: '_value_', context: nil) - end - end - end - - context 'when usage ping is disabled' do - before do - stub_application_setting(usage_ping_enabled: false) - end - - it 'does not send an event' do - expect(SnowplowTracker::Tracker).not_to receive(:new) - - described_class.event('category', 'action', label: '_label_', property: '_property_', - value: '_value_', context: nil) - end - end - end end diff --git a/spec/lib/quality/test_level_spec.rb b/spec/lib/quality/test_level_spec.rb index 0239c974755..1dd0a09542a 100644 --- a/spec/lib/quality/test_level_spec.rb +++ b/spec/lib/quality/test_level_spec.rb @@ -174,6 +174,10 @@ RSpec.describe Quality::TestLevel do expect(subject.level_for('spec/lib/gitlab/background_migration/archive_legacy_traces_spec.rb')).to eq(:migration) end + it 'returns the correct level for an EE file without passing a prefix' do + expect(subject.level_for('ee/spec/migrations/geo/migrate_ci_job_artifacts_to_separate_registry_spec.rb')).to eq(:migration) + end + it 'returns the correct level for a geo migration test' do expect(described_class.new('ee/').level_for('ee/spec/migrations/geo/migrate_ci_job_artifacts_to_separate_registry_spec.rb')).to eq(:migration) end |