diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2023-12-27 00:14:29 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2023-12-27 00:14:29 +0300 |
commit | d6b0aec9b8801487055e096cfe96823424e6f745 (patch) | |
tree | 4d445695ae43a6b34c4d2d76b54f2b35fabb5ab0 /spec | |
parent | a2f6474ee7cf1f5a48aa4c0176d3cc560d58f17d (diff) |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'spec')
-rw-r--r-- | spec/factories/ml/model_version_metadata.rb | 11 | ||||
-rw-r--r-- | spec/fixtures/api/schemas/ml/get_latest_versions.json | 4 | ||||
-rw-r--r-- | spec/fixtures/api/schemas/ml/get_model_version.json | 17 | ||||
-rw-r--r-- | spec/models/ml/candidate_metric_spec.rb | 2 | ||||
-rw-r--r-- | spec/models/ml/candidate_param_spec.rb | 2 | ||||
-rw-r--r-- | spec/models/ml/model_metadata_spec.rb | 2 | ||||
-rw-r--r-- | spec/models/ml/model_version_metadata_spec.rb | 29 | ||||
-rw-r--r-- | spec/models/ml/model_version_spec.rb | 29 | ||||
-rw-r--r-- | spec/requests/api/ml/mlflow/model_versions_spec.rb | 24 | ||||
-rw-r--r-- | spec/services/ml/create_model_version_service_spec.rb | 31 |
10 files changed, 145 insertions, 6 deletions
diff --git a/spec/factories/ml/model_version_metadata.rb b/spec/factories/ml/model_version_metadata.rb new file mode 100644 index 00000000000..de13465e5bf --- /dev/null +++ b/spec/factories/ml/model_version_metadata.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :ml_model_version_metadata, class: '::Ml::ModelVersionMetadata' do + association :model_version, factory: :ml_model_versions + association :project, factory: :project + + sequence(:name) { |n| "metadata_#{n}" } + sequence(:value) { |n| "value#{n}" } + end +end diff --git a/spec/fixtures/api/schemas/ml/get_latest_versions.json b/spec/fixtures/api/schemas/ml/get_latest_versions.json index cb2308fa637..3bf9a43a795 100644 --- a/spec/fixtures/api/schemas/ml/get_latest_versions.json +++ b/spec/fixtures/api/schemas/ml/get_latest_versions.json @@ -21,7 +21,7 @@ "run_id", "status", "status_message", - "metadata", + "tags", "run_link", "aliases" ], @@ -59,7 +59,7 @@ "status_message": { "type": "string" }, - "metadata": { + "tags": { "type": "array", "items": { } diff --git a/spec/fixtures/api/schemas/ml/get_model_version.json b/spec/fixtures/api/schemas/ml/get_model_version.json index 214c993ed73..89779428edf 100644 --- a/spec/fixtures/api/schemas/ml/get_model_version.json +++ b/spec/fixtures/api/schemas/ml/get_model_version.json @@ -18,7 +18,7 @@ "run_id", "status", "status_message", - "metadata", + "tags", "run_link", "aliases" ], @@ -56,9 +56,22 @@ "status_message": { "type": "string" }, - "metadata": { + "tags": { "type": "array", "items": { + "type": "object", + "required": [ + "key", + "value" + ], + "properties": { + "key": { + "type": "string" + }, + "value": { + "type": "string" + } + } } }, "run_link": { diff --git a/spec/models/ml/candidate_metric_spec.rb b/spec/models/ml/candidate_metric_spec.rb index 9f9a6e8e3ba..9ceaa83a6fa 100644 --- a/spec/models/ml/candidate_metric_spec.rb +++ b/spec/models/ml/candidate_metric_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ml::CandidateMetric do +RSpec.describe Ml::CandidateMetric, feature_category: :mlops do describe 'associations' do it { is_expected.to belong_to(:candidate) } end diff --git a/spec/models/ml/candidate_param_spec.rb b/spec/models/ml/candidate_param_spec.rb index ff38e471219..89232b10855 100644 --- a/spec/models/ml/candidate_param_spec.rb +++ b/spec/models/ml/candidate_param_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ml::CandidateParam do +RSpec.describe Ml::CandidateParam, feature_category: :mlops do describe 'associations' do it { is_expected.to belong_to(:candidate) } end diff --git a/spec/models/ml/model_metadata_spec.rb b/spec/models/ml/model_metadata_spec.rb index f06c7a2ce50..0afc7bb7a2e 100644 --- a/spec/models/ml/model_metadata_spec.rb +++ b/spec/models/ml/model_metadata_spec.rb @@ -5,6 +5,8 @@ require 'spec_helper' RSpec.describe Ml::ModelMetadata, feature_category: :mlops do describe 'associations' do it { is_expected.to belong_to(:model).required } + it { is_expected.to validate_length_of(:name).is_at_most(255) } + it { is_expected.to validate_length_of(:value).is_at_most(5000) } end describe 'validations' do diff --git a/spec/models/ml/model_version_metadata_spec.rb b/spec/models/ml/model_version_metadata_spec.rb new file mode 100644 index 00000000000..7c8ffb9b0d7 --- /dev/null +++ b/spec/models/ml/model_version_metadata_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ml::ModelVersionMetadata, feature_category: :mlops do + describe 'associations' do + it { is_expected.to belong_to(:model_version).required } + it { is_expected.to belong_to(:project).required } + it { is_expected.to validate_length_of(:name).is_at_most(255) } + it { is_expected.to validate_length_of(:value).is_at_most(5000) } + end + + describe 'validations' do + let_it_be(:metadata) { create(:ml_model_version_metadata, name: 'some_metadata') } + let_it_be(:model_version) { metadata.model_version } + + it 'is unique within the model version' do + expect do + model_version.metadata.create!(name: 'some_metadata', value: 'blah') + end.to raise_error.with_message(/Name 'some_metadata' already taken/) + end + + it 'a model version is required' do + expect do + described_class.create!(name: 'some_metadata', value: 'blah') + end.to raise_error.with_message(/Model version must exist/) + end + end +end diff --git a/spec/models/ml/model_version_spec.rb b/spec/models/ml/model_version_spec.rb index 95d4a545f52..9db9f7e34ab 100644 --- a/spec/models/ml/model_version_spec.rb +++ b/spec/models/ml/model_version_spec.rb @@ -19,6 +19,7 @@ RSpec.describe Ml::ModelVersion, feature_category: :mlops do it { is_expected.to belong_to(:model) } it { is_expected.to belong_to(:package).class_name('Packages::MlModel::Package') } it { is_expected.to have_one(:candidate).class_name('Ml::Candidate') } + it { is_expected.to have_many(:metadata) } end describe 'validation' do @@ -99,6 +100,34 @@ RSpec.describe Ml::ModelVersion, feature_category: :mlops do end end + describe '#add_metadata' do + it 'accepts an array of metadata and persists it to the model version' do + input = [ + { project_id: base_project.id, key: 'tag1', value: 'value1' }, + { project_id: base_project.id, key: 'tag2', value: 'value2' } + ] + + expect { model_version1.add_metadata(input) }.to change { model_version1.metadata.count }.by(2) + end + + it 'raises an error when duplicate key names are supplied' do + input = [ + { project_id: base_project.id, key: 'tag1', value: 'value1' }, + { project_id: base_project.id, key: 'tag1', value: 'value2' } + ] + + expect { model_version1.add_metadata(input) }.to raise_error(ActiveRecord::RecordInvalid) + end + + it 'raises an error when validation fails' do + input = [ + { project_id: base_project.id, key: nil, value: 'value1' } + ] + + expect { model_version1.add_metadata(input) }.to raise_error(ActiveRecord::RecordInvalid) + end + end + describe '#find_or_create!' do let_it_be(:existing_model_version) { create(:ml_model_versions, model: model1, version: '1.0.0') } diff --git a/spec/requests/api/ml/mlflow/model_versions_spec.rb b/spec/requests/api/ml/mlflow/model_versions_spec.rb index e62bccf1507..16990642764 100644 --- a/spec/requests/api/ml/mlflow/model_versions_spec.rb +++ b/spec/requests/api/ml/mlflow/model_versions_spec.rb @@ -164,6 +164,30 @@ RSpec.describe API::Ml::Mlflow::ModelVersions, feature_category: :mlops do it_behaves_like 'MLflow|Not Found - Resource Does Not Exist' end + + # TODO: Ensure consisted error responses https://gitlab.com/gitlab-org/gitlab/-/issues/429731 + context 'when a duplicate tag name is supplied' do + let(:params) do + { name: model_name, tags: [{ key: 'key1', value: 'value1' }, { key: 'key1', value: 'value2' }] } + end + + it "returns a validation error", :aggregate_failures do + expect(json_response).to include({ 'error_code' => 'INVALID_PARAMETER_VALUE' }) + expect(model.metadata.count).to be 0 + end + end + + # TODO: Ensure consisted error responses https://gitlab.com/gitlab-org/gitlab/-/issues/429731 + context 'when an empty tag name is supplied' do + let(:params) do + { name: model_name, tags: [{ key: '', value: 'value1' }, { key: 'key1', value: 'value2' }] } + end + + it "returns a validation error", :aggregate_failures do + expect(json_response).to include({ 'error_code' => 'INVALID_PARAMETER_VALUE' }) + expect(model.metadata.count).to be 0 + end + end end it_behaves_like 'MLflow|an authenticated resource' diff --git a/spec/services/ml/create_model_version_service_spec.rb b/spec/services/ml/create_model_version_service_spec.rb index b3aead4a92c..f9025fe8edb 100644 --- a/spec/services/ml/create_model_version_service_spec.rb +++ b/spec/services/ml/create_model_version_service_spec.rb @@ -75,5 +75,36 @@ RSpec.describe ::Ml::CreateModelVersionService, feature_category: :mlops do expect(model.reload.latest_version.package.name).to eq(model.name) expect(model.latest_version.package.version).to eq(model.latest_version.version) end + + context 'when metadata are supplied, add them as metadata' do + let(:metadata) { [{ key: 'key1', value: 'value1' }, { key: 'key2', value: 'value2' }] } + let(:params) { { metadata: metadata } } + + it 'creates metadata records', :aggregate_failures do + expect { service }.to change { Ml::ModelVersion.count }.by(1) + + expect(service.metadata.count).to be 2 + end + end + + # TODO: Ensure consisted error responses https://gitlab.com/gitlab-org/gitlab/-/issues/429731 + context 'for metadata with duplicate keys, it does not create duplicate records' do + let(:metadata) { [{ key: 'key1', value: 'value1' }, { key: 'key1', value: 'value2' }] } + let(:params) { { metadata: metadata } } + + it 'raises an error', :aggregate_failures do + expect { service }.to raise_error(ActiveRecord::RecordInvalid) + end + end + + # # TODO: Ensure consisted error responses https://gitlab.com/gitlab-org/gitlab/-/issues/429731 + context 'for metadata with invalid keys, it does not create invalid records' do + let(:metadata) { [{ key: 'key1', value: 'value1' }, { key: '', value: 'value2' }] } + let(:params) { { metadata: metadata } } + + it 'raises an error', :aggregate_failures do + expect { service }.to raise_error(ActiveRecord::RecordInvalid) + end + end end end |