From 2b5079efdb7c4e7d5a607d95747ddeb0b8af9678 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Mon, 15 May 2023 21:07:42 +0000 Subject: Add latest changes from gitlab-org/gitlab@master --- spec/factories/organizations.rb | 13 ++- .../components/editor/ci_editor_header_spec.js | 38 ++++++- .../components/pipeline_editor_tabs_spec.js | 2 + .../pipeline_editor/pipeline_editor_home_spec.js | 1 + .../list/__snapshots__/item_spec.js.snap | 8 +- .../pages/__snapshots__/index_spec.js.snap | 8 +- .../super_sidebar/components/sidebar_menu_spec.js | 47 +++++++-- spec/helpers/ci/pipeline_editor_helper_spec.rb | 5 + .../helpers/projects/ml/experiments_helper_spec.rb | 35 ------- .../admin/menus/admin_settings_menu_spec.rb | 3 +- .../sidebars/groups/menus/settings_menu_spec.rb | 6 ++ spec/lib/sidebars/menu_spec.rb | 2 + .../sidebars/projects/menus/settings_menu_spec.rb | 6 ++ ...20230509131736_add_default_organization_spec.rb | 20 ++++ spec/models/organization_spec.rb | 96 +++++++++++++++++- .../ml/candidate_details_presenter_spec.rb | 111 +++++++++++++++++++++ spec/support/helpers/test_env.rb | 1 + .../admin/menus/admin_menus_shared_examples.rb | 6 +- 18 files changed, 352 insertions(+), 56 deletions(-) create mode 100644 spec/migrations/20230509131736_add_default_organization_spec.rb create mode 100644 spec/presenters/ml/candidate_details_presenter_spec.rb (limited to 'spec') diff --git a/spec/factories/organizations.rb b/spec/factories/organizations.rb index a6684a8f95f..7ff0493d140 100644 --- a/spec/factories/organizations.rb +++ b/spec/factories/organizations.rb @@ -1,5 +1,16 @@ # frozen_string_literal: true FactoryBot.define do - factory :organization + factory :organization do + sequence(:name) { |n| "Organization ##{n}" } + + trait :default do + id { Organization::DEFAULT_ORGANIZATION_ID } + name { 'Default' } + initialize_with do + # Ensure we only use one default organization + Organization.find_by(id: Organization::DEFAULT_ORGANIZATION_ID) || new(**attributes) + end + end + end end diff --git a/spec/frontend/ci/pipeline_editor/components/editor/ci_editor_header_spec.js b/spec/frontend/ci/pipeline_editor/components/editor/ci_editor_header_spec.js index 2861fc35342..f1a5c4169fb 100644 --- a/spec/frontend/ci/pipeline_editor/components/editor/ci_editor_header_spec.js +++ b/spec/frontend/ci/pipeline_editor/components/editor/ci_editor_header_spec.js @@ -11,12 +11,25 @@ describe('CI Editor Header', () => { let wrapper; let trackingSpy = null; - const createComponent = ({ showDrawer = false, showJobAssistantDrawer = false } = {}) => { + const createComponent = ({ + showDrawer = false, + showJobAssistantDrawer = false, + showAiAssistantDrawer = false, + aiChatAvailable = false, + aiCiConfigGenerator = false, + } = {}) => { wrapper = extendedWrapper( shallowMount(CiEditorHeader, { + provide: { + aiChatAvailable, + glFeatures: { + aiCiConfigGenerator, + }, + }, propsData: { showDrawer, showJobAssistantDrawer, + showAiAssistantDrawer, }, }), ); @@ -24,6 +37,7 @@ describe('CI Editor Header', () => { const findLinkBtn = () => wrapper.findByTestId('template-repo-link'); const findHelpBtn = () => wrapper.findByTestId('drawer-toggle'); + const findAiAssistnantBtn = () => wrapper.findByTestId('ai-assistant-drawer-toggle'); afterEach(() => { unmockTracking(); @@ -39,7 +53,29 @@ describe('CI Editor Header', () => { label, }); }; + describe('Ai Assistant toggle button', () => { + describe('when feature is unavailable', () => { + it('should not show ai button when feature toggle is off', () => { + createComponent({ aiChatAvailable: true }); + mockTracking(undefined, wrapper.element, jest.spyOn); + expect(findAiAssistnantBtn().exists()).toBe(false); + }); + + it('should not show ai button when feature is unavailable', () => { + createComponent({ aiCiConfigGenerator: true }); + mockTracking(undefined, wrapper.element, jest.spyOn); + expect(findAiAssistnantBtn().exists()).toBe(false); + }); + }); + describe('when feature is available', () => { + it('should show ai button', () => { + createComponent({ aiCiConfigGenerator: true, aiChatAvailable: true }); + mockTracking(undefined, wrapper.element, jest.spyOn); + expect(findAiAssistnantBtn().exists()).toBe(true); + }); + }); + }); describe('link button', () => { beforeEach(() => { createComponent(); diff --git a/spec/frontend/ci/pipeline_editor/components/pipeline_editor_tabs_spec.js b/spec/frontend/ci/pipeline_editor/components/pipeline_editor_tabs_spec.js index cbdf01105c7..471b033913b 100644 --- a/spec/frontend/ci/pipeline_editor/components/pipeline_editor_tabs_spec.js +++ b/spec/frontend/ci/pipeline_editor/components/pipeline_editor_tabs_spec.js @@ -57,6 +57,7 @@ describe('Pipeline editor tabs component', () => { isNewCiConfigFile: true, showDrawer: false, showJobAssistantDrawer: false, + showAiAssistantDrawer: false, ...props, }, data() { @@ -65,6 +66,7 @@ describe('Pipeline editor tabs component', () => { }; }, provide: { + aiChatAvailable: false, ciConfigPath: '/path/to/ci-config', ciLintPath: mockCiLintPath, currentBranch: 'main', diff --git a/spec/frontend/ci/pipeline_editor/pipeline_editor_home_spec.js b/spec/frontend/ci/pipeline_editor/pipeline_editor_home_spec.js index 7ec6d4c6a01..576263d5418 100644 --- a/spec/frontend/ci/pipeline_editor/pipeline_editor_home_spec.js +++ b/spec/frontend/ci/pipeline_editor/pipeline_editor_home_spec.js @@ -41,6 +41,7 @@ describe('Pipeline editor home wrapper', () => { ...props, }, provide: { + aiChatAvailable: false, projectFullPath: '', totalBranches: 19, glFeatures: { diff --git a/spec/frontend/design_management/components/list/__snapshots__/item_spec.js.snap b/spec/frontend/design_management/components/list/__snapshots__/item_spec.js.snap index 7773950708f..9451f35ac5b 100644 --- a/spec/frontend/design_management/components/list/__snapshots__/item_spec.js.snap +++ b/spec/frontend/design_management/components/list/__snapshots__/item_spec.js.snap @@ -38,13 +38,13 @@ exports[`Design management list item component with notes renders item with mult
-
+
-
+
{ let wrapper; @@ -17,14 +25,10 @@ describe('SidebarMenu component', () => { }); }; - describe('computed', () => { - const menuItems = [ - { id: 1, title: 'No subitems' }, - { id: 2, title: 'With subitems', items: [{ id: 21, title: 'Pinned subitem' }] }, - { id: 3, title: 'Empty subitems array', items: [] }, - { id: 4, title: 'Also with subitems', items: [{ id: 41, title: 'Subitem' }] }, - ]; + const findPinnedSection = () => wrapper.findComponent(PinnedSection); + const findMainMenuSeparator = () => wrapper.findByTestId('main-menu-separator'); + describe('computed', () => { describe('supportsPins', () => { it('is true for the project sidebar', () => { createWrapper({ ...sidebarData, panel_type: 'project' }); @@ -148,4 +152,33 @@ describe('SidebarMenu component', () => { }); }); }); + + describe('Menu separators', () => { + it('should add the separator above pinned section', () => { + createWrapper({ + ...sidebarData, + current_menu_items: menuItems, + panel_type: 'project', + }); + expect(findPinnedSection().props('separated')).toBe(true); + }); + + it('should add the separator above main menu items when there is a pinned section', () => { + createWrapper({ + ...sidebarData, + current_menu_items: menuItems, + panel_type: PANELS_WITH_PINS[0], + }); + expect(findMainMenuSeparator().exists()).toBe(true); + }); + + it('should NOT add the separator above main menu items when there is no pinned section', () => { + createWrapper({ + ...sidebarData, + current_menu_items: menuItems, + panel_type: 'explore', + }); + expect(findMainMenuSeparator().exists()).toBe(false); + }); + }); }); diff --git a/spec/helpers/ci/pipeline_editor_helper_spec.rb b/spec/helpers/ci/pipeline_editor_helper_spec.rb index 1a0f2e10a20..b45882d9888 100644 --- a/spec/helpers/ci/pipeline_editor_helper_spec.rb +++ b/spec/helpers/ci/pipeline_editor_helper_spec.rb @@ -4,6 +4,7 @@ require 'spec_helper' RSpec.describe Ci::PipelineEditorHelper do let_it_be(:project) { create(:project) } + let_it_be(:user) { create(:user) } describe 'can_view_pipeline_editor?' do subject { helper.can_view_pipeline_editor?(project) } @@ -62,6 +63,10 @@ RSpec.describe Ci::PipelineEditorHelper do .to receive(:image_path) .with('illustrations/project-run-CICD-pipelines-sm.svg') .and_return('illustrations/validate.svg') + + allow(helper) + .to receive(:current_user) + .and_return(user) end subject(:pipeline_editor_data) { helper.js_pipeline_editor_data(project) } diff --git a/spec/helpers/projects/ml/experiments_helper_spec.rb b/spec/helpers/projects/ml/experiments_helper_spec.rb index e0fbc8ac488..021d518a329 100644 --- a/spec/helpers/projects/ml/experiments_helper_spec.rb +++ b/spec/helpers/projects/ml/experiments_helper_spec.rb @@ -98,41 +98,6 @@ RSpec.describe Projects::Ml::ExperimentsHelper, feature_category: :mlops do end end - describe '#show_candidate_view_model' do - let(:candidate) { candidate0 } - - subject { Gitlab::Json.parse(helper.show_candidate_view_model(candidate))['candidate'] } - - it 'generates the correct params' do - expect(subject['params']).to include( - hash_including('name' => 'param1', 'value' => 'p1'), - hash_including('name' => 'param2', 'value' => 'p2') - ) - end - - it 'generates the correct metrics' do - expect(subject['metrics']).to include( - hash_including('name' => 'metric1', 'value' => 0.1), - hash_including('name' => 'metric2', 'value' => 0.2), - hash_including('name' => 'metric3', 'value' => 0.3) - ) - end - - it 'generates the correct info' do - expected_info = { - 'iid' => candidate.iid, - 'eid' => candidate.eid, - 'path_to_artifact' => "/#{project.full_path}/-/packages/#{candidate.artifact.id}", - 'experiment_name' => candidate.experiment.name, - 'path_to_experiment' => "/#{project.full_path}/-/ml/experiments/#{experiment.iid}", - 'status' => 'running', - 'path' => "/#{project.full_path}/-/ml/candidates/#{candidate.iid}" - } - - expect(subject['info']).to include(expected_info) - end - end - describe '#experiments_as_data' do let(:experiments) { [experiment] } diff --git a/spec/lib/sidebars/admin/menus/admin_settings_menu_spec.rb b/spec/lib/sidebars/admin/menus/admin_settings_menu_spec.rb index be23dd4d25b..4c9f603e99f 100644 --- a/spec/lib/sidebars/admin/menus/admin_settings_menu_spec.rb +++ b/spec/lib/sidebars/admin/menus/admin_settings_menu_spec.rb @@ -6,7 +6,8 @@ RSpec.describe Sidebars::Admin::Menus::AdminSettingsMenu, feature_category: :nav it_behaves_like 'Admin menu', link: '/admin/application_settings/general', title: s_('Admin|Settings'), - icon: 'settings' + icon: 'settings', + separated: true it_behaves_like 'Admin menu with sub menus' end diff --git a/spec/lib/sidebars/groups/menus/settings_menu_spec.rb b/spec/lib/sidebars/groups/menus/settings_menu_spec.rb index c5246fe93dd..bc30d7628af 100644 --- a/spec/lib/sidebars/groups/menus/settings_menu_spec.rb +++ b/spec/lib/sidebars/groups/menus/settings_menu_spec.rb @@ -25,6 +25,12 @@ RSpec.describe Sidebars::Groups::Menus::SettingsMenu, :with_license do end end + describe '#separated?' do + it 'returns true' do + expect(menu.separated?).to be true + end + end + describe 'Menu items' do subject { menu.renderable_items.find { |e| e.item_id == item_id } } diff --git a/spec/lib/sidebars/menu_spec.rb b/spec/lib/sidebars/menu_spec.rb index 21aec10ec27..4f77cb3aed4 100644 --- a/spec/lib/sidebars/menu_spec.rb +++ b/spec/lib/sidebars/menu_spec.rb @@ -54,6 +54,7 @@ RSpec.describe Sidebars::Menu, feature_category: :navigation do link: "foo2", is_active: true, pill_count: nil, + separated: false, items: [ { id: 'id1', @@ -87,6 +88,7 @@ RSpec.describe Sidebars::Menu, feature_category: :navigation do link: nil, is_active: false, pill_count: 'foo', + separated: false, items: [] }) end diff --git a/spec/lib/sidebars/projects/menus/settings_menu_spec.rb b/spec/lib/sidebars/projects/menus/settings_menu_spec.rb index c7aca0fb97e..4be99892631 100644 --- a/spec/lib/sidebars/projects/menus/settings_menu_spec.rb +++ b/spec/lib/sidebars/projects/menus/settings_menu_spec.rb @@ -22,6 +22,12 @@ RSpec.describe Sidebars::Projects::Menus::SettingsMenu do end end + describe '#separated?' do + it 'returns true' do + expect(subject.separated?).to be true + end + end + describe 'Menu items' do subject { described_class.new(context).renderable_items.find { |e| e.item_id == item_id } } diff --git a/spec/migrations/20230509131736_add_default_organization_spec.rb b/spec/migrations/20230509131736_add_default_organization_spec.rb new file mode 100644 index 00000000000..539216c57ee --- /dev/null +++ b/spec/migrations/20230509131736_add_default_organization_spec.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +require 'spec_helper' + +require_migration! + +RSpec.describe AddDefaultOrganization, feature_category: :cell do + let(:organization) { table(:organizations) } + + it "correctly migrates up and down" do + reversible_migration do |migration| + migration.before -> { + expect(organization.where(id: 1, name: 'Default')).to be_empty + } + migration.after -> { + expect(organization.where(id: 1, name: 'Default')).not_to be_empty + } + end + end +end diff --git a/spec/models/organization_spec.rb b/spec/models/organization_spec.rb index 9966a7132ce..e1aac88e640 100644 --- a/spec/models/organization_spec.rb +++ b/spec/models/organization_spec.rb @@ -2,9 +2,97 @@ require 'spec_helper' -# rubocop: disable Lint/EmptyBlock -# rubocop: disable RSpec/EmptyExampleGroup RSpec.describe Organization, type: :model, feature_category: :cell do + let_it_be(:organization) { create(:organization) } + let_it_be(:default_organization) { create(:organization, :default) } + + describe 'validations' do + subject { create(:organization) } + + it { is_expected.to validate_presence_of(:name) } + it { is_expected.to validate_uniqueness_of(:name).case_insensitive } + it { is_expected.to validate_length_of(:name).is_at_most(255) } + end + + context 'when using scopes' do + describe '.without_default' do + it 'excludes default organization' do + expect(described_class.without_default).not_to include(default_organization) + end + + it 'includes other organizations organization' do + expect(described_class.without_default).to include(organization) + end + end + end + + describe '#id' do + context 'when organization is default' do + it 'has id 1' do + expect(default_organization.id).to eq(1) + end + end + + context 'when organization is not default' do + it 'does not have id 1' do + expect(organization.id).not_to eq(1) + end + end + end + + describe '#destroy!' do + context 'when trying to delete the default organization' do + it 'raises an error' do + expect do + default_organization.destroy! + end.to raise_error(ActiveRecord::RecordNotDestroyed, _('Cannot delete the default organization')) + end + end + + context 'when trying to delete a non-default organization' do + let(:to_be_removed) { create(:organization) } + + it 'does not raise error' do + expect { to_be_removed.destroy! }.not_to raise_error + end + end + end + + describe '#destroy' do + context 'when trying to delete the default organization' do + it 'returns false' do + expect(default_organization.destroy).to eq(false) + end + end + + context 'when trying to delete a non-default organization' do + let(:to_be_removed) { create(:organization) } + + it 'returns true' do + expect(to_be_removed.destroy).to eq(to_be_removed) + end + end + end + + describe '#default?' do + context 'when organization is default' do + it 'returns true' do + expect(default_organization.default?).to eq(true) + end + end + + context 'when organization is not default' do + it 'returns false' do + expect(organization.default?).to eq(false) + end + end + end + + describe '#name' do + context 'when organization is default' do + it 'returns Default' do + expect(default_organization.name).to eq('Default') + end + end + end end -# rubocop: enable RSpec/EmptyExampleGroup -# rubocop: enable Lint/EmptyBlock diff --git a/spec/presenters/ml/candidate_details_presenter_spec.rb b/spec/presenters/ml/candidate_details_presenter_spec.rb new file mode 100644 index 00000000000..d83ffbc7129 --- /dev/null +++ b/spec/presenters/ml/candidate_details_presenter_spec.rb @@ -0,0 +1,111 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ::Ml::CandidateDetailsPresenter, feature_category: :mlops do + let_it_be(:project) { create(:project, :private) } # rubocop:disable RSpec/FactoryBot/AvoidCreate + let_it_be(:user) { project.creator } + let_it_be(:experiment) { create(:ml_experiments, user: user, project: project) } # rubocop:disable RSpec/FactoryBot/AvoidCreate + let_it_be(:candidate) do + create(:ml_candidates, :with_artifact, experiment: experiment, user: user, project: project) # rubocop:disable RSpec/FactoryBot/AvoidCreate + end + + let_it_be(:metrics) do + [ + build_stubbed(:ml_candidate_metrics, name: 'metric1', value: 0.1, candidate: candidate), + build_stubbed(:ml_candidate_metrics, name: 'metric2', value: 0.2, candidate: candidate), + build_stubbed(:ml_candidate_metrics, name: 'metric3', value: 0.3, candidate: candidate) + ] + end + + let_it_be(:params) do + [ + build_stubbed(:ml_candidate_params, name: 'param1', value: 'p1', candidate: candidate), + build_stubbed(:ml_candidate_params, name: 'param2', value: 'p2', candidate: candidate) + ] + end + + subject { Gitlab::Json.parse(described_class.new(candidate).present)['candidate'] } + + before do + allow(candidate).to receive(:latest_metrics).and_return(metrics) + allow(candidate).to receive(:params).and_return(params) + end + + describe '#execute' do + context 'when candidate has metrics, params and artifacts' do + it 'generates the correct params' do + expect(subject['params']).to include( + hash_including('name' => 'param1', 'value' => 'p1'), + hash_including('name' => 'param2', 'value' => 'p2') + ) + end + + it 'generates the correct metrics' do + expect(subject['metrics']).to include( + hash_including('name' => 'metric1', 'value' => 0.1), + hash_including('name' => 'metric2', 'value' => 0.2), + hash_including('name' => 'metric3', 'value' => 0.3) + ) + end + + it 'generates the correct info' do + expected_info = { + 'iid' => candidate.iid, + 'eid' => candidate.eid, + 'path_to_artifact' => "/#{project.full_path}/-/packages/#{candidate.artifact.id}", + 'experiment_name' => candidate.experiment.name, + 'path_to_experiment' => "/#{project.full_path}/-/ml/experiments/#{experiment.iid}", + 'status' => 'running', + 'path' => "/#{project.full_path}/-/ml/candidates/#{candidate.iid}" + } + + expect(subject['info']).to include(expected_info) + end + end + + context 'when candidate has job' do + let_it_be(:pipeline) { build_stubbed(:ci_pipeline, project: project, user: user) } + let_it_be(:build) { candidate.ci_build = build_stubbed(:ci_build, pipeline: pipeline, user: user) } + + it 'generates the correct ci' do + expected_info = { + 'path' => "/#{project.full_path}/-/jobs/#{build.id}", + 'name' => 'test', + 'user' => { + 'path' => "/#{pipeline.user.username}", + 'username' => pipeline.user.username + } + } + + expect(subject.dig('info', 'ci_job')).to include(expected_info) + end + + context 'when build user is nil' do + it 'does not include build user info' do + expected_info = { + 'path' => "/#{project.full_path}/-/jobs/#{build.id}", + 'name' => 'test' + } + + allow(build).to receive(:user).and_return(nil) + + expect(subject.dig('info', 'ci_job')).to eq(expected_info) + end + end + + context 'and job is from MR' do + let_it_be(:mr) { pipeline.merge_request = build_stubbed(:merge_request, source_project: project) } + + it 'generates the correct ci' do + expected_info = { + 'path' => "/#{project.full_path}/-/merge_requests/#{mr.iid}", + 'title' => mr.title + } + + expect(subject.dig('info', 'ci_job', 'merge_request')).to include(expected_info) + end + end + end + end +end diff --git a/spec/support/helpers/test_env.rb b/spec/support/helpers/test_env.rb index a53e1e1002c..ceb567e54c4 100644 --- a/spec/support/helpers/test_env.rb +++ b/spec/support/helpers/test_env.rb @@ -372,6 +372,7 @@ module TestEnv def seed_db Gitlab::DatabaseImporters::WorkItems::BaseTypeImporter.upsert_types Gitlab::DatabaseImporters::WorkItems::HierarchyRestrictionsImporter.upsert_restrictions + FactoryBot.create(:organization, :default) end private diff --git a/spec/support/shared_examples/lib/sidebars/admin/menus/admin_menus_shared_examples.rb b/spec/support/shared_examples/lib/sidebars/admin/menus/admin_menus_shared_examples.rb index a9fd66528bd..f913c6b8a9e 100644 --- a/spec/support/shared_examples/lib/sidebars/admin/menus/admin_menus_shared_examples.rb +++ b/spec/support/shared_examples/lib/sidebars/admin/menus/admin_menus_shared_examples.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -RSpec.shared_examples 'Admin menu' do |link:, title:, icon:| +RSpec.shared_examples 'Admin menu' do |link:, title:, icon:, separated: false| let_it_be(:user) { build(:user, :admin) } before do @@ -23,6 +23,10 @@ RSpec.shared_examples 'Admin menu' do |link:, title:, icon:| expect(subject.sprite_icon).to be icon end + it 'renders the separator if needed' do + expect(subject.separated?).to be separated + end + describe '#render?' do context 'when user is admin' do it 'renders' do -- cgit v1.2.1