From fd31bd1fc7954f69025d8e6bbe7f772ea9fb4bb0 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Tue, 11 Oct 2022 12:09:17 +0000 Subject: Add latest changes from gitlab-org/gitlab@master --- spec/fixtures/api/schemas/ml/get_experiment.json | 27 ++- spec/fixtures/api/schemas/ml/list_experiments.json | 39 ++++ spec/frontend/notebook/cells/output/index_spec.js | 1 + .../ci/pipeline/chain/limit/active_jobs_spec.rb | 2 - spec/lib/gitlab/ci/pipeline/chain/sequence_spec.rb | 13 -- .../importer/protected_branch_importer_spec.rb | 64 ++++++- .../representation/protected_branch_spec.rb | 11 +- .../models/ci/job_token/project_scope_link_spec.rb | 5 +- spec/models/ml/experiment_spec.rb | 33 ++-- spec/requests/api/ml/mlflow_spec.rb | 102 +++++++---- .../candidate_repository_spec.rb | 199 +++++++++++++++++++++ .../experiment_repository_spec.rb | 85 +++++++++ .../pages/invalidate_domain_cache_worker_spec.rb | 32 ++++ spec/workers/run_pipeline_schedule_worker_spec.rb | 3 +- 14 files changed, 530 insertions(+), 86 deletions(-) create mode 100644 spec/fixtures/api/schemas/ml/list_experiments.json create mode 100644 spec/services/ml/experiment_tracking/candidate_repository_spec.rb create mode 100644 spec/services/ml/experiment_tracking/experiment_repository_spec.rb (limited to 'spec') diff --git a/spec/fixtures/api/schemas/ml/get_experiment.json b/spec/fixtures/api/schemas/ml/get_experiment.json index cf8da7f999f..482455a89e1 100644 --- a/spec/fixtures/api/schemas/ml/get_experiment.json +++ b/spec/fixtures/api/schemas/ml/get_experiment.json @@ -6,18 +6,31 @@ "properties": { "experiment": { "type": "object", - "required" : [ + "required": [ "experiment_id", "name", "artifact_location", "lifecycle_stage" ], - "properties" : { - "experiment_id": { "type": "string" }, - "name": { "type": "string" }, - "artifact_location": { "type": "string" }, - "lifecycle_stage": { "type": { "enum" : ["active", "deleted"] } } + "properties": { + "experiment_id": { + "type": "string" + }, + "name": { + "type": "string" + }, + "artifact_location": { + "type": "string" + }, + "lifecycle_stage": { + "type": { + "enum": [ + "active", + "deleted" + ] + } + } } } } -} +} \ No newline at end of file diff --git a/spec/fixtures/api/schemas/ml/list_experiments.json b/spec/fixtures/api/schemas/ml/list_experiments.json new file mode 100644 index 00000000000..4c3e834abc6 --- /dev/null +++ b/spec/fixtures/api/schemas/ml/list_experiments.json @@ -0,0 +1,39 @@ +{ + "type": "object", + "required": [ + "experiments" + ], + "properties": { + "experiments": { + "type": "array", + "items": { + "type": "object", + "required": [ + "experiment_id", + "name", + "artifact_location", + "lifecycle_stage" + ], + "properties": { + "experiment_id": { + "type": "string" + }, + "name": { + "type": "string" + }, + "artifact_location": { + "type": "string" + }, + "lifecycle_stage": { + "type": { + "enum": [ + "active", + "deleted" + ] + } + } + } + } + } + } +} \ No newline at end of file diff --git a/spec/frontend/notebook/cells/output/index_spec.js b/spec/frontend/notebook/cells/output/index_spec.js index 97a7e22be60..8bf049235a9 100644 --- a/spec/frontend/notebook/cells/output/index_spec.js +++ b/spec/frontend/notebook/cells/output/index_spec.js @@ -53,6 +53,7 @@ describe('Output component', () => { expect(iframe.exists()).toBe(true); expect(iframe.element.getAttribute('sandbox')).toBe(''); expect(iframe.element.getAttribute('srcdoc')).toBe('

test

'); + expect(iframe.element.getAttribute('scrolling')).toBe('auto'); }); it('renders multiple raw HTML outputs', () => { diff --git a/spec/lib/gitlab/ci/pipeline/chain/limit/active_jobs_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/limit/active_jobs_spec.rb index 9587d6d4e23..bc453f1502b 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/limit/active_jobs_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/limit/active_jobs_spec.rb @@ -13,7 +13,6 @@ RSpec.describe ::Gitlab::Ci::Pipeline::Chain::Limit::ActiveJobs do ::Gitlab::Ci::Pipeline::Chain::Command, project: project, current_user: user, - limit_active_jobs_early?: feature_flag_enabled, save_incompleted: true, pipeline_seed: pipeline_seed_double ) @@ -29,7 +28,6 @@ RSpec.describe ::Gitlab::Ci::Pipeline::Chain::Limit::ActiveJobs do let(:existing_pipeline) { create(:ci_pipeline, project: project) } let(:step) { described_class.new(pipeline, command) } - let(:feature_flag_enabled) { true } let(:limit) { 10 } subject { step.perform! } diff --git a/spec/lib/gitlab/ci/pipeline/chain/sequence_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/sequence_spec.rb index ee32661f267..c69aa661b05 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/sequence_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/sequence_spec.rb @@ -100,19 +100,6 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Sequence do expect(histogram).to have_received(:observe) .with(hash_including(plan: project.actual_plan_name), 4) end - - context 'when feature flag ci_limit_active_jobs_early is disabled' do - before do - stub_feature_flags(ci_limit_active_jobs_early: false) - end - - it 'counts all the active builds' do - subject.build! - - expect(histogram).to have_received(:observe) - .with(hash_including(plan: project.actual_plan_name), 3) - end - end end end end diff --git a/spec/lib/gitlab/github_import/importer/protected_branch_importer_spec.rb b/spec/lib/gitlab/github_import/importer/protected_branch_importer_spec.rb index 12d143f3692..4529c2675ff 100644 --- a/spec/lib/gitlab/github_import/importer/protected_branch_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/protected_branch_importer_spec.rb @@ -7,12 +7,14 @@ RSpec.describe Gitlab::GithubImport::Importer::ProtectedBranchImporter do let(:branch_name) { 'protection' } let(:allow_force_pushes_on_github) { true } - let(:required_conversation_resolution) { true } + let(:required_conversation_resolution) { false } + let(:required_signatures) { false } let(:github_protected_branch) do Gitlab::GithubImport::Representation::ProtectedBranch.new( id: branch_name, allow_force_pushes: allow_force_pushes_on_github, - required_conversation_resolution: required_conversation_resolution + required_conversation_resolution: required_conversation_resolution, + required_signatures: required_signatures ) end @@ -54,6 +56,12 @@ RSpec.describe Gitlab::GithubImport::Importer::ProtectedBranchImporter do it 'does not change only_allow_merge_if_all_discussions_are_resolved' do expect { importer.execute }.not_to change(project, :only_allow_merge_if_all_discussions_are_resolved) end + + it 'does not change push_rule for the project' do + expect(project).not_to receive(:push_rule) + + importer.execute + end end context 'when branch is protected on GitLab' do @@ -115,6 +123,46 @@ RSpec.describe Gitlab::GithubImport::Importer::ProtectedBranchImporter do it_behaves_like 'does not change project attributes' end + + context 'when required_signatures rule is enabled' do + let(:required_signatures) { true } + let(:push_rules_feature_available?) { true } + + before do + stub_licensed_features(push_rules: push_rules_feature_available?) + end + + context 'when the push_rules feature is available', if: Gitlab.ee? do + context 'when project push_rules did previously exist' do + before do + create(:push_rule, project: project) + end + + it 'updates push_rule reject_unsigned_commits attribute' do + expect { importer.execute }.to change { project.reload.push_rule.reject_unsigned_commits }.to(true) + end + end + + context 'when project push_rules did not previously exist' do + it 'creates project push_rule with the enabled reject_unsigned_commits attribute' do + expect { importer.execute }.to change(project, :push_rule).from(nil) + expect(project.push_rule.reject_unsigned_commits).to be_truthy + end + end + end + + context 'when the push_rules feature is not available' do + let(:push_rules_feature_available?) { false } + + it_behaves_like 'does not change project attributes' + end + end + + context 'when required_signatures rule is disabled' do + let(:required_signatures) { false } + + it_behaves_like 'does not change project attributes' + end end context "when branch is not default" do @@ -129,6 +177,18 @@ RSpec.describe Gitlab::GithubImport::Importer::ProtectedBranchImporter do it_behaves_like 'does not change project attributes' end + + context 'when required_signatures rule is enabled' do + let(:required_signatures) { true } + + it_behaves_like 'does not change project attributes' + end + + context 'when required_signatures rule is disabled' do + let(:required_signatures) { false } + + it_behaves_like 'does not change project attributes' + end end end end diff --git a/spec/lib/gitlab/github_import/representation/protected_branch_spec.rb b/spec/lib/gitlab/github_import/representation/protected_branch_spec.rb index 1d8426c0ad8..21fd3b2e44a 100644 --- a/spec/lib/gitlab/github_import/representation/protected_branch_spec.rb +++ b/spec/lib/gitlab/github_import/representation/protected_branch_spec.rb @@ -25,7 +25,10 @@ RSpec.describe Gitlab::GithubImport::Representation::ProtectedBranch do describe '.from_api_response' do let(:response) do - response = Struct.new(:url, :allow_force_pushes, :required_conversation_resolution, keyword_init: true) + response = Struct.new( + :url, :allow_force_pushes, :required_conversation_resolution, :required_signatures, + keyword_init: true + ) enabled_setting = Struct.new(:enabled, keyword_init: true) response.new( url: 'https://example.com/branches/main/protection', @@ -34,6 +37,9 @@ RSpec.describe Gitlab::GithubImport::Representation::ProtectedBranch do ), required_conversation_resolution: enabled_setting.new( enabled: true + ), + required_signatures: enabled_setting.new( + enabled: true ) ) end @@ -49,7 +55,8 @@ RSpec.describe Gitlab::GithubImport::Representation::ProtectedBranch do { 'id' => 'main', 'allow_force_pushes' => true, - 'required_conversation_resolution' => true + 'required_conversation_resolution' => true, + 'required_signatures' => true } end diff --git a/spec/models/ci/job_token/project_scope_link_spec.rb b/spec/models/ci/job_token/project_scope_link_spec.rb index 4c1f11130a4..92ed86b55b2 100644 --- a/spec/models/ci/job_token/project_scope_link_spec.rb +++ b/spec/models/ci/job_token/project_scope_link_spec.rb @@ -7,6 +7,7 @@ RSpec.describe Ci::JobToken::ProjectScopeLink do it { is_expected.to belong_to(:target_project) } it { is_expected.to belong_to(:added_by) } + let_it_be(:group) { create(:group) } let_it_be(:project) { create(:project) } it_behaves_like 'cleanup by a loose foreign key' do @@ -97,14 +98,14 @@ RSpec.describe Ci::JobToken::ProjectScopeLink do context 'loose foreign key on ci_job_token_project_scope_links.source_project_id' do it_behaves_like 'cleanup by a loose foreign key' do - let!(:parent) { create(:project) } + let!(:parent) { create(:project, namespace: group) } let!(:model) { create(:ci_job_token_project_scope_link, source_project: parent) } end end context 'loose foreign key on ci_job_token_project_scope_links.target_project_id' do it_behaves_like 'cleanup by a loose foreign key' do - let!(:parent) { create(:project) } + let!(:parent) { create(:project, namespace: group) } let!(:model) { create(:ci_job_token_project_scope_link, target_project: parent) } end end diff --git a/spec/models/ml/experiment_spec.rb b/spec/models/ml/experiment_spec.rb index e300f82d290..789bb3aa88a 100644 --- a/spec/models/ml/experiment_spec.rb +++ b/spec/models/ml/experiment_spec.rb @@ -3,16 +3,19 @@ require 'spec_helper' RSpec.describe Ml::Experiment do + let_it_be(:exp) { create(:ml_experiments) } + let_it_be(:exp2) { create(:ml_experiments, project: exp.project) } + + let(:iid) { exp.iid } + let(:exp_name) { exp.name } + describe 'associations' do it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:user) } it { is_expected.to have_many(:candidates) } end - describe '#by_project_id_and_iid?' do - let(:exp) { create(:ml_experiments) } - let(:iid) { exp.iid } - + describe '#by_project_id_and_iid' do subject { described_class.by_project_id_and_iid(exp.project_id, iid) } context 'if exists' do @@ -26,10 +29,7 @@ RSpec.describe Ml::Experiment do end end - describe '#by_project_id_and_name?' do - let(:exp) { create(:ml_experiments) } - let(:exp_name) { exp.name } - + describe '#by_project_id_and_name' do subject { described_class.by_project_id_and_name(exp.project_id, exp_name) } context 'if exists' do @@ -43,20 +43,17 @@ RSpec.describe Ml::Experiment do end end - describe '#has_record?' do - let(:exp) { create(:ml_experiments) } - let(:exp_name) { exp.name } + describe '#by_project_id' do + let(:project_id) { exp.project_id } - subject { described_class.has_record?(exp.project_id, exp_name) } + subject { described_class.by_project_id(project_id) } - context 'if exists' do - it { is_expected.to be_truthy } - end + it { is_expected.to match_array([exp, exp2]) } - context 'if does not exist' do - let(:exp_name) { 'hello' } + context 'when project does not have experiment' do + let(:project_id) { non_existing_record_iid } - it { is_expected.to be_falsey } + it { is_expected.to be_empty } end end end diff --git a/spec/requests/api/ml/mlflow_spec.rb b/spec/requests/api/ml/mlflow_spec.rb index 859fb9de936..09e9359c0b3 100644 --- a/spec/requests/api/ml/mlflow_spec.rb +++ b/spec/requests/api/ml/mlflow_spec.rb @@ -139,9 +139,9 @@ RSpec.describe API::Ml::Mlflow do end end - describe 'GET /projects/:id/ml/mflow/api/2.0/mlflow/experiments/get' do + describe 'GET /projects/:id/ml/mlflow/api/2.0/mlflow/experiments/get' do let(:experiment_iid) { experiment.iid.to_s } - let(:route) { "/projects/#{project_id}/ml/mflow/api/2.0/mlflow/experiments/get?experiment_id=#{experiment_iid}" } + let(:route) { "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/experiments/get?experiment_id=#{experiment_iid}" } it 'returns the experiment', :aggregate_failures do expect(response).to have_gitlab_http_status(:ok) @@ -165,7 +165,7 @@ RSpec.describe API::Ml::Mlflow do end context 'and experiment_id is not passed' do - let(:route) { "/projects/#{project_id}/ml/mflow/api/2.0/mlflow/experiments/get" } + let(:route) { "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/experiments/get" } it_behaves_like 'Not Found - Resource Does Not Exist' end @@ -176,10 +176,40 @@ RSpec.describe API::Ml::Mlflow do end end - describe 'GET /projects/:id/ml/mflow/api/2.0/mlflow/experiments/get-by-name' do + describe 'GET /projects/:id/ml/mlflow/api/2.0/mlflow/experiments/list' do + let(:route) { "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/experiments/list" } + + it 'returns the experiments' do + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('ml/list_experiments') + expect(json_response).to include({ + 'experiments' => [ + 'experiment_id' => experiment.iid.to_s, + 'name' => experiment.name, + 'lifecycle_stage' => 'active', + 'artifact_location' => 'not_implemented' + ] + }) + end + + context 'when there are no experiments' do + let(:project_id) { another_project.id } + + it 'returns an empty list' do + expect(json_response).to include({ 'experiments' => [] }) + end + end + + describe 'Error States' do + it_behaves_like 'shared error cases' + it_behaves_like 'Requires read_api scope' + end + end + + describe 'GET /projects/:id/ml/mlflow/api/2.0/mlflow/experiments/get-by-name' do let(:experiment_name) { experiment.name } let(:route) do - "/projects/#{project_id}/ml/mflow/api/2.0/mlflow/experiments/get-by-name?experiment_name=#{experiment_name}" + "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/experiments/get-by-name?experiment_name=#{experiment_name}" end it 'returns the experiment', :aggregate_failures do @@ -203,7 +233,7 @@ RSpec.describe API::Ml::Mlflow do end context 'when has access but experiment_name is not passed' do - let(:route) { "/projects/#{project_id}/ml/mflow/api/2.0/mlflow/experiments/get-by-name" } + let(:route) { "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/experiments/get-by-name" } it_behaves_like 'Not Found - Resource Does Not Exist' end @@ -213,16 +243,16 @@ RSpec.describe API::Ml::Mlflow do end end - describe 'POST /projects/:id/ml/mflow/api/2.0/mlflow/experiments/create' do + describe 'POST /projects/:id/ml/mlflow/api/2.0/mlflow/experiments/create' do let(:route) do - "/projects/#{project_id}/ml/mflow/api/2.0/mlflow/experiments/create" + "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/experiments/create" end let(:params) { { name: 'new_experiment' } } let(:request) { post api(route), params: params, headers: headers } it 'creates the experiment', :aggregate_failures do - expect(response).to have_gitlab_http_status(:created) + expect(response).to have_gitlab_http_status(:ok) expect(json_response).to include('experiment_id' ) end @@ -244,7 +274,7 @@ RSpec.describe API::Ml::Mlflow do end context 'when project does not exist' do - let(:route) { "/projects/#{non_existing_record_id}/ml/mflow/api/2.0/mlflow/experiments/create" } + let(:route) { "/projects/#{non_existing_record_id}/ml/mlflow/api/2.0/mlflow/experiments/create" } it_behaves_like 'Not Found', '404 Project Not Found' end @@ -255,8 +285,8 @@ RSpec.describe API::Ml::Mlflow do end describe 'Runs' do - describe 'POST /projects/:id/ml/mflow/api/2.0/mlflow/runs/create' do - let(:route) { "/projects/#{project_id}/ml/mflow/api/2.0/mlflow/runs/create" } + describe 'POST /projects/:id/ml/mlflow/api/2.0/mlflow/runs/create' do + let(:route) { "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/runs/create" } let(:params) { { experiment_id: experiment.iid.to_s, start_time: Time.now.to_i } } let(:request) { post api(route), params: params, headers: headers } @@ -270,7 +300,7 @@ RSpec.describe API::Ml::Mlflow do 'lifecycle_stage' => "active" } - expect(response).to have_gitlab_http_status(:created) + expect(response).to have_gitlab_http_status(:ok) expect(response).to match_response_schema('ml/run') expect(json_response['run']).to include('info' => hash_including(**expected_properties), 'data' => { 'metrics' => [], 'params' => [] }) @@ -300,8 +330,8 @@ RSpec.describe API::Ml::Mlflow do end end - describe 'GET /projects/:id/ml/mflow/api/2.0/mlflow/runs/get' do - let(:route) { "/projects/#{project_id}/ml/mflow/api/2.0/mlflow/runs/get" } + describe 'GET /projects/:id/ml/mlflow/api/2.0/mlflow/runs/get' do + let(:route) { "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/runs/get" } let(:default_params) { { 'run_id' => candidate.iid } } it 'gets the run', :aggregate_failures do @@ -314,7 +344,7 @@ RSpec.describe API::Ml::Mlflow do 'lifecycle_stage' => "active" } - expect(response).to have_gitlab_http_status(:success) + expect(response).to have_gitlab_http_status(:ok) expect(response).to match_response_schema('ml/run') expect(json_response['run']).to include( 'info' => hash_including(**expected_properties), @@ -337,10 +367,10 @@ RSpec.describe API::Ml::Mlflow do end end - describe 'POST /projects/:id/ml/mflow/api/2.0/mlflow/runs/update' do + describe 'POST /projects/:id/ml/mlflow/api/2.0/mlflow/runs/update' do let(:default_params) { { run_id: candidate.iid.to_s, status: 'FAILED', end_time: Time.now.to_i } } let(:request) { post api(route), params: params, headers: headers } - let(:route) { "/projects/#{project_id}/ml/mflow/api/2.0/mlflow/runs/update" } + let(:route) { "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/runs/update" } it 'updates the run', :aggregate_failures do expected_properties = { @@ -353,7 +383,7 @@ RSpec.describe API::Ml::Mlflow do 'lifecycle_stage' => 'active' } - expect(response).to have_gitlab_http_status(:success) + expect(response).to have_gitlab_http_status(:ok) expect(response).to match_response_schema('ml/update_run') expect(json_response).to include('run_info' => hash_including(**expected_properties)) end @@ -377,15 +407,15 @@ RSpec.describe API::Ml::Mlflow do end end - describe 'POST /projects/:id/ml/mflow/api/2.0/mlflow/runs/log-metric' do - let(:route) { "/projects/#{project_id}/ml/mflow/api/2.0/mlflow/runs/log-metric" } + describe 'POST /projects/:id/ml/mlflow/api/2.0/mlflow/runs/log-metric' do + let(:route) { "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/runs/log-metric" } let(:default_params) { { run_id: candidate.iid.to_s, key: 'some_key', value: 10.0, timestamp: Time.now.to_i } } let(:request) { post api(route), params: params, headers: headers } it 'logs the metric', :aggregate_failures do candidate.metrics.reload - expect(response).to have_gitlab_http_status(:success) + expect(response).to have_gitlab_http_status(:ok) expect(json_response).to be_empty expect(candidate.metrics.length).to eq(3) end @@ -398,32 +428,26 @@ RSpec.describe API::Ml::Mlflow do end end - describe 'POST /projects/:id/ml/mflow/api/2.0/mlflow/runs/log-parameter' do - let(:route) { "/projects/#{project_id}/ml/mflow/api/2.0/mlflow/runs/log-parameter" } + describe 'POST /projects/:id/ml/mlflow/api/2.0/mlflow/runs/log-parameter' do + let(:route) { "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/runs/log-parameter" } let(:default_params) { { run_id: candidate.iid.to_s, key: 'some_key', value: 'value' } } let(:request) { post api(route), params: params, headers: headers } it 'logs the parameter', :aggregate_failures do candidate.params.reload - expect(response).to have_gitlab_http_status(:success) + expect(response).to have_gitlab_http_status(:ok) expect(json_response).to be_empty expect(candidate.params.length).to eq(3) end - context 'when parameter was already logged' do - let(:params) { default_params.tap { |p| p[:key] = candidate.params[0].name } } - - it 'does not log', :aggregate_failures do - candidate.params.reload + describe 'Error Cases' do + context 'when parameter was already logged' do + let(:params) { default_params.tap { |p| p[:key] = candidate.params[0].name } } - expect(response).to have_gitlab_http_status(:success) - expect(json_response).to be_empty - expect(candidate.params.length).to eq(2) + it_behaves_like 'Bad Request' end - end - describe 'Error Cases' do it_behaves_like 'shared error cases' it_behaves_like 'Requires api scope' it_behaves_like 'run_id param error cases' @@ -431,12 +455,12 @@ RSpec.describe API::Ml::Mlflow do end end - describe 'POST /projects/:id/ml/mflow/api/2.0/mlflow/runs/log-batch' do + describe 'POST /projects/:id/ml/mlflow/api/2.0/mlflow/runs/log-batch' do let(:candidate2) do create(:ml_candidates, user: experiment.user, start_time: 1234, experiment: experiment) end - let(:route) { "/projects/#{project_id}/ml/mflow/api/2.0/mlflow/runs/log-batch" } + let(:route) { "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/runs/log-batch" } let(:default_params) do { run_id: candidate2.iid.to_s, @@ -451,7 +475,7 @@ RSpec.describe API::Ml::Mlflow do let(:request) { post api(route), params: params, headers: headers } it 'logs parameters and metrics', :aggregate_failures do - expect(response).to have_gitlab_http_status(:success) + expect(response).to have_gitlab_http_status(:ok) expect(json_response).to be_empty expect(candidate2.params.size).to eq(1) expect(candidate2.metrics.size).to eq(2) @@ -465,7 +489,7 @@ RSpec.describe API::Ml::Mlflow do it 'does not log', :aggregate_failures do candidate.params.reload - expect(response).to have_gitlab_http_status(:success) + expect(response).to have_gitlab_http_status(:ok) expect(candidate2.params.size).to eq(1) end end diff --git a/spec/services/ml/experiment_tracking/candidate_repository_spec.rb b/spec/services/ml/experiment_tracking/candidate_repository_spec.rb new file mode 100644 index 00000000000..8002b2ebc86 --- /dev/null +++ b/spec/services/ml/experiment_tracking/candidate_repository_spec.rb @@ -0,0 +1,199 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ::Ml::ExperimentTracking::CandidateRepository do + let_it_be(:project) { create(:project) } + let_it_be(:user) { create(:user) } + let_it_be(:experiment) { create(:ml_experiments, user: user, project: project) } + let_it_be(:candidate) { create(:ml_candidates, user: user, experiment: experiment) } + + let(:repository) { described_class.new(project, user) } + + describe '#by_iid' do + let(:iid) { candidate.iid } + + subject { repository.by_iid(iid) } + + it { is_expected.to eq(candidate) } + + context 'when iid does not exist' do + let(:iid) { non_existing_record_iid.to_s } + + it { is_expected.to be_nil } + end + + context 'when iid belongs to a different project' do + let(:repository) { described_class.new(create(:project), user) } + + it { is_expected.to be_nil } + end + end + + describe '#create!' do + subject { repository.create!(experiment, 1234) } + + it 'creates the candidate' do + expect(subject.start_time).to eq(1234) + expect(subject.iid).not_to be_nil + expect(subject.end_time).to be_nil + end + end + + describe '#update' do + let(:end_time) { 123456 } + let(:status) { 'running' } + + subject { repository.update(candidate, status, end_time) } + + it { is_expected.to be_truthy } + + context 'when end_time is missing ' do + let(:end_time) { nil } + + it { is_expected.to be_truthy } + end + + context 'when status is wrong' do + let(:status) { 's' } + + it 'fails assigning the value' do + expect { subject }.to raise_error(ArgumentError) + end + end + + context 'when status is missing' do + let(:status) { nil } + + it { is_expected.to be_truthy } + end + end + + describe '#add_metric!' do + let(:props) { { name: 'abc', value: 1234, tracked: 12345678, step: 0 } } + let(:metrics_before) { candidate.metrics.size } + + before do + metrics_before + end + + subject { repository.add_metric!(candidate, props[:name], props[:value], props[:tracked], props[:step]) } + + it 'adds a new metric' do + expect { subject }.to change { candidate.metrics.size }.by(1) + end + + context 'when name missing' do + let(:props) { { value: 1234, tracked: 12345678, step: 0 } } + + it 'does not add metric' do + expect { subject }.to raise_error(ActiveRecord::RecordInvalid) + end + end + end + + describe '#add_param!' do + let(:props) { { name: 'abc', value: 'def' } } + + subject { repository.add_param!(candidate, props[:name], props[:value]) } + + it 'adds a new param' do + expect { subject }.to change { candidate.params.size }.by(1) + end + + context 'when name missing' do + let(:props) { { value: 1234 } } + + it 'throws RecordInvalid' do + expect { subject }.to raise_error(ActiveRecord::RecordInvalid) + end + end + + context 'when param was already added' do + it 'throws RecordInvalid' do + repository.add_param!(candidate, 'new', props[:value]) + + expect { repository.add_param!(candidate, 'new', props[:value]) }.to raise_error(ActiveRecord::RecordInvalid) + end + end + end + + describe "#add_params" do + let(:params) do + [{ key: 'model_class', value: 'LogisticRegression' }, { 'key': 'pythonEnv', value: '3.10' }] + end + + subject { repository.add_params(candidate, params) } + + it 'adds the parameters' do + expect { subject }.to change { candidate.reload.params.size }.by(2) + end + + context 'if parameter misses key' do + let(:params) do + [{ value: 'LogisticRegression' }] + end + + it 'does not throw and does not add' do + expect { subject }.to raise_error(ActiveRecord::ActiveRecordError) + end + end + + context 'if parameter misses value' do + let(:params) do + [{ key: 'pythonEnv2' }] + end + + it 'does not throw and does not add' do + expect { subject }.to raise_error(ActiveRecord::ActiveRecordError) + end + end + + context 'if parameter repeated do' do + let(:params) do + [ + { 'key': 'pythonEnv0', value: '2.7' }, + { 'key': 'pythonEnv1', value: '3.9' }, + { 'key': 'pythonEnv1', value: '3.10' } + ] + end + + before do + repository.add_param!(candidate, 'pythonEnv0', '0') + end + + it 'does not throw and adds only the first of each kind' do + expect { subject }.to change { candidate.reload.params.size }.by(1) + end + end + end + + describe "#add_metrics" do + let(:metrics) do + [ + { key: 'mae', value: 2.5, timestamp: 1552550804 }, + { key: 'rmse', value: 2.7, timestamp: 1552550804 } + ] + end + + subject { repository.add_metrics(candidate, metrics) } + + it 'adds the metrics' do + expect { subject }.to change { candidate.reload.metrics.size }.by(2) + end + + context 'when metrics have repeated keys' do + let(:metrics) do + [ + { key: 'mae', value: 2.5, timestamp: 1552550804 }, + { key: 'rmse', value: 2.7, timestamp: 1552550804 }, + { key: 'mae', value: 2.7, timestamp: 1552550805 } + ] + end + + it 'adds all of them' do + expect { subject }.to change { candidate.reload.metrics.size }.by(3) + end + end + end +end diff --git a/spec/services/ml/experiment_tracking/experiment_repository_spec.rb b/spec/services/ml/experiment_tracking/experiment_repository_spec.rb new file mode 100644 index 00000000000..80e1fa025d1 --- /dev/null +++ b/spec/services/ml/experiment_tracking/experiment_repository_spec.rb @@ -0,0 +1,85 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ::Ml::ExperimentTracking::ExperimentRepository do + let_it_be(:project) { create(:project) } + let_it_be(:user) { create(:user) } + let_it_be(:experiment) { create(:ml_experiments, user: user, project: project) } + let_it_be(:experiment2) { create(:ml_experiments, user: user, project: project) } + let_it_be(:experiment3) { create(:ml_experiments, user: user, project: project) } + let_it_be(:experiment4) { create(:ml_experiments, user: user) } + + let(:repository) { described_class.new(project, user) } + + describe '#by_iid_or_name' do + let(:iid) { experiment.iid } + let(:name) { nil } + + subject { repository.by_iid_or_name(iid: iid, name: name) } + + context 'when iid passed' do + it('fetches the experiment') { is_expected.to eq(experiment) } + + context 'and name passed' do + let(:name) { experiment2.name } + + it('ignores the name') { is_expected.to eq(experiment) } + end + + context 'and does not exist' do + let(:iid) { non_existing_record_iid } + + it { is_expected.to eq(nil) } + end + end + + context 'when iid is not passed', 'and name is passed' do + let(:iid) { nil } + + context 'when name exists' do + let(:name) { experiment2.name } + + it('fetches the experiment') { is_expected.to eq(experiment2) } + end + + context 'when name does not exist' do + let(:name) { non_existing_record_iid } + + it { is_expected.to eq(nil) } + end + end + end + + describe '#all' do + it 'fetches experiments for project' do + expect(repository.all).to match_array([experiment, experiment2, experiment3]) + end + end + + describe '#create!' do + let(:name) { 'hello' } + + subject { repository.create!(name) } + + it 'creates the candidate' do + expect { subject }.to change { repository.all.size }.by(1) + end + + context 'when name exists' do + let(:name) { experiment.name } + + it 'throws error' do + expect { subject }.to raise_error(ActiveRecord::ActiveRecordError) + end + end + + context 'when name is missing' do + let(:name) { nil } + + it 'throws error' do + expect { subject }.to raise_error(ActiveRecord::ActiveRecordError) + end + end + end +end diff --git a/spec/workers/pages/invalidate_domain_cache_worker_spec.rb b/spec/workers/pages/invalidate_domain_cache_worker_spec.rb index 7ed6cb41ce2..9658aacd093 100644 --- a/spec/workers/pages/invalidate_domain_cache_worker_spec.rb +++ b/spec/workers/pages/invalidate_domain_cache_worker_spec.rb @@ -165,6 +165,38 @@ RSpec.describe Pages::InvalidateDomainCacheWorker do { type: :namespace, id: 3 } ] + context 'when project attributes change' do + Projects::ProjectAttributesChangedEvent::PAGES_RELATED_ATTRIBUTES.each do |attribute| + it_behaves_like 'clears caches with', + event_class: Projects::ProjectAttributesChangedEvent, + event_data: { + project_id: 1, + namespace_id: 2, + root_namespace_id: 3, + attributes: [attribute] + }, + caches: [ + { type: :project, id: 1 }, + { type: :namespace, id: 3 } + ] + end + + it 'does not clear the cache when the attributes is not pages related' do + event = Projects::ProjectAttributesChangedEvent.new( + data: { + project_id: 1, + namespace_id: 2, + root_namespace_id: 3, + attributes: ['unknown'] + } + ) + + expect(described_class).not_to receive(:clear_cache) + + ::Gitlab::EventStore.publish(event) + end + end + context 'when namespace based cache keys are duplicated' do # de-dups namespace cache keys it_behaves_like 'clears caches with', diff --git a/spec/workers/run_pipeline_schedule_worker_spec.rb b/spec/workers/run_pipeline_schedule_worker_spec.rb index 846b4455bf9..10c22b736d2 100644 --- a/spec/workers/run_pipeline_schedule_worker_spec.rb +++ b/spec/workers/run_pipeline_schedule_worker_spec.rb @@ -4,7 +4,8 @@ require 'spec_helper' RSpec.describe RunPipelineScheduleWorker do describe '#perform' do - let_it_be(:project) { create(:project) } + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, namespace: group) } let_it_be(:user) { create(:user) } let_it_be(:pipeline_schedule) { create(:ci_pipeline_schedule, :nightly, project: project ) } -- cgit v1.2.3