diff options
Diffstat (limited to 'spec')
46 files changed, 648 insertions, 690 deletions
diff --git a/spec/controllers/admin/sessions_controller_spec.rb b/spec/controllers/admin/sessions_controller_spec.rb index bd0bb0bd81f..be996aee1d2 100644 --- a/spec/controllers/admin/sessions_controller_spec.rb +++ b/spec/controllers/admin/sessions_controller_spec.rb @@ -122,7 +122,7 @@ describe Admin::SessionsController, :do_not_mock_admin_mode do describe '#destroy' do context 'for regular users' do it 'shows error page' do - get :destroy + post :destroy expect(response).to have_gitlab_http_status(404) expect(controller.current_user_mode.admin_mode?).to be(false) @@ -139,7 +139,7 @@ describe Admin::SessionsController, :do_not_mock_admin_mode do post :create, params: { password: user.password } expect(controller.current_user_mode.admin_mode?).to be(true) - get :destroy + post :destroy expect(response).to have_gitlab_http_status(:found) expect(response).to redirect_to(root_path) diff --git a/spec/features/merge_request/maintainer_edits_fork_spec.rb b/spec/features/merge_request/maintainer_edits_fork_spec.rb index 4e161d530d3..4f2c5fc73d8 100644 --- a/spec/features/merge_request/maintainer_edits_fork_spec.rb +++ b/spec/features/merge_request/maintainer_edits_fork_spec.rb @@ -32,8 +32,6 @@ describe 'a maintainer edits files on a source-branch of an MR from a fork', :js wait_for_requests end - it_behaves_like 'rendering a single diff version' - it 'mentions commits will go to the source branch' do expect(page).to have_content('Your changes can be committed to fix because a merge request is open.') end diff --git a/spec/features/merge_request/user_comments_on_diff_spec.rb b/spec/features/merge_request/user_comments_on_diff_spec.rb index 6a23b6cdf60..19b8a7f74b7 100644 --- a/spec/features/merge_request/user_comments_on_diff_spec.rb +++ b/spec/features/merge_request/user_comments_on_diff_spec.rb @@ -13,15 +13,12 @@ describe 'User comments on a diff', :js do let(:user) { create(:user) } before do - stub_feature_flags(single_mr_diff_view: false) project.add_maintainer(user) sign_in(user) visit(diffs_project_merge_request_path(project, merge_request)) end - it_behaves_like 'rendering a single diff version' - context 'when viewing comments' do context 'when toggling inline comments' do context 'in a single file' do diff --git a/spec/features/merge_request/user_creates_image_diff_notes_spec.rb b/spec/features/merge_request/user_creates_image_diff_notes_spec.rb index e6634a8ff39..e0724a04ea3 100644 --- a/spec/features/merge_request/user_creates_image_diff_notes_spec.rb +++ b/spec/features/merge_request/user_creates_image_diff_notes_spec.rb @@ -9,7 +9,6 @@ describe 'Merge request > User creates image diff notes', :js do let(:user) { project.creator } before do - stub_feature_flags(single_mr_diff_view: false) sign_in(user) # Stub helper to return any blob file as image from public app folder. @@ -18,8 +17,6 @@ describe 'Merge request > User creates image diff notes', :js do allow_any_instance_of(DiffHelper).to receive(:diff_file_old_blob_raw_url).and_return('/favicon.png') end - it_behaves_like 'rendering a single diff version' - context 'create commit diff notes' do commit_id = '2f63565e7aac07bcdadb654e253078b727143ec4' diff --git a/spec/features/merge_request/user_expands_diff_spec.rb b/spec/features/merge_request/user_expands_diff_spec.rb index 9b040271468..9bce5264817 100644 --- a/spec/features/merge_request/user_expands_diff_spec.rb +++ b/spec/features/merge_request/user_expands_diff_spec.rb @@ -7,7 +7,6 @@ describe 'User expands diff', :js do let(:merge_request) { create(:merge_request, source_branch: 'expand-collapse-files', source_project: project, target_project: project) } before do - stub_feature_flags(single_mr_diff_view: false) stub_feature_flags(diffs_batch_load: false) allow(Gitlab::Git::Diff).to receive(:size_limit).and_return(100.kilobytes) @@ -18,8 +17,6 @@ describe 'User expands diff', :js do wait_for_requests end - it_behaves_like 'rendering a single diff version' - it 'allows user to expand diff' do page.within find('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9"]') do click_link 'Click to expand it.' diff --git a/spec/features/merge_request/user_posts_diff_notes_spec.rb b/spec/features/merge_request/user_posts_diff_notes_spec.rb index 6328c0a5133..8b16760606c 100644 --- a/spec/features/merge_request/user_posts_diff_notes_spec.rb +++ b/spec/features/merge_request/user_posts_diff_notes_spec.rb @@ -14,15 +14,12 @@ describe 'Merge request > User posts diff notes', :js do let(:test_note_comment) { 'this is a test note!' } before do - stub_feature_flags(single_mr_diff_view: false) set_cookie('sidebar_collapsed', 'true') project.add_developer(user) sign_in(user) end - it_behaves_like 'rendering a single diff version' - context 'when hovering over a parallel view diff file' do before do visit diffs_project_merge_request_path(project, merge_request, view: 'parallel') diff --git a/spec/features/merge_request/user_posts_notes_spec.rb b/spec/features/merge_request/user_posts_notes_spec.rb index c0655581b18..f24e7090605 100644 --- a/spec/features/merge_request/user_posts_notes_spec.rb +++ b/spec/features/merge_request/user_posts_notes_spec.rb @@ -165,9 +165,9 @@ describe 'Merge request > User posts notes', :js do find('.js-note-edit').click page.within('.current-note-edit-form') do - expect(find('#note_note').value).to eq('This is the new content') + expect(find('#note_note').value).to include('This is the new content') first('.js-md').click - expect(find('#note_note').value).to eq('This is the new content****') + expect(find('#note_note').value).to include('This is the new content****') end end diff --git a/spec/features/merge_request/user_resolves_conflicts_spec.rb b/spec/features/merge_request/user_resolves_conflicts_spec.rb index f0949fefa3b..ce85e81868d 100644 --- a/spec/features/merge_request/user_resolves_conflicts_spec.rb +++ b/spec/features/merge_request/user_resolves_conflicts_spec.rb @@ -9,7 +9,6 @@ describe 'Merge request > User resolves conflicts', :js do before do # In order to have the diffs collapsed, we need to disable the increase feature stub_feature_flags(gitlab_git_diff_size_limit_increase: false) - stub_feature_flags(single_mr_diff_view: false) end def create_merge_request(source_branch) @@ -18,8 +17,6 @@ describe 'Merge request > User resolves conflicts', :js do end end - it_behaves_like 'rendering a single diff version' - shared_examples 'conflicts are resolved in Interactive mode' do it 'conflicts are resolved in Interactive mode' do within find('.files-wrapper .diff-file', text: 'files/ruby/popen.rb') do diff --git a/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb b/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb index 9cbea8a8466..eb86b1e33af 100644 --- a/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb +++ b/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb @@ -20,12 +20,9 @@ describe 'Merge request > User resolves diff notes and threads', :js do end before do - stub_feature_flags(single_mr_diff_view: false) stub_feature_flags(diffs_batch_load: false) end - it_behaves_like 'rendering a single diff version' - context 'no threads' do before do project.add_maintainer(user) diff --git a/spec/features/merge_request/user_sees_avatar_on_diff_notes_spec.rb b/spec/features/merge_request/user_sees_avatar_on_diff_notes_spec.rb index 70afe056c64..3e77b9e75d6 100644 --- a/spec/features/merge_request/user_sees_avatar_on_diff_notes_spec.rb +++ b/spec/features/merge_request/user_sees_avatar_on_diff_notes_spec.rb @@ -21,7 +21,6 @@ describe 'Merge request > User sees avatars on diff notes', :js do let!(:note) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: position) } before do - stub_feature_flags(single_mr_diff_view: false) stub_feature_flags(diffs_batch_load: false) project.add_maintainer(user) sign_in user @@ -29,8 +28,6 @@ describe 'Merge request > User sees avatars on diff notes', :js do set_cookie('sidebar_collapsed', 'true') end - it_behaves_like 'rendering a single diff version' - context 'discussion tab' do before do visit project_merge_request_path(project, merge_request) diff --git a/spec/features/merge_request/user_sees_diff_spec.rb b/spec/features/merge_request/user_sees_diff_spec.rb index de142344c26..2d91d09a486 100644 --- a/spec/features/merge_request/user_sees_diff_spec.rb +++ b/spec/features/merge_request/user_sees_diff_spec.rb @@ -10,12 +10,9 @@ describe 'Merge request > User sees diff', :js do let(:merge_request) { create(:merge_request, source_project: project) } before do - stub_feature_flags(single_mr_diff_view: false) stub_feature_flags(diffs_batch_load: false) end - it_behaves_like 'rendering a single diff version' - context 'when linking to note' do describe 'with unresolved note' do let(:note) { create :diff_note_on_merge_request, project: project, noteable: merge_request } diff --git a/spec/features/merge_request/user_sees_mr_with_deleted_source_branch_spec.rb b/spec/features/merge_request/user_sees_mr_with_deleted_source_branch_spec.rb index e28d2ca5536..59e5f5c847d 100644 --- a/spec/features/merge_request/user_sees_mr_with_deleted_source_branch_spec.rb +++ b/spec/features/merge_request/user_sees_mr_with_deleted_source_branch_spec.rb @@ -11,14 +11,11 @@ describe 'Merge request > User sees MR with deleted source branch', :js do let(:user) { project.creator } before do - stub_feature_flags(single_mr_diff_view: false) merge_request.update!(source_branch: 'this-branch-does-not-exist') sign_in(user) visit project_merge_request_path(project, merge_request) end - it_behaves_like 'rendering a single diff version' - it 'shows a message about missing source branch' do expect(page).to have_content('Source branch does not exist.') end diff --git a/spec/features/merge_request/user_sees_versions_spec.rb b/spec/features/merge_request/user_sees_versions_spec.rb index b3aef601c7b..cab86f3fd94 100644 --- a/spec/features/merge_request/user_sees_versions_spec.rb +++ b/spec/features/merge_request/user_sees_versions_spec.rb @@ -16,7 +16,6 @@ describe 'Merge request > User sees versions', :js do let!(:params) { {} } before do - stub_feature_flags(single_mr_diff_view: false) stub_feature_flags(diffs_batch_load: false) project.add_maintainer(user) @@ -24,8 +23,6 @@ describe 'Merge request > User sees versions', :js do visit diffs_project_merge_request_path(project, merge_request, params) end - it_behaves_like 'rendering a single diff version' - shared_examples 'allows commenting' do |file_id:, line_code:, comment:| it do diff_file_selector = ".diff-file[id='#{file_id}']" diff --git a/spec/features/merge_request/user_suggests_changes_on_diff_spec.rb b/spec/features/merge_request/user_suggests_changes_on_diff_spec.rb index 085fe38b47a..95cb0a2dee3 100644 --- a/spec/features/merge_request/user_suggests_changes_on_diff_spec.rb +++ b/spec/features/merge_request/user_suggests_changes_on_diff_spec.rb @@ -25,15 +25,12 @@ describe 'User comments on a diff', :js do let(:user) { create(:user) } before do - stub_feature_flags(single_mr_diff_view: false) project.add_maintainer(user) sign_in(user) visit(diffs_project_merge_request_path(project, merge_request)) end - it_behaves_like 'rendering a single diff version' - context 'single suggestion note' do it 'hides suggestion popover' do click_diff_line(find("[id='#{sample_compare.changes[1][:line_code]}']")) diff --git a/spec/features/merge_request/user_toggles_whitespace_changes_spec.rb b/spec/features/merge_request/user_toggles_whitespace_changes_spec.rb index 5e59bc87e68..4db067a4e41 100644 --- a/spec/features/merge_request/user_toggles_whitespace_changes_spec.rb +++ b/spec/features/merge_request/user_toggles_whitespace_changes_spec.rb @@ -8,7 +8,6 @@ describe 'Merge request > User toggles whitespace changes', :js do let(:user) { project.creator } before do - stub_feature_flags(single_mr_diff_view: false) project.add_maintainer(user) sign_in(user) visit diffs_project_merge_request_path(project, merge_request) @@ -16,8 +15,6 @@ describe 'Merge request > User toggles whitespace changes', :js do find('.js-show-diff-settings').click end - it_behaves_like 'rendering a single diff version' - it 'has a button to toggle whitespace changes' do expect(page).to have_content 'Show whitespace changes' end diff --git a/spec/features/merge_request/user_views_diffs_spec.rb b/spec/features/merge_request/user_views_diffs_spec.rb index 313f438e23b..e0e4058dd47 100644 --- a/spec/features/merge_request/user_views_diffs_spec.rb +++ b/spec/features/merge_request/user_views_diffs_spec.rb @@ -9,7 +9,6 @@ describe 'User views diffs', :js do let(:project) { create(:project, :public, :repository) } before do - stub_feature_flags(single_mr_diff_view: false) stub_feature_flags(diffs_batch_load: false) visit(diffs_project_merge_request_path(project, merge_request)) @@ -18,8 +17,6 @@ describe 'User views diffs', :js do find('.js-toggle-tree-list').click end - it_behaves_like 'rendering a single diff version' - shared_examples 'unfold diffs' do it 'unfolds diffs upwards' do first('.js-unfold').click diff --git a/spec/features/projects/blobs/edit_spec.rb b/spec/features/projects/blobs/edit_spec.rb index 0a5bc64b429..a1d6a8896c7 100644 --- a/spec/features/projects/blobs/edit_spec.rb +++ b/spec/features/projects/blobs/edit_spec.rb @@ -12,11 +12,9 @@ describe 'Editing file blob', :js do let(:readme_file_path) { 'README.md' } before do - stub_feature_flags(web_ide_default: false, single_mr_diff_view: false) + stub_feature_flags(web_ide_default: false) end - it_behaves_like 'rendering a single diff version' - context 'as a developer' do let(:user) { create(:user) } let(:role) { :developer } diff --git a/spec/features/projects/view_on_env_spec.rb b/spec/features/projects/view_on_env_spec.rb index c2d4cefad12..8b25565c08a 100644 --- a/spec/features/projects/view_on_env_spec.rb +++ b/spec/features/projects/view_on_env_spec.rb @@ -9,14 +9,11 @@ describe 'View on environment', :js do let(:user) { project.creator } before do - stub_feature_flags(single_mr_diff_view: false) stub_feature_flags(diffs_batch_load: false) project.add_maintainer(user) end - it_behaves_like 'rendering a single diff version' - context 'when the branch has a route map' do let(:route_map) do <<-MAP.strip_heredoc diff --git a/spec/features/triggers_spec.rb b/spec/features/triggers_spec.rb index 19cd21e4161..af406961bbc 100644 --- a/spec/features/triggers_spec.rb +++ b/spec/features/triggers_spec.rb @@ -65,22 +65,6 @@ describe 'Triggers', :js do expect(page.find('.triggers-list')).to have_content new_trigger_title expect(page.find('.triggers-list .trigger-owner')).to have_content user.name end - - it 'edit "legacy" trigger and save' do - # Create new trigger without owner association, i.e. Legacy trigger - create(:ci_trigger, owner: user, project: @project).update_attribute(:owner, nil) - visit project_settings_ci_cd_path(@project) - - # See if the trigger can be edited and description is blank - find('a[title="Edit"]').send_keys(:return) - expect(page.find('#trigger_description').value).to have_content '' - - # See if trigger can be updated with description and saved successfully - fill_in 'trigger_description', with: new_trigger_title - click_button 'Save trigger' - expect(page.find('.flash-notice')).to have_content 'Trigger was successfully updated.' - expect(page.find('.triggers-list')).to have_content new_trigger_title - end end describe 'trigger "Revoke" workflow' do @@ -106,43 +90,18 @@ describe 'Triggers', :js do end describe 'show triggers workflow' do - before do - stub_feature_flags(use_legacy_pipeline_triggers: false) - end - it 'contains trigger description placeholder' do expect(page.find('#trigger_description')['placeholder']).to eq 'Trigger description' end - it 'show "invalid" badge for legacy trigger' do - create(:ci_trigger, owner: user, project: @project).update_attribute(:owner, nil) - visit project_settings_ci_cd_path(@project) - - expect(page.find('.triggers-list')).to have_content 'invalid' - end - it 'show "invalid" badge for trigger with owner having insufficient permissions' do create(:ci_trigger, owner: guest_user, project: @project, description: trigger_title) visit project_settings_ci_cd_path(@project) - # See if trigger without owner (i.e. legacy) shows "legacy" badge and is non-editable expect(page.find('.triggers-list')).to have_content 'invalid' expect(page.find('.triggers-list')).not_to have_selector('a[title="Edit"]') end - it 'do not show "Edit" or full token for legacy trigger' do - create(:ci_trigger, owner: user, project: @project, description: trigger_title) - .update_attribute(:owner, nil) - visit project_settings_ci_cd_path(@project) - - # See if trigger not owned shows only first few token chars and doesn't have copy-to-clipboard button - expect(page.find('.triggers-list')).to have_content(@project.triggers.first.token[0..3]) - expect(page.find('.triggers-list')).not_to have_selector('button.btn-clipboard') - - # See if trigger is non-editable - expect(page.find('.triggers-list')).not_to have_selector('a[title="Edit"]') - end - it 'do not show "Edit" or full token for not owned trigger' do # Create trigger with user different from current_user create(:ci_trigger, owner: user2, project: @project, description: trigger_title) @@ -169,56 +128,5 @@ describe 'Triggers', :js do expect(page.find('.triggers-list .trigger-owner')).to have_content user.name expect(page.find('.triggers-list')).to have_selector('a[title="Edit"]') end - - context 'when :use_legacy_pipeline_triggers feature flag is enabled' do - before do - stub_feature_flags(use_legacy_pipeline_triggers: true) - end - - it 'show "legacy" badge for legacy trigger' do - create(:ci_trigger, owner: nil, project: @project) - visit project_settings_ci_cd_path(@project) - - # See if trigger without owner (i.e. legacy) shows "legacy" badge and is editable - expect(page.find('.triggers-list')).to have_content 'legacy' - expect(page.find('.triggers-list')).to have_selector('a[title="Edit"]') - end - - it 'show "invalid" badge for trigger with owner having insufficient permissions' do - create(:ci_trigger, owner: guest_user, project: @project, description: trigger_title) - visit project_settings_ci_cd_path(@project) - - # See if trigger without owner (i.e. legacy) shows "legacy" badge and is non-editable - expect(page.find('.triggers-list')).to have_content 'invalid' - expect(page.find('.triggers-list')).not_to have_selector('a[title="Edit"]') - end - - it 'do not show "Edit" or full token for not owned trigger' do - # Create trigger with user different from current_user - create(:ci_trigger, owner: user2, project: @project, description: trigger_title) - visit project_settings_ci_cd_path(@project) - - # See if trigger not owned by current_user shows only first few token chars and doesn't have copy-to-clipboard button - expect(page.find('.triggers-list')).to have_content(@project.triggers.first.token[0..3]) - expect(page.find('.triggers-list')).not_to have_selector('button.btn-clipboard') - - # See if trigger owner name doesn't match with current_user and trigger is non-editable - expect(page.find('.triggers-list .trigger-owner')).not_to have_content user.name - expect(page.find('.triggers-list')).not_to have_selector('a[title="Edit"]') - end - - it 'show "Edit" and full token for owned trigger' do - create(:ci_trigger, owner: user, project: @project, description: trigger_title) - visit project_settings_ci_cd_path(@project) - - # See if trigger shows full token and has copy-to-clipboard button - expect(page.find('.triggers-list')).to have_content @project.triggers.first.token - expect(page.find('.triggers-list')).to have_selector('button.btn-clipboard') - - # See if trigger owner name matches with current_user and is editable - expect(page.find('.triggers-list .trigger-owner')).to have_content user.name - expect(page.find('.triggers-list')).to have_selector('a[title="Edit"]') - end - end end end diff --git a/spec/fixtures/lib/gitlab/import_export/group/project.json b/spec/fixtures/lib/gitlab/import_export/group/project.json index 47faf271cca..ce4fa1981ff 100644 --- a/spec/fixtures/lib/gitlab/import_export/group/project.json +++ b/spec/fixtures/lib/gitlab/import_export/group/project.json @@ -175,6 +175,67 @@ } } ] + }, + { + "id": 3, + "title": "Issue with Epic", + "author_id": 1, + "project_id": 8, + "created_at": "2019-12-08T19:41:11.233Z", + "updated_at": "2019-12-08T19:41:53.194Z", + "position": 0, + "branch_name": null, + "description": "Donec at nulla vitae sem molestie rutrum ut at sem.", + "state": "opened", + "iid": 3, + "updated_by_id": null, + "confidential": false, + "due_date": null, + "moved_to_id": null, + "issue_assignees": [], + "notes": [], + "milestone": { + "id": 2, + "title": "A group milestone", + "description": "Group-level milestone", + "due_date": null, + "created_at": "2016-06-14T15:02:04.415Z", + "updated_at": "2016-06-14T15:02:04.415Z", + "state": "active", + "iid": 1, + "group_id": 100 + }, + "epic": { + "id": 1, + "group_id": 5, + "author_id": 1, + "assignee_id": null, + "iid": 1, + "updated_by_id": null, + "last_edited_by_id": null, + "lock_version": 0, + "start_date": null, + "end_date": null, + "last_edited_at": null, + "created_at": "2019-12-08T19:37:07.098Z", + "updated_at": "2019-12-08T19:43:11.568Z", + "title": "An epic", + "description": null, + "start_date_sourcing_milestone_id": null, + "due_date_sourcing_milestone_id": null, + "start_date_fixed": null, + "due_date_fixed": null, + "start_date_is_fixed": null, + "due_date_is_fixed": null, + "closed_by_id": null, + "closed_at": null, + "parent_id": null, + "relative_position": null, + "state_id": "opened", + "start_date_sourcing_epic_id": null, + "due_date_sourcing_epic_id": null, + "milestone_id": null + } } ], "snippets": [ diff --git a/spec/frontend/error_tracking/components/error_details_spec.js b/spec/frontend/error_tracking/components/error_details_spec.js index eefaff4aba7..b5ce9383eb9 100644 --- a/spec/frontend/error_tracking/components/error_details_spec.js +++ b/spec/frontend/error_tracking/components/error_details_spec.js @@ -13,6 +13,7 @@ describe('ErrorDetails', () => { let wrapper; let actions; let getters; + let mocks; const findInput = name => { const inputs = wrapper.findAll(GlFormInput).filter(c => c.attributes('name') === name); @@ -24,13 +25,27 @@ describe('ErrorDetails', () => { stubs: { LoadingButton }, localVue, store, + mocks, propsData: { + issueId: '123', + projectPath: '/root/gitlab-test', issueDetailsPath: '/123/details', issueStackTracePath: '/stacktrace', projectIssuesPath: '/test-project/issues/', csrfToken: 'fakeToken', }, }); + wrapper.setData({ + GQLerror: { + id: 129381, + title: 'Issue title', + externalUrl: 'http://sentry.gitlab.net/gitlab', + firstSeen: '2017-05-26T13:32:48Z', + lastSeen: '2018-05-26T13:32:48Z', + count: 12, + userCount: 2, + }, + }); } beforeEach(() => { @@ -61,6 +76,19 @@ describe('ErrorDetails', () => { }, }, }); + + const query = jest.fn(); + mocks = { + $apollo: { + query, + queries: { + GQLerror: { + loading: true, + stopPolling: jest.fn(), + }, + }, + }, + }; }); afterEach(() => { @@ -85,10 +113,11 @@ describe('ErrorDetails', () => { beforeEach(() => { store.state.details.loading = false; store.state.details.error.id = 1; + mocks.$apollo.queries.GQLerror.loading = false; + mountComponent(); }); it('should show Sentry error details without stacktrace', () => { - mountComponent(); expect(wrapper.find(GlLink).exists()).toBe(true); expect(wrapper.find(GlLoadingIcon).exists()).toBe(true); expect(wrapper.find(Stacktrace).exists()).toBe(false); @@ -99,13 +128,17 @@ describe('ErrorDetails', () => { it('should show language and error level badges', () => { store.state.details.error.tags = { level: 'error', logger: 'ruby' }; mountComponent(); - expect(wrapper.findAll(GlBadge).length).toBe(2); + return wrapper.vm.$nextTick().then(() => { + expect(wrapper.findAll(GlBadge).length).toBe(2); + }); }); it('should NOT show the badge if the tag is not present', () => { store.state.details.error.tags = { level: 'error' }; mountComponent(); - expect(wrapper.findAll(GlBadge).length).toBe(1); + return wrapper.vm.$nextTick().then(() => { + expect(wrapper.findAll(GlBadge).length).toBe(1); + }); }); }); @@ -113,8 +146,10 @@ describe('ErrorDetails', () => { it('should show stacktrace', () => { store.state.details.loadingStacktrace = false; mountComponent(); - expect(wrapper.find(GlLoadingIcon).exists()).toBe(false); - expect(wrapper.find(Stacktrace).exists()).toBe(true); + return wrapper.vm.$nextTick().then(() => { + expect(wrapper.find(GlLoadingIcon).exists()).toBe(false); + expect(wrapper.find(Stacktrace).exists()).toBe(true); + }); }); it('should NOT show stacktrace if no entries', () => { @@ -128,15 +163,6 @@ describe('ErrorDetails', () => { describe('When a user clicks the create issue button', () => { beforeEach(() => { - store.state.details.error = { - id: 129381, - title: 'Issue title', - external_url: 'http://sentry.gitlab.net/gitlab', - first_seen: '2017-05-26T13:32:48Z', - last_seen: '2018-05-26T13:32:48Z', - count: 12, - user_count: 2, - }; mountComponent(); }); diff --git a/spec/frontend/helpers/diffs_helper_spec.js b/spec/frontend/helpers/diffs_helper_spec.js new file mode 100644 index 00000000000..b223d48bf5c --- /dev/null +++ b/spec/frontend/helpers/diffs_helper_spec.js @@ -0,0 +1,113 @@ +import * as diffsHelper from '~/helpers/diffs_helper'; + +describe('diffs helper', () => { + function getDiffFile(withOverrides = {}) { + return { + parallel_diff_lines: ['line'], + highlighted_diff_lines: ['line'], + blob: { + readable_text: 'text', + }, + ...withOverrides, + }; + } + + describe('hasInlineLines', () => { + it('is false when the file does not exist', () => { + expect(diffsHelper.hasInlineLines()).toBeFalsy(); + }); + + it('is false when the file does not have the highlighted_diff_lines property', () => { + const missingInline = getDiffFile({ highlighted_diff_lines: undefined }); + + expect(diffsHelper.hasInlineLines(missingInline)).toBeFalsy(); + }); + + it('is false when the file has zero highlighted_diff_lines', () => { + const emptyInline = getDiffFile({ highlighted_diff_lines: [] }); + + expect(diffsHelper.hasInlineLines(emptyInline)).toBeFalsy(); + }); + + it('is true when the file has at least 1 highlighted_diff_lines', () => { + expect(diffsHelper.hasInlineLines(getDiffFile())).toBeTruthy(); + }); + }); + + describe('hasParallelLines', () => { + it('is false when the file does not exist', () => { + expect(diffsHelper.hasParallelLines()).toBeFalsy(); + }); + + it('is false when the file does not have the parallel_diff_lines property', () => { + const missingInline = getDiffFile({ parallel_diff_lines: undefined }); + + expect(diffsHelper.hasParallelLines(missingInline)).toBeFalsy(); + }); + + it('is false when the file has zero parallel_diff_lines', () => { + const emptyInline = getDiffFile({ parallel_diff_lines: [] }); + + expect(diffsHelper.hasParallelLines(emptyInline)).toBeFalsy(); + }); + + it('is true when the file has at least 1 parallel_diff_lines', () => { + expect(diffsHelper.hasParallelLines(getDiffFile())).toBeTruthy(); + }); + }); + + describe('isSingleViewStyle', () => { + it('is true when the file has at least 1 inline line but no parallel lines for any reason', () => { + const noParallelLines = getDiffFile({ parallel_diff_lines: undefined }); + const emptyParallelLines = getDiffFile({ parallel_diff_lines: [] }); + + expect(diffsHelper.isSingleViewStyle(noParallelLines)).toBeTruthy(); + expect(diffsHelper.isSingleViewStyle(emptyParallelLines)).toBeTruthy(); + }); + + it('is true when the file has at least 1 parallel line but no inline lines for any reason', () => { + const noInlineLines = getDiffFile({ highlighted_diff_lines: undefined }); + const emptyInlineLines = getDiffFile({ highlighted_diff_lines: [] }); + + expect(diffsHelper.isSingleViewStyle(noInlineLines)).toBeTruthy(); + expect(diffsHelper.isSingleViewStyle(emptyInlineLines)).toBeTruthy(); + }); + + it('is true when the file does not have any inline lines or parallel lines for any reason', () => { + const noLines = getDiffFile({ + highlighted_diff_lines: undefined, + parallel_diff_lines: undefined, + }); + const emptyLines = getDiffFile({ + highlighted_diff_lines: [], + parallel_diff_lines: [], + }); + + expect(diffsHelper.isSingleViewStyle(noLines)).toBeTruthy(); + expect(diffsHelper.isSingleViewStyle(emptyLines)).toBeTruthy(); + expect(diffsHelper.isSingleViewStyle()).toBeTruthy(); + }); + + it('is false when the file has both inline and parallel lines', () => { + expect(diffsHelper.isSingleViewStyle(getDiffFile())).toBeFalsy(); + }); + }); + + describe.each` + context | inline | parallel | blob | expected + ${'only has inline lines'} | ${['line']} | ${undefined} | ${undefined} | ${true} + ${'only has parallel lines'} | ${undefined} | ${['line']} | ${undefined} | ${true} + ${"doesn't have inline, parallel, or blob"} | ${undefined} | ${undefined} | ${undefined} | ${true} + ${'has blob readable text'} | ${undefined} | ${undefined} | ${{ readable_text: 'text' }} | ${false} + `('when hasDiff', ({ context, inline, parallel, blob, expected }) => { + it(`${context}`, () => { + const diffFile = getDiffFile({ + highlighted_diff_lines: inline, + parallel_diff_lines: parallel, + blob, + }); + + expect(diffsHelper.hasDiff(diffFile)).toEqual(expected); + }); + }); +}); diff --git a/spec/frontend/monitoring/components/dashboard_spec.js b/spec/frontend/monitoring/components/dashboard_spec.js index 4caa66cf600..efae8b941ee 100644 --- a/spec/frontend/monitoring/components/dashboard_spec.js +++ b/spec/frontend/monitoring/components/dashboard_spec.js @@ -345,7 +345,7 @@ describe('Dashboard', () => { it('metrics can be swapped', done => { const firstDraggable = findDraggables().at(0); - const mockMetrics = [...metricsGroupsAPIResponse[1].panels]; + const mockMetrics = [...metricsGroupsAPIResponse.panel_groups[1].panels]; const firstTitle = mockMetrics[0].title; const secondTitle = mockMetrics[1].title; diff --git a/spec/frontend/monitoring/mock_data.js b/spec/frontend/monitoring/mock_data.js index 6ded22b4a3f..77c92d0eca6 100644 --- a/spec/frontend/monitoring/mock_data.js +++ b/spec/frontend/monitoring/mock_data.js @@ -331,77 +331,80 @@ export const mockedQueryResultPayloadCoresTotal = { ], }; -export const metricsGroupsAPIResponse = [ - { - group: 'Response metrics (NGINX Ingress VTS)', - priority: 10, - panels: [ - { - metrics: [ - { - id: 'response_metrics_nginx_ingress_throughput_status_code', - label: 'Status Code', - metric_id: 1, - prometheus_endpoint_path: - '/root/autodevops-deploy/environments/32/prometheus/api/v1/query_range?query=sum%28rate%28nginx_upstream_responses_total%7Bupstream%3D~%22%25%7Bkube_namespace%7D-%25%7Bci_environment_slug%7D-.%2A%22%7D%5B2m%5D%29%29+by+%28status_code%29', - query_range: - 'sum(rate(nginx_upstream_responses_total{upstream=~"%{kube_namespace}-%{ci_environment_slug}-.*"}[2m])) by (status_code)', - unit: 'req / sec', - }, - ], - title: 'Throughput', - type: 'area-chart', - weight: 1, - y_label: 'Requests / Sec', - }, - ], - }, - { - group: 'System metrics (Kubernetes)', - priority: 5, - panels: [ - { - title: 'Memory Usage (Pod average)', - type: 'area-chart', - y_label: 'Memory Used per Pod', - weight: 2, - metrics: [ - { - id: 'system_metrics_kubernetes_container_memory_average', - query_range: - 'avg(sum(container_memory_usage_bytes{container_name!="POD",pod_name=~"^%{ci_environment_slug}-([^c].*|c([^a]|a([^n]|n([^a]|a([^r]|r[^y])))).*|)-(.*)",namespace="%{kube_namespace}"}) by (job)) without (job) / count(avg(container_memory_usage_bytes{container_name!="POD",pod_name=~"^%{ci_environment_slug}-([^c].*|c([^a]|a([^n]|n([^a]|a([^r]|r[^y])))).*|)-(.*)",namespace="%{kube_namespace}"}) without (job)) /1024/1024', - label: 'Pod average', - unit: 'MB', - metric_id: 17, - prometheus_endpoint_path: - '/root/autodevops-deploy/environments/32/prometheus/api/v1/query_range?query=avg%28sum%28container_memory_usage_bytes%7Bcontainer_name%21%3D%22POD%22%2Cpod_name%3D~%22%5E%25%7Bci_environment_slug%7D-%28%5B%5Ec%5D.%2A%7Cc%28%5B%5Ea%5D%7Ca%28%5B%5En%5D%7Cn%28%5B%5Ea%5D%7Ca%28%5B%5Er%5D%7Cr%5B%5Ey%5D%29%29%29%29.%2A%7C%29-%28.%2A%29%22%2Cnamespace%3D%22%25%7Bkube_namespace%7D%22%7D%29+by+%28job%29%29+without+%28job%29+%2F+count%28avg%28container_memory_usage_bytes%7Bcontainer_name%21%3D%22POD%22%2Cpod_name%3D~%22%5E%25%7Bci_environment_slug%7D-%28%5B%5Ec%5D.%2A%7Cc%28%5B%5Ea%5D%7Ca%28%5B%5En%5D%7Cn%28%5B%5Ea%5D%7Ca%28%5B%5Er%5D%7Cr%5B%5Ey%5D%29%29%29%29.%2A%7C%29-%28.%2A%29%22%2Cnamespace%3D%22%25%7Bkube_namespace%7D%22%7D%29+without+%28job%29%29+%2F1024%2F1024', - appearance: { - line: { - width: 2, +export const metricsGroupsAPIResponse = { + dashboard: 'Environment metrics', + panel_groups: [ + { + group: 'Response metrics (NGINX Ingress VTS)', + priority: 10, + panels: [ + { + metrics: [ + { + id: 'response_metrics_nginx_ingress_throughput_status_code', + label: 'Status Code', + metric_id: 1, + prometheus_endpoint_path: + '/root/autodevops-deploy/environments/32/prometheus/api/v1/query_range?query=sum%28rate%28nginx_upstream_responses_total%7Bupstream%3D~%22%25%7Bkube_namespace%7D-%25%7Bci_environment_slug%7D-.%2A%22%7D%5B2m%5D%29%29+by+%28status_code%29', + query_range: + 'sum(rate(nginx_upstream_responses_total{upstream=~"%{kube_namespace}-%{ci_environment_slug}-.*"}[2m])) by (status_code)', + unit: 'req / sec', + }, + ], + title: 'Throughput', + type: 'area-chart', + weight: 1, + y_label: 'Requests / Sec', + }, + ], + }, + { + group: 'System metrics (Kubernetes)', + priority: 5, + panels: [ + { + title: 'Memory Usage (Pod average)', + type: 'area-chart', + y_label: 'Memory Used per Pod', + weight: 2, + metrics: [ + { + id: 'system_metrics_kubernetes_container_memory_average', + query_range: + 'avg(sum(container_memory_usage_bytes{container_name!="POD",pod_name=~"^%{ci_environment_slug}-([^c].*|c([^a]|a([^n]|n([^a]|a([^r]|r[^y])))).*|)-(.*)",namespace="%{kube_namespace}"}) by (job)) without (job) / count(avg(container_memory_usage_bytes{container_name!="POD",pod_name=~"^%{ci_environment_slug}-([^c].*|c([^a]|a([^n]|n([^a]|a([^r]|r[^y])))).*|)-(.*)",namespace="%{kube_namespace}"}) without (job)) /1024/1024', + label: 'Pod average', + unit: 'MB', + metric_id: 17, + prometheus_endpoint_path: + '/root/autodevops-deploy/environments/32/prometheus/api/v1/query_range?query=avg%28sum%28container_memory_usage_bytes%7Bcontainer_name%21%3D%22POD%22%2Cpod_name%3D~%22%5E%25%7Bci_environment_slug%7D-%28%5B%5Ec%5D.%2A%7Cc%28%5B%5Ea%5D%7Ca%28%5B%5En%5D%7Cn%28%5B%5Ea%5D%7Ca%28%5B%5Er%5D%7Cr%5B%5Ey%5D%29%29%29%29.%2A%7C%29-%28.%2A%29%22%2Cnamespace%3D%22%25%7Bkube_namespace%7D%22%7D%29+by+%28job%29%29+without+%28job%29+%2F+count%28avg%28container_memory_usage_bytes%7Bcontainer_name%21%3D%22POD%22%2Cpod_name%3D~%22%5E%25%7Bci_environment_slug%7D-%28%5B%5Ec%5D.%2A%7Cc%28%5B%5Ea%5D%7Ca%28%5B%5En%5D%7Cn%28%5B%5Ea%5D%7Ca%28%5B%5Er%5D%7Cr%5B%5Ey%5D%29%29%29%29.%2A%7C%29-%28.%2A%29%22%2Cnamespace%3D%22%25%7Bkube_namespace%7D%22%7D%29+without+%28job%29%29+%2F1024%2F1024', + appearance: { + line: { + width: 2, + }, }, }, - }, - ], - }, - { - title: 'Core Usage (Total)', - type: 'area-chart', - y_label: 'Total Cores', - weight: 3, - metrics: [ - { - id: 'system_metrics_kubernetes_container_cores_total', - query_range: - 'avg(sum(rate(container_cpu_usage_seconds_total{container_name!="POD",pod_name=~"^%{ci_environment_slug}-(.*)",namespace="%{kube_namespace}"}[15m])) by (job)) without (job)', - label: 'Total', - unit: 'cores', - metric_id: 13, - }, - ], - }, - ], - }, -]; + ], + }, + { + title: 'Core Usage (Total)', + type: 'area-chart', + y_label: 'Total Cores', + weight: 3, + metrics: [ + { + id: 'system_metrics_kubernetes_container_cores_total', + query_range: + 'avg(sum(rate(container_cpu_usage_seconds_total{container_name!="POD",pod_name=~"^%{ci_environment_slug}-(.*)",namespace="%{kube_namespace}"}[15m])) by (job)) without (job)', + label: 'Total', + unit: 'cores', + metric_id: 13, + }, + ], + }, + ], + }, + ], +}; export const environmentData = [ { diff --git a/spec/frontend/monitoring/store/actions_spec.js b/spec/frontend/monitoring/store/actions_spec.js index f38bd4384e2..c1ad59ac95b 100644 --- a/spec/frontend/monitoring/store/actions_spec.js +++ b/spec/frontend/monitoring/store/actions_spec.js @@ -298,7 +298,7 @@ describe('Monitoring store actions', () => { ); expect(commit).toHaveBeenCalledWith( types.RECEIVE_METRICS_DATA_SUCCESS, - metricsDashboardResponse.dashboard.panel_groups, + metricsDashboardResponse.dashboard, ); expect(dispatch).toHaveBeenCalledWith('fetchPrometheusMetrics', params); }); @@ -441,7 +441,7 @@ describe('Monitoring store actions', () => { beforeEach(() => { state = storeState(); [metric] = metricsDashboardResponse.dashboard.panel_groups[0].panels[0].metrics; - [data] = metricsGroupsAPIResponse[0].panels[0].metrics; + [data] = metricsGroupsAPIResponse.panel_groups[0].panels[0].metrics; }); it('commits result', done => { diff --git a/spec/frontend/monitoring/store/mutations_spec.js b/spec/frontend/monitoring/store/mutations_spec.js index 60107a03674..e89c22268d4 100644 --- a/spec/frontend/monitoring/store/mutations_spec.js +++ b/spec/frontend/monitoring/store/mutations_spec.js @@ -100,12 +100,12 @@ describe('Monitoring mutations', () => { values: [[0, 1], [1, 1], [1, 3]], }, ]; - const dashboardGroups = metricsDashboardResponse.dashboard.panel_groups; + const { dashboard } = metricsDashboardResponse; const getMetric = () => stateCopy.dashboard.panel_groups[0].panels[0].metrics[0]; describe('REQUEST_METRIC_RESULT', () => { beforeEach(() => { - mutations[types.RECEIVE_METRICS_DATA_SUCCESS](stateCopy, dashboardGroups); + mutations[types.RECEIVE_METRICS_DATA_SUCCESS](stateCopy, dashboard); }); it('stores a loading state on a metric', () => { expect(stateCopy.showEmptyState).toBe(true); @@ -128,7 +128,7 @@ describe('Monitoring mutations', () => { describe('RECEIVE_METRIC_RESULT_SUCCESS', () => { beforeEach(() => { - mutations[types.RECEIVE_METRICS_DATA_SUCCESS](stateCopy, dashboardGroups); + mutations[types.RECEIVE_METRICS_DATA_SUCCESS](stateCopy, dashboard); }); it('clears empty state', () => { expect(stateCopy.showEmptyState).toBe(true); @@ -161,7 +161,7 @@ describe('Monitoring mutations', () => { describe('RECEIVE_METRIC_RESULT_FAILURE', () => { beforeEach(() => { - mutations[types.RECEIVE_METRICS_DATA_SUCCESS](stateCopy, dashboardGroups); + mutations[types.RECEIVE_METRICS_DATA_SUCCESS](stateCopy, dashboard); }); it('maintains the loading state when a metric fails', () => { expect(stateCopy.showEmptyState).toBe(true); diff --git a/spec/helpers/projects/error_tracking_helper_spec.rb b/spec/helpers/projects/error_tracking_helper_spec.rb index 753144eef89..7c67448034b 100644 --- a/spec/helpers/projects/error_tracking_helper_spec.rb +++ b/spec/helpers/projects/error_tracking_helper_spec.rb @@ -80,11 +80,20 @@ describe Projects::ErrorTrackingHelper do let(:issue_id) { 1234 } let(:route_params) { [project.owner, project, issue_id, { format: :json }] } let(:details_path) { details_namespace_project_error_tracking_index_path(*route_params) } + let(:project_path) { project.full_path } let(:stack_trace_path) { stack_trace_namespace_project_error_tracking_index_path(*route_params) } let(:issues_path) { project_issues_path(project) } let(:result) { helper.error_details_data(project, issue_id) } + it 'returns the correct issue id' do + expect(result['issue-id']).to eq issue_id + end + + it 'returns the correct project path' do + expect(result['project-path']).to eq project_path + end + it 'returns the correct details path' do expect(result['issue-details-path']).to eq details_path end diff --git a/spec/javascripts/diffs/components/app_spec.js b/spec/javascripts/diffs/components/app_spec.js index 266bbc1eadd..4b4a710df2d 100644 --- a/spec/javascripts/diffs/components/app_spec.js +++ b/spec/javascripts/diffs/components/app_spec.js @@ -10,6 +10,7 @@ import CompareVersions from '~/diffs/components/compare_versions.vue'; import HiddenFilesWarning from '~/diffs/components/hidden_files_warning.vue'; import CommitWidget from '~/diffs/components/commit_widget.vue'; import TreeList from '~/diffs/components/tree_list.vue'; +import { INLINE_DIFF_VIEW_TYPE, PARALLEL_DIFF_VIEW_TYPE } from '~/diffs/constants'; import createDiffsStore from '../create_diffs_store'; import diffsMockData from '../mock_data/merge_request_diffs'; @@ -41,7 +42,6 @@ describe('diffs/components/app', () => { changesEmptyStateIllustration: '', dismissEndpoint: '', showSuggestPopover: true, - useSingleDiffStyle: false, ...props, }, store, @@ -53,6 +53,12 @@ describe('diffs/components/app', () => { }); } + function getOppositeViewType(currentViewType) { + return currentViewType === INLINE_DIFF_VIEW_TYPE + ? PARALLEL_DIFF_VIEW_TYPE + : INLINE_DIFF_VIEW_TYPE; + } + beforeEach(() => { // setup globals (needed for component to mount :/) window.mrTabs = jasmine.createSpyObj('mrTabs', ['resetViewContainer']); @@ -82,9 +88,146 @@ describe('diffs/components/app', () => { spyOn(wrapper.vm, 'startRenderDiffsQueue'); spyOn(wrapper.vm, 'unwatchDiscussions'); store.state.diffs.retrievingBatches = true; + store.state.diffs.diffFiles = []; wrapper.vm.$nextTick(done); }); + describe('when the diff view type changes and it should load a single diff view style', () => { + const noLinesDiff = { + highlighted_diff_lines: [], + parallel_diff_lines: [], + }; + const parallelLinesDiff = { + highlighted_diff_lines: [], + parallel_diff_lines: ['line'], + }; + const inlineLinesDiff = { + highlighted_diff_lines: ['line'], + parallel_diff_lines: [], + }; + const fullDiff = { + highlighted_diff_lines: ['line'], + parallel_diff_lines: ['line'], + }; + + function expectFetchToOccur({ + vueInstance, + done = () => {}, + batch = false, + existingFiles = 1, + } = {}) { + vueInstance.$nextTick(() => { + expect(vueInstance.diffFiles.length).toEqual(existingFiles); + + if (!batch) { + expect(vueInstance.fetchDiffFiles).toHaveBeenCalled(); + expect(vueInstance.fetchDiffFilesBatch).not.toHaveBeenCalled(); + } else { + expect(vueInstance.fetchDiffFiles).not.toHaveBeenCalled(); + expect(vueInstance.fetchDiffFilesBatch).toHaveBeenCalled(); + } + + done(); + }); + } + + beforeEach(() => { + wrapper.vm.glFeatures.singleMrDiffView = true; + }); + + it('fetches diffs if it has none', done => { + wrapper.vm.isLatestVersion = () => false; + + store.state.diffs.diffViewType = getOppositeViewType(wrapper.vm.diffViewType); + + expectFetchToOccur({ vueInstance: wrapper.vm, batch: false, existingFiles: 0, done }); + }); + + it('fetches diffs if it has both view styles, but no lines in either', done => { + wrapper.vm.isLatestVersion = () => false; + + store.state.diffs.diffFiles.push(noLinesDiff); + store.state.diffs.diffViewType = getOppositeViewType(wrapper.vm.diffViewType); + + expectFetchToOccur({ vueInstance: wrapper.vm, done }); + }); + + it('fetches diffs if it only has inline view style', done => { + wrapper.vm.isLatestVersion = () => false; + + store.state.diffs.diffFiles.push(inlineLinesDiff); + store.state.diffs.diffViewType = getOppositeViewType(wrapper.vm.diffViewType); + + expectFetchToOccur({ vueInstance: wrapper.vm, done }); + }); + + it('fetches diffs if it only has parallel view style', done => { + wrapper.vm.isLatestVersion = () => false; + + store.state.diffs.diffFiles.push(parallelLinesDiff); + store.state.diffs.diffViewType = getOppositeViewType(wrapper.vm.diffViewType); + + expectFetchToOccur({ vueInstance: wrapper.vm, done }); + }); + + it('fetches batch diffs if it has none', done => { + wrapper.vm.glFeatures.diffsBatchLoad = true; + + store.state.diffs.diffViewType = getOppositeViewType(wrapper.vm.diffViewType); + + expectFetchToOccur({ vueInstance: wrapper.vm, batch: true, existingFiles: 0, done }); + }); + + it('fetches batch diffs if it has both view styles, but no lines in either', done => { + wrapper.vm.glFeatures.diffsBatchLoad = true; + + store.state.diffs.diffFiles.push(noLinesDiff); + store.state.diffs.diffViewType = getOppositeViewType(wrapper.vm.diffViewType); + + expectFetchToOccur({ vueInstance: wrapper.vm, batch: true, done }); + }); + + it('fetches batch diffs if it only has inline view style', done => { + wrapper.vm.glFeatures.diffsBatchLoad = true; + + store.state.diffs.diffFiles.push(inlineLinesDiff); + store.state.diffs.diffViewType = getOppositeViewType(wrapper.vm.diffViewType); + + expectFetchToOccur({ vueInstance: wrapper.vm, batch: true, done }); + }); + + it('fetches batch diffs if it only has parallel view style', done => { + wrapper.vm.glFeatures.diffsBatchLoad = true; + + store.state.diffs.diffFiles.push(parallelLinesDiff); + store.state.diffs.diffViewType = getOppositeViewType(wrapper.vm.diffViewType); + + expectFetchToOccur({ vueInstance: wrapper.vm, batch: true, done }); + }); + + it('does not fetch diffs if it has already fetched both styles of diff', () => { + wrapper.vm.glFeatures.diffsBatchLoad = false; + + store.state.diffs.diffFiles.push(fullDiff); + store.state.diffs.diffViewType = getOppositeViewType(wrapper.vm.diffViewType); + + expect(wrapper.vm.diffFiles.length).toEqual(1); + expect(wrapper.vm.fetchDiffFiles).not.toHaveBeenCalled(); + expect(wrapper.vm.fetchDiffFilesBatch).not.toHaveBeenCalled(); + }); + + it('does not fetch batch diffs if it has already fetched both styles of diff', () => { + wrapper.vm.glFeatures.diffsBatchLoad = true; + + store.state.diffs.diffFiles.push(fullDiff); + store.state.diffs.diffViewType = getOppositeViewType(wrapper.vm.diffViewType); + + expect(wrapper.vm.diffFiles.length).toEqual(1); + expect(wrapper.vm.fetchDiffFiles).not.toHaveBeenCalled(); + expect(wrapper.vm.fetchDiffFilesBatch).not.toHaveBeenCalled(); + }); + }); + it('calls fetchDiffFiles if diffsBatchLoad is not enabled', done => { wrapper.vm.glFeatures.diffsBatchLoad = false; wrapper.vm.fetchData(false); diff --git a/spec/javascripts/diffs/store/actions_spec.js b/spec/javascripts/diffs/store/actions_spec.js index 98a5348c3bc..436d7338361 100644 --- a/spec/javascripts/diffs/store/actions_spec.js +++ b/spec/javascripts/diffs/store/actions_spec.js @@ -120,7 +120,7 @@ describe('DiffsStoreActions', () => { describe('fetchDiffFiles', () => { it('should fetch diff files', done => { - const endpoint = '/fetch/diff/files?w=1'; + const endpoint = '/fetch/diff/files?view=inline&w=1'; const mock = new MockAdapter(axios); const res = { diff_files: 1, merge_request_diffs: [] }; mock.onGet(endpoint).reply(200, res); @@ -128,7 +128,7 @@ describe('DiffsStoreActions', () => { testAction( fetchDiffFiles, {}, - { endpoint }, + { endpoint, diffFiles: [], showWhitespace: false, diffViewType: 'inline' }, [ { type: types.SET_LOADING, payload: true }, { type: types.SET_LOADING, payload: false }, diff --git a/spec/javascripts/diffs/store/mutations_spec.js b/spec/javascripts/diffs/store/mutations_spec.js index 93dbf03e1ed..24405dcc796 100644 --- a/spec/javascripts/diffs/store/mutations_spec.js +++ b/spec/javascripts/diffs/store/mutations_spec.js @@ -52,7 +52,14 @@ describe('DiffsStoreMutations', () => { describe('SET_DIFF_DATA', () => { it('should set diff data type properly', () => { - const state = {}; + const state = { + diffFiles: [ + { + content_sha: diffFileMockData.content_sha, + file_hash: diffFileMockData.file_hash, + }, + ], + }; const diffMock = { diff_files: [diffFileMockData], }; @@ -62,9 +69,41 @@ describe('DiffsStoreMutations', () => { const firstLine = state.diffFiles[0].parallel_diff_lines[0]; expect(firstLine.right.text).toBeUndefined(); + expect(state.diffFiles.length).toEqual(1); expect(state.diffFiles[0].renderIt).toEqual(true); expect(state.diffFiles[0].collapsed).toEqual(false); }); + + describe('given diffsBatchLoad feature flag is enabled', () => { + beforeEach(() => { + gon.features = { diffsBatchLoad: true }; + }); + + afterEach(() => { + delete gon.features; + }); + + it('should not modify the existing state', () => { + const state = { + diffFiles: [ + { + content_sha: diffFileMockData.content_sha, + file_hash: diffFileMockData.file_hash, + highlighted_diff_lines: [], + }, + ], + }; + const diffMock = { + diff_files: [diffFileMockData], + }; + + mutations[types.SET_DIFF_DATA](state, diffMock); + + // If the batch load is enabled, there shouldn't be any processing + // done on the existing state object, so we shouldn't have this. + expect(state.diffFiles[0].parallel_diff_lines).toBeUndefined(); + }); + }); }); describe('SET_DIFFSET_DIFF_DATA_BATCH_DATA', () => { @@ -168,11 +207,17 @@ describe('DiffsStoreMutations', () => { it('should update the state with the given data for the given file hash', () => { const fileHash = 123; const state = { - diffFiles: [{}, { file_hash: fileHash, existing_field: 0 }], + diffFiles: [{}, { content_sha: 'abc', file_hash: fileHash, existing_field: 0 }], }; const data = { diff_files: [ - { file_hash: fileHash, extra_field: 1, existing_field: 1, viewer: { name: 'text' } }, + { + content_sha: 'abc', + file_hash: fileHash, + extra_field: 1, + existing_field: 1, + viewer: { name: 'text' }, + }, ], }; @@ -208,7 +253,7 @@ describe('DiffsStoreMutations', () => { discussions: [], }, right: { - line_code: 'ABC_1', + line_code: 'ABC_2', discussions: [], }, }, @@ -274,7 +319,7 @@ describe('DiffsStoreMutations', () => { discussions: [], }, right: { - line_code: 'ABC_1', + line_code: 'ABC_2', discussions: [], }, }, @@ -352,7 +397,7 @@ describe('DiffsStoreMutations', () => { discussions: [], }, right: { - line_code: 'ABC_1', + line_code: 'ABC_2', discussions: [], }, }, @@ -448,6 +493,7 @@ describe('DiffsStoreMutations', () => { discussions: [], }, ], + parallel_diff_lines: [], }, ], }; diff --git a/spec/javascripts/diffs/store/utils_spec.js b/spec/javascripts/diffs/store/utils_spec.js index 65eb4c9d2a3..638b4510221 100644 --- a/spec/javascripts/diffs/store/utils_spec.js +++ b/spec/javascripts/diffs/store/utils_spec.js @@ -314,11 +314,29 @@ describe('DiffsStoreUtils', () => { }); describe('prepareDiffData', () => { + let mock; let preparedDiff; + let splitInlineDiff; + let splitParallelDiff; + let completedDiff; beforeEach(() => { - preparedDiff = { diff_files: [getDiffFileMock()] }; + mock = getDiffFileMock(); + preparedDiff = { diff_files: [mock] }; + splitInlineDiff = { + diff_files: [Object.assign({}, mock, { parallel_diff_lines: undefined })], + }; + splitParallelDiff = { + diff_files: [Object.assign({}, mock, { highlighted_diff_lines: undefined })], + }; + completedDiff = { + diff_files: [Object.assign({}, mock, { highlighted_diff_lines: undefined })], + }; + utils.prepareDiffData(preparedDiff); + utils.prepareDiffData(splitInlineDiff); + utils.prepareDiffData(splitParallelDiff); + utils.prepareDiffData(completedDiff, [mock]); }); it('sets the renderIt and collapsed attribute on files', () => { @@ -359,6 +377,19 @@ describe('DiffsStoreUtils', () => { expect(firstLine.line_code).toEqual(firstLine.right.line_code); }); + + it('guarantees an empty array for both diff styles', () => { + expect(splitInlineDiff.diff_files[0].parallel_diff_lines.length).toEqual(0); + expect(splitInlineDiff.diff_files[0].highlighted_diff_lines.length).toBeGreaterThan(0); + expect(splitParallelDiff.diff_files[0].parallel_diff_lines.length).toBeGreaterThan(0); + expect(splitParallelDiff.diff_files[0].highlighted_diff_lines.length).toEqual(0); + }); + + it('merges existing diff files with newly loaded diff files to ensure split diffs are eventually completed', () => { + expect(completedDiff.diff_files.length).toEqual(1); + expect(completedDiff.diff_files[0].parallel_diff_lines.length).toBeGreaterThan(0); + expect(completedDiff.diff_files[0].highlighted_diff_lines.length).toBeGreaterThan(0); + }); }); describe('isDiscussionApplicableToLine', () => { diff --git a/spec/lib/gitlab/background_migration/activate_prometheus_services_for_shared_cluster_applications_spec.rb b/spec/lib/gitlab/background_migration/activate_prometheus_services_for_shared_cluster_applications_spec.rb deleted file mode 100644 index 9d9281f4e98..00000000000 --- a/spec/lib/gitlab/background_migration/activate_prometheus_services_for_shared_cluster_applications_spec.rb +++ /dev/null @@ -1,75 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -describe Gitlab::BackgroundMigration::ActivatePrometheusServicesForSharedClusterApplications, :migration, schema: 2019_12_20_102807 do - include MigrationHelpers::PrometheusServiceHelpers - - let(:namespaces) { table(:namespaces) } - let(:projects) { table(:projects) } - let(:services) { table(:services) } - let(:namespace) { namespaces.create(name: 'user', path: 'user') } - let(:project) { projects.create(namespace_id: namespace.id) } - - let(:columns) do - %w(project_id active properties type template push_events - issues_events merge_requests_events tag_push_events - note_events category default wiki_page_events pipeline_events - confidential_issues_events commit_events job_events - confidential_note_events deployment_events) - end - - describe '#perform' do - it 'is idempotent' do - expect { subject.perform(project.id) }.to change { services.order(:id).map { |row| row.attributes } } - - expect { subject.perform(project.id) }.not_to change { services.order(:id).map { |row| row.attributes } } - end - - context 'non prometheus services' do - it 'does not change them' do - other_type = 'SomeOtherService' - services.create(service_params_for(project.id, active: true, type: other_type)) - - expect { subject.perform(project.id) }.not_to change { services.where(type: other_type).order(:id).map { |row| row.attributes } } - end - end - - context 'prometheus services are configured manually ' do - it 'does not change them' do - properties = '{"api_url":"http://test.dev","manual_configuration":"1"}' - services.create(service_params_for(project.id, properties: properties, active: false)) - - expect { subject.perform(project.id) }.not_to change { services.order(:id).map { |row| row.attributes } } - end - end - - context 'prometheus integration services do not exist' do - it 'creates missing services entries' do - subject.perform(project.id) - - rows = services.order(:id).map { |row| row.attributes.slice(*columns).symbolize_keys } - - expect([service_params_for(project.id, active: true)]).to eq rows - end - end - - context 'prometheus integration services exist' do - context 'in active state' do - it 'does not change them' do - services.create(service_params_for(project.id, active: true)) - - expect { subject.perform(project.id) }.not_to change { services.order(:id).map { |row| row.attributes } } - end - end - - context 'not in active state' do - it 'sets active attribute to true' do - service = services.create(service_params_for(project.id)) - - expect { subject.perform(project.id) }.to change { service.reload.active? }.from(false).to(true) - end - end - end - end -end diff --git a/spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb index ac370433955..24d3beb35b9 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb @@ -76,45 +76,8 @@ describe Gitlab::Ci::Pipeline::Chain::Validate::Abilities do end end - context 'when pipeline triggered by legacy trigger' do - let(:user) { nil } - let(:trigger_request) do - build_stubbed(:ci_trigger_request, trigger: build_stubbed(:ci_trigger, owner: nil)) - end - - context 'when :use_legacy_pipeline_triggers feature flag is enabled' do - before do - stub_feature_flags(use_legacy_pipeline_triggers: true) - step.perform! - end - - it 'allows legacy triggers to create a pipeline' do - expect(pipeline).to be_valid - end - - it 'does not break the chain' do - expect(step.break?).to eq false - end - end - - context 'when :use_legacy_pipeline_triggers feature flag is disabled' do - before do - stub_feature_flags(use_legacy_pipeline_triggers: false) - step.perform! - end - - it 'prevents legacy triggers from creating a pipeline' do - expect(pipeline.errors.to_a).to include /Trigger token is invalid/ - end - - it 'breaks the pipeline builder chain' do - expect(step.break?).to eq true - end - end - end - - describe '#allowed_to_create?' do - subject { step.allowed_to_create? } + describe '#allowed_to_write_ref?' do + subject { step.send(:allowed_to_write_ref?) } context 'when user is a developer' do before do diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 07439880beb..ee4db935b58 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -578,3 +578,30 @@ zoom_meetings: sentry_issue: - issue design_versions: *version +epic: +- subscriptions +- award_emoji +- description_versions +- author +- assignee +- issues +- epic_issues +- milestone +- notes +- label_links +- labels +- todos +- metrics +- group +- parent +- children +- updated_by +- last_edited_by +- closed_by +- start_date_sourcing_milestone +- due_date_sourcing_milestone +- start_date_sourcing_epic +- due_date_sourcing_epic +- events +- resource_label_events +- user_mentions
\ No newline at end of file diff --git a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb index 4d12d05211b..56426bb8110 100644 --- a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb @@ -36,10 +36,6 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do end context 'JSON' do - before do - stub_feature_flags(use_legacy_pipeline_triggers: false) - end - it 'restores models based on JSON' do expect(@restored_project_json).to be_truthy end @@ -502,7 +498,7 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do end it_behaves_like 'restores project successfully', - issues: 2, + issues: 3, labels: 2, label_with_priorities: 'A project label', milestones: 2, @@ -515,7 +511,7 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do it 'restores issue states' do expect(project.issues.with_state(:closed).count).to eq(1) - expect(project.issues.with_state(:opened).count).to eq(1) + expect(project.issues.with_state(:opened).count).to eq(2) end end diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index e5014eeca09..c921e7cadde 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -766,3 +766,33 @@ ContainerExpirationPolicy: - older_than - keep_n - enabled +Epic: + - id + - milestone_id + - group_id + - author_id + - assignee_id + - iid + - updated_by_id + - last_edited_by_id + - lock_version + - start_date + - end_date + - last_edited_at + - created_at + - updated_at + - title + - description + - start_date_sourcing_milestone_id + - due_date_sourcing_milestone_id + - start_date_fixed + - due_date_fixed + - start_date_is_fixed + - due_date_is_fixed + - closed_by_id + - closed_at + - parent_id + - relative_position + - state_id + - start_date_sourcing_epic_id + - due_date_sourcing_epic_id diff --git a/spec/migrations/20191204114127_delete_legacy_triggers_spec.rb b/spec/migrations/20191204114127_delete_legacy_triggers_spec.rb new file mode 100644 index 00000000000..c2660d699ca --- /dev/null +++ b/spec/migrations/20191204114127_delete_legacy_triggers_spec.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20191204114127_delete_legacy_triggers.rb') + +describe DeleteLegacyTriggers, :migration, schema: 2019_11_25_140458 do + let(:ci_trigger_table) { table(:ci_triggers) } + let(:user) { table(:users).create!(name: 'test', email: 'test@example.com', projects_limit: 1) } + + before do + @trigger_with_user = ci_trigger_table.create!(owner_id: user.id) + ci_trigger_table.create!(owner_id: nil) + ci_trigger_table.create!(owner_id: nil) + end + + it 'removes legacy triggers which has null owner_id' do + expect do + migrate! + end.to change(ci_trigger_table, :count).by(-2) + + expect(ci_trigger_table.all).to eq([@trigger_with_user]) + end +end diff --git a/spec/migrations/patch_prometheus_services_for_shared_cluster_applications_spec.rb b/spec/migrations/patch_prometheus_services_for_shared_cluster_applications_spec.rb deleted file mode 100644 index 74f6d56a2eb..00000000000 --- a/spec/migrations/patch_prometheus_services_for_shared_cluster_applications_spec.rb +++ /dev/null @@ -1,134 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' -require Rails.root.join('db', 'post_migrate', '20191220102807_patch_prometheus_services_for_shared_cluster_applications.rb') - -describe PatchPrometheusServicesForSharedClusterApplications, :migration, :sidekiq do - include MigrationHelpers::PrometheusServiceHelpers - - let(:namespaces) { table(:namespaces) } - let(:projects) { table(:projects) } - let(:services) { table(:services) } - let(:clusters) { table(:clusters) } - let(:cluster_groups) { table(:cluster_groups) } - let(:clusters_applications_prometheus) { table(:clusters_applications_prometheus) } - let(:namespace) { namespaces.create!(name: 'gitlab', path: 'gitlab-org') } - - let(:application_statuses) do - { - errored: -1, - installed: 3, - updated: 5 - } - end - - let(:cluster_types) do - { - instance_type: 1, - group_type: 2 - } - end - - describe '#up' do - let!(:project_with_missing_service) { projects.create!(name: 'gitlab', path: 'gitlab-ce', namespace_id: namespace.id) } - let(:project_with_inactive_service) { projects.create!(name: 'gitlab', path: 'gitlab-ee', namespace_id: namespace.id) } - let(:project_with_active_service) { projects.create!(name: 'gitlab', path: 'gitlab-ee', namespace_id: namespace.id) } - let(:project_with_manual_active_service) { projects.create!(name: 'gitlab', path: 'gitlab-ee', namespace_id: namespace.id) } - let(:project_with_manual_inactive_service) { projects.create!(name: 'gitlab', path: 'gitlab-ee', namespace_id: namespace.id) } - let(:project_with_active_not_prometheus_service) { projects.create!(name: 'gitlab', path: 'gitlab-ee', namespace_id: namespace.id) } - let(:project_with_inactive_not_prometheus_service) { projects.create!(name: 'gitlab', path: 'gitlab-ee', namespace_id: namespace.id) } - - before do - services.create(service_params_for(project_with_inactive_service.id, active: false)) - services.create(service_params_for(project_with_active_service.id, active: true)) - services.create(service_params_for(project_with_active_not_prometheus_service.id, active: true, type: 'other')) - services.create(service_params_for(project_with_inactive_not_prometheus_service.id, active: false, type: 'other')) - services.create(service_params_for(project_with_manual_inactive_service.id, active: false, properties: { some: 'data' }.to_json)) - services.create(service_params_for(project_with_manual_active_service.id, active: true, properties: { some: 'data' }.to_json)) - end - - shared_examples 'patch prometheus services post migration' do - context 'prometheus application is installed on the cluster' do - it 'schedules a background migration' do - clusters_applications_prometheus.create(cluster_id: cluster.id, status: application_statuses[:installed], version: '123') - - Sidekiq::Testing.fake! do - Timecop.freeze do - background_migrations = [["ActivatePrometheusServicesForSharedClusterApplications", project_with_missing_service.id], - ["ActivatePrometheusServicesForSharedClusterApplications", project_with_inactive_service.id], - ["ActivatePrometheusServicesForSharedClusterApplications", project_with_active_not_prometheus_service.id], - ["ActivatePrometheusServicesForSharedClusterApplications", project_with_inactive_not_prometheus_service.id]] - - migrate! - - enqueued_migrations = BackgroundMigrationWorker.jobs.map { |job| job['args'] } - expect(enqueued_migrations).to match_array(background_migrations) - end - end - end - end - - context 'prometheus application was recently updated on the cluster' do - it 'schedules a background migration' do - clusters_applications_prometheus.create(cluster_id: cluster.id, status: application_statuses[:updated], version: '123') - - Sidekiq::Testing.fake! do - Timecop.freeze do - background_migrations = [["ActivatePrometheusServicesForSharedClusterApplications", project_with_missing_service.id], - ["ActivatePrometheusServicesForSharedClusterApplications", project_with_inactive_service.id], - ["ActivatePrometheusServicesForSharedClusterApplications", project_with_active_not_prometheus_service.id], - ["ActivatePrometheusServicesForSharedClusterApplications", project_with_inactive_not_prometheus_service.id]] - - migrate! - - enqueued_migrations = BackgroundMigrationWorker.jobs.map { |job| job['args'] } - expect(enqueued_migrations).to match_array(background_migrations) - end - end - end - end - - context 'prometheus application failed to install on the cluster' do - it 'does not schedule a background migration' do - clusters_applications_prometheus.create(cluster_id: cluster.id, status: application_statuses[:errored], version: '123') - - Sidekiq::Testing.fake! do - Timecop.freeze do - migrate! - - expect(BackgroundMigrationWorker.jobs.size).to eq 0 - end - end - end - end - - context 'prometheus application is NOT installed on the cluster' do - it 'does not schedule a background migration' do - Sidekiq::Testing.fake! do - Timecop.freeze do - migrate! - - expect(BackgroundMigrationWorker.jobs.size).to eq 0 - end - end - end - end - end - - context 'Cluster is group_type' do - let(:cluster) { clusters.create(name: 'cluster', cluster_type: cluster_types[:group_type]) } - - before do - cluster_groups.create(group_id: namespace.id, cluster_id: cluster.id) - end - - it_behaves_like 'patch prometheus services post migration' - end - - context 'Cluster is instance_type' do - let(:cluster) { clusters.create(name: 'cluster', cluster_type: cluster_types[:instance_type]) } - - it_behaves_like 'patch prometheus services post migration' - end - end -end diff --git a/spec/models/ci/trigger_spec.rb b/spec/models/ci/trigger_spec.rb index 5b5d6f51b33..5b0815f8156 100644 --- a/spec/models/ci/trigger_spec.rb +++ b/spec/models/ci/trigger_spec.rb @@ -11,6 +11,10 @@ describe Ci::Trigger do it { is_expected.to have_many(:trigger_requests) } end + describe 'validations' do + it { is_expected.to validate_presence_of(:owner) } + end + describe 'before_validation' do it 'sets an random token if none provided' do trigger = create(:ci_trigger_without_token, project: project) @@ -35,63 +39,22 @@ describe Ci::Trigger do end end - describe '#legacy?' do - let(:trigger) { create(:ci_trigger, owner: owner, project: project) } - - subject { trigger } - - context 'when owner is blank' do - let(:owner) { nil } - - it { is_expected.to be_legacy } - end - - context 'when owner is set' do - let(:owner) { create(:user) } - - it { is_expected.not_to be_legacy } - end - end - describe '#can_access_project?' do let(:owner) { create(:user) } let(:trigger) { create(:ci_trigger, owner: owner, project: project) } - context 'when owner is blank' do + subject { trigger.can_access_project? } + + context 'and is member of the project' do before do - stub_feature_flags(use_legacy_pipeline_triggers: false) - trigger.update_attribute(:owner, nil) + project.add_developer(owner) end - subject { trigger.can_access_project? } - - it { is_expected.to eq(false) } - - context 'when :use_legacy_pipeline_triggers feature flag is enabled' do - before do - stub_feature_flags(use_legacy_pipeline_triggers: true) - end - - subject { trigger.can_access_project? } - - it { is_expected.to eq(true) } - end + it { is_expected.to eq(true) } end - context 'when owner is set' do - subject { trigger.can_access_project? } - - context 'and is member of the project' do - before do - project.add_developer(owner) - end - - it { is_expected.to eq(true) } - end - - context 'and is not member of the project' do - it { is_expected.to eq(false) } - end + context 'and is not member of the project' do + it { is_expected.to eq(false) } end end end diff --git a/spec/policies/ci/trigger_policy_spec.rb b/spec/policies/ci/trigger_policy_spec.rb index e936277a391..28e5a2b2cd6 100644 --- a/spec/policies/ci/trigger_policy_spec.rb +++ b/spec/policies/ci/trigger_policy_spec.rb @@ -10,60 +10,6 @@ describe Ci::TriggerPolicy do subject { described_class.new(user, trigger) } describe '#rules' do - context 'when owner is undefined' do - before do - stub_feature_flags(use_legacy_pipeline_triggers: false) - trigger.update_attribute(:owner, nil) - end - - context 'when user is maintainer of the project' do - before do - project.add_maintainer(user) - end - - it { is_expected.to be_allowed(:manage_trigger) } - it { is_expected.not_to be_allowed(:admin_trigger) } - end - - context 'when user is developer of the project' do - before do - project.add_developer(user) - end - - it { is_expected.not_to be_allowed(:manage_trigger) } - it { is_expected.not_to be_allowed(:admin_trigger) } - end - - context 'when :use_legacy_pipeline_triggers feature flag is enabled' do - before do - stub_feature_flags(use_legacy_pipeline_triggers: true) - end - - context 'when user is maintainer of the project' do - before do - project.add_maintainer(user) - end - - it { is_expected.to be_allowed(:manage_trigger) } - it { is_expected.to be_allowed(:admin_trigger) } - end - - context 'when user is developer of the project' do - before do - project.add_developer(user) - end - - it { is_expected.not_to be_allowed(:manage_trigger) } - it { is_expected.not_to be_allowed(:admin_trigger) } - end - - context 'when user is not member of the project' do - it { is_expected.not_to be_allowed(:manage_trigger) } - it { is_expected.not_to be_allowed(:admin_trigger) } - end - end - end - context 'when owner is an user' do before do trigger.update!(owner: user) diff --git a/spec/requests/api/triggers_spec.rb b/spec/requests/api/triggers_spec.rb index fd1104fa978..d54d112cd9f 100644 --- a/spec/requests/api/triggers_spec.rb +++ b/spec/requests/api/triggers_spec.rb @@ -87,22 +87,6 @@ describe API::Triggers do expect(pipeline.variables.map { |v| { v.key => v.value } }.last).to eq(variables) end end - - context 'when legacy trigger' do - before do - trigger.update(owner: nil) - end - - it 'creates pipeline' do - post api("/projects/#{project.id}/trigger/pipeline"), params: options.merge(ref: 'master') - - expect(response).to have_gitlab_http_status(201) - expect(json_response).to include('id' => pipeline.id) - pipeline.builds.reload - expect(pipeline.builds.pending.size).to eq(2) - expect(pipeline.builds.size).to eq(5) - end - end end context 'when triggering a pipeline from a trigger token' do diff --git a/spec/routing/admin_routing_spec.rb b/spec/routing/admin_routing_spec.rb index a82bdfe3ce8..93b2c19c74a 100644 --- a/spec/routing/admin_routing_spec.rb +++ b/spec/routing/admin_routing_spec.rb @@ -161,3 +161,17 @@ describe Admin::GroupsController, "routing" do expect(get("/admin/groups/#{name}/edit")).to route_to('admin/groups#edit', id: name) end end + +describe Admin::SessionsController, "routing" do + it "to #new" do + expect(get("/admin/session/new")).to route_to('admin/sessions#new') + end + + it "to #create" do + expect(post("/admin/session")).to route_to('admin/sessions#create') + end + + it "to #destroy" do + expect(post("/admin/session/destroy")).to route_to('admin/sessions#destroy') + end +end diff --git a/spec/routing/routing_spec.rb b/spec/routing/routing_spec.rb index 6f67cdb1222..ff002469e3c 100644 --- a/spec/routing/routing_spec.rb +++ b/spec/routing/routing_spec.rb @@ -256,10 +256,8 @@ describe "Authentication", "routing" do expect(post("/users/sign_in")).to route_to('sessions#create') end - # sign_out with GET instead of DELETE facilitates ad-hoc single-sign-out processes - # (https://gitlab.com/gitlab-org/gitlab-foss/issues/39708) - it "GET /users/sign_out" do - expect(get("/users/sign_out")).to route_to('sessions#destroy') + it "POST /users/sign_out" do + expect(post("/users/sign_out")).to route_to('sessions#destroy') end it "POST /users/password" do diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index 2876f7b5f59..dc67efe0fbe 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -1123,21 +1123,6 @@ describe Ci::CreatePipelineService do it_behaves_like 'when ref is protected' end - context 'when ref is not protected' do - context 'when trigger belongs to no one' do - let(:user) {} - let(:trigger) { create(:ci_trigger, owner: nil) } - let(:trigger_request) { create(:ci_trigger_request, trigger: trigger) } - let(:pipeline) { execute_service(trigger_request: trigger_request) } - - it 'creates an unprotected pipeline' do - expect(pipeline).to be_persisted - expect(pipeline).not_to be_protected - expect(Ci::Pipeline.count).to eq(1) - end - end - end - context 'when pipeline is running for a tag' do before do config = YAML.dump(test: { script: 'test', only: ['branches'] }, diff --git a/spec/support/migrations_helpers/prometheus_service_helpers.rb b/spec/support/migrations_helpers/prometheus_service_helpers.rb deleted file mode 100644 index 88f2f71ee1e..00000000000 --- a/spec/support/migrations_helpers/prometheus_service_helpers.rb +++ /dev/null @@ -1,35 +0,0 @@ -# frozen_string_literal: true - -module MigrationHelpers - module PrometheusServiceHelpers - def service_params_for(project_id, params = {}) - { - project_id: project_id, - active: false, - properties: '{}', - type: 'PrometheusService', - template: false, - push_events: true, - issues_events: true, - merge_requests_events: true, - tag_push_events: true, - note_events: true, - category: 'monitoring', - default: false, - wiki_page_events: true, - pipeline_events: true, - confidential_issues_events: true, - commit_events: true, - job_events: true, - confidential_note_events: true, - deployment_events: false - }.merge(params) - end - - def row_attributes(entity) - entity.attributes.with_indifferent_access.tap do |hash| - hash.merge!(hash.slice(:created_at, :updated_at).transform_values { |v| v.to_s(:db) }) - end - end - end -end diff --git a/spec/support/shared_examples/merge_requests_rendering_a_single_diff_version.rb b/spec/support/shared_examples/merge_requests_rendering_a_single_diff_version.rb deleted file mode 100644 index 18d025a4b07..00000000000 --- a/spec/support/shared_examples/merge_requests_rendering_a_single_diff_version.rb +++ /dev/null @@ -1,21 +0,0 @@ -# frozen_string_literal: true - -# This pending test can be removed when `single_mr_diff_view` is enabled by default -# disabling the feature flag above is then not needed anymore. -RSpec.shared_examples 'rendering a single diff version' do |attribute| - before do - stub_feature_flags(diffs_batch_load: false) - end - - pending 'allows editing diff settings single_mr_diff_view is enabled' do - project = create(:project, :repository) - user = project.creator - merge_request = create(:merge_request, source_project: project) - stub_feature_flags(single_mr_diff_view: true) - sign_in(user) - - visit(diffs_project_merge_request_path(project, merge_request)) - - expect(page).to have_selector('.js-show-diff-settings') - end -end |