diff options
Diffstat (limited to 'spec')
53 files changed, 748 insertions, 438 deletions
diff --git a/spec/controllers/dashboard/projects_controller_spec.rb b/spec/controllers/dashboard/projects_controller_spec.rb index ea68eae12ed..6591901a9dc 100644 --- a/spec/controllers/dashboard/projects_controller_spec.rb +++ b/spec/controllers/dashboard/projects_controller_spec.rb @@ -11,8 +11,10 @@ describe Dashboard::ProjectsController do end context 'user logged in' do + let(:user) { create(:user) } + before do - sign_in create(:user) + sign_in(user) end context 'external authorization' do @@ -24,6 +26,20 @@ describe Dashboard::ProjectsController do expect(response).to have_gitlab_http_status(200) end end + + it 'orders the projects by last activity by default' do + project = create(:project) + project.add_developer(user) + project.update!(last_repository_updated_at: 3.days.ago, last_activity_at: 3.days.ago) + + project2 = create(:project) + project2.add_developer(user) + project2.update!(last_repository_updated_at: 10.days.ago, last_activity_at: 10.days.ago) + + get :index + + expect(assigns(:projects)).to eq([project, project2]) + end end end diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 8d2412f97ef..4e1cac67d23 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -318,6 +318,53 @@ describe ProjectsController do end end + describe '#housekeeping' do + let(:group) { create(:group) } + let(:project) { create(:project, group: group) } + let(:housekeeping) { Projects::HousekeepingService.new(project) } + + context 'when authenticated as owner' do + before do + group.add_owner(user) + sign_in(user) + + allow(Projects::HousekeepingService).to receive(:new).with(project, :gc).and_return(housekeeping) + end + + it 'forces a full garbage collection' do + expect(housekeeping).to receive(:execute).once + + post :housekeeping, + params: { + namespace_id: project.namespace.path, + id: project.path + } + + expect(response).to have_gitlab_http_status(302) + end + end + + context 'when authenticated as developer' do + let(:developer) { create(:user) } + + before do + group.add_developer(developer) + end + + it 'does not execute housekeeping' do + expect(housekeeping).not_to receive(:execute) + + post :housekeeping, + params: { + namespace_id: project.namespace.path, + id: project.path + } + + expect(response).to have_gitlab_http_status(302) + end + end + end + describe "#update" do render_views diff --git a/spec/features/dashboard/shortcuts_spec.rb b/spec/features/dashboard/shortcuts_spec.rb index 55f5ff04d01..254bb12573c 100644 --- a/spec/features/dashboard/shortcuts_spec.rb +++ b/spec/features/dashboard/shortcuts_spec.rb @@ -22,7 +22,7 @@ describe 'Dashboard shortcuts', :js do find('body').send_keys([:shift, 'T']) - check_page_title('Todos') + check_page_title('To-Do List') find('body').send_keys([:shift, 'P']) diff --git a/spec/features/dashboard/todos/todos_spec.rb b/spec/features/dashboard/todos/todos_spec.rb index d58e3b2841e..c48229fc0a0 100644 --- a/spec/features/dashboard/todos/todos_spec.rb +++ b/spec/features/dashboard/todos/todos_spec.rb @@ -13,7 +13,7 @@ describe 'Dashboard Todos' do end it 'shows "All done" message' do - expect(page).to have_content 'Todos let you see what you should do next' + expect(page).to have_content 'Your To-Do List shows what to work on next' end end @@ -72,7 +72,7 @@ describe 'Dashboard Todos' do end it 'updates todo count' do - expect(page).to have_content 'Todos 0' + expect(page).to have_content 'To Do 0' expect(page).to have_content 'Done 1' end @@ -101,7 +101,7 @@ describe 'Dashboard Todos' do end it 'updates todo count' do - expect(page).to have_content 'Todos 1' + expect(page).to have_content 'To Do 1' expect(page).to have_content 'Done 0' end end @@ -211,7 +211,7 @@ describe 'Dashboard Todos' do describe 'restoring the todo' do before do within first('.todo') do - click_link 'Add todo' + click_link 'Add a To Do' end end @@ -220,7 +220,7 @@ describe 'Dashboard Todos' do end it 'updates todo count' do - expect(page).to have_content 'Todos 1' + expect(page).to have_content 'To Do 1' expect(page).to have_content 'Done 0' end end @@ -276,7 +276,7 @@ describe 'Dashboard Todos' do end it 'shows "All done" message!' do - expect(page).to have_content 'Todos 0' + expect(page).to have_content 'To Do 0' expect(page).to have_content "You're all done!" expect(page).not_to have_selector('.gl-pagination') end @@ -303,7 +303,7 @@ describe 'Dashboard Todos' do it 'updates todo count' do mark_all_and_undo - expect(page).to have_content 'Todos 2' + expect(page).to have_content 'To Do 2' expect(page).to have_content 'Done 0' end diff --git a/spec/features/issues/todo_spec.rb b/spec/features/issues/todo_spec.rb index 0114178b9be..07ae159eef4 100644 --- a/spec/features/issues/todo_spec.rb +++ b/spec/features/issues/todo_spec.rb @@ -13,8 +13,8 @@ describe 'Manually create a todo item from issue', :js do it 'creates todo when clicking button' do page.within '.issuable-sidebar' do - click_button 'Add todo' - expect(page).to have_content 'Mark todo as done' + click_button 'Add a To Do' + expect(page).to have_content 'Mark as done' end page.within '.header-content .todos-count' do @@ -30,8 +30,8 @@ describe 'Manually create a todo item from issue', :js do it 'marks a todo as done' do page.within '.issuable-sidebar' do - click_button 'Add todo' - click_button 'Mark todo as done' + click_button 'Add a To Do' + click_button 'Mark as done' end expect(page).to have_selector('.todos-count', visible: false) diff --git a/spec/features/merge_request/user_sees_merge_widget_spec.rb b/spec/features/merge_request/user_sees_merge_widget_spec.rb index 733e8aa3eba..30e30751693 100644 --- a/spec/features/merge_request/user_sees_merge_widget_spec.rb +++ b/spec/features/merge_request/user_sees_merge_widget_spec.rb @@ -519,6 +519,8 @@ describe 'Merge request > User sees merge widget', :js do end before do + allow_any_instance_of(TestSuiteComparerEntity) + .to receive(:max_tests).and_return(2) allow_any_instance_of(MergeRequest) .to receive(:has_test_reports?).and_return(true) allow_any_instance_of(MergeRequest) @@ -551,7 +553,7 @@ describe 'Merge request > User sees merge widget', :js do expect(page).to have_content('rspec found no changed test results out of 1 total test') expect(page).to have_content('junit found 1 failed test result out of 1 total test') expect(page).to have_content('New') - expect(page).to have_content('subtractTest') + expect(page).to have_content('addTest') end end end @@ -562,7 +564,7 @@ describe 'Merge request > User sees merge widget', :js do click_button 'Expand' within(".js-report-section-container") do - click_button 'subtractTest' + click_button 'addTest' expect(page).to have_content('6.66') expect(page).to have_content(sample_java_failed_message.gsub!(/\s+/, ' ').strip) @@ -596,7 +598,7 @@ describe 'Merge request > User sees merge widget', :js do expect(page).to have_content('rspec found 1 failed test result out of 1 total test') expect(page).to have_content('junit found no changed test results out of 1 total test') expect(page).not_to have_content('New') - expect(page).to have_content('Test#sum when a is 2 and b is 2 returns summary') + expect(page).to have_content('Test#sum when a is 1 and b is 3 returns summary') end end end @@ -607,7 +609,7 @@ describe 'Merge request > User sees merge widget', :js do click_button 'Expand' within(".js-report-section-container") do - click_button 'Test#sum when a is 2 and b is 2 returns summary' + click_button 'Test#sum when a is 1 and b is 3 returns summary' expect(page).to have_content('2.22') expect(page).to have_content(sample_rspec_failed_message.gsub!(/\s+/, ' ').strip) @@ -628,13 +630,7 @@ describe 'Merge request > User sees merge widget', :js do let(:head_reports) do Gitlab::Ci::Reports::TestReports.new.tap do |reports| reports.get_suite('rspec').add_test_case(create_test_case_rspec_success) - reports.get_suite('junit').add_test_case(create_test_case_java_resolved) - end - end - - let(:create_test_case_java_resolved) do - create_test_case_java_failed.tap do |test_case| - test_case.instance_variable_set("@status", Gitlab::Ci::Reports::TestCase::STATUS_SUCCESS) + reports.get_suite('junit').add_test_case(create_test_case_java_success) end end @@ -646,7 +642,7 @@ describe 'Merge request > User sees merge widget', :js do within(".js-report-section-container") do expect(page).to have_content('rspec found no changed test results out of 1 total test') expect(page).to have_content('junit found 1 fixed test result out of 1 total test') - expect(page).to have_content('subtractTest') + expect(page).to have_content('addTest') end end end @@ -657,15 +653,53 @@ describe 'Merge request > User sees merge widget', :js do click_button 'Expand' within(".js-report-section-container") do - click_button 'subtractTest' + click_button 'addTest' - expect(page).to have_content('6.66') + expect(page).to have_content('5.55') end end end end end + context 'properly truncates the report' do + let(:base_reports) do + Gitlab::Ci::Reports::TestReports.new.tap do |reports| + 10.times do |index| + reports.get_suite('rspec').add_test_case( + create_test_case_rspec_failed(index)) + reports.get_suite('junit').add_test_case( + create_test_case_java_success(index)) + end + end + end + + let(:head_reports) do + Gitlab::Ci::Reports::TestReports.new.tap do |reports| + 10.times do |index| + reports.get_suite('rspec').add_test_case( + create_test_case_rspec_failed(index)) + reports.get_suite('junit').add_test_case( + create_test_case_java_failed(index)) + end + end + end + + it 'shows test reports summary which includes the resolved failure' do + within(".js-reports-container") do + click_button 'Expand' + + expect(page).to have_content('Test summary contained 20 failed test results out of 20 total tests') + within(".js-report-section-container") do + expect(page).to have_content('rspec found 10 failed test results out of 10 total tests') + expect(page).to have_content('junit found 10 failed test results out of 10 total tests') + + expect(page).to have_content('Test#sum when a is 1 and b is 3 returns summary', count: 2) + end + end + end + end + def comparer Gitlab::Ci::Reports::TestReportsComparer.new(base_reports, head_reports) end diff --git a/spec/features/oauth_login_spec.rb b/spec/features/oauth_login_spec.rb index 5ebfc32952d..86331728f88 100644 --- a/spec/features/oauth_login_spec.rb +++ b/spec/features/oauth_login_spec.rb @@ -16,16 +16,8 @@ describe 'OAuth Login', :js, :allow_forgery_protection do providers = [:github, :twitter, :bitbucket, :gitlab, :google_oauth2, :facebook, :cas3, :auth0, :authentiq, :salesforce] - before(:all) do - # The OmniAuth `full_host` parameter doesn't get set correctly (it gets set to something like `http://localhost` - # here), and causes integration tests to fail with 404s. We set the `full_host` by removing the request path (and - # anything after it) from the request URI. - @omniauth_config_full_host = OmniAuth.config.full_host - OmniAuth.config.full_host = ->(request) { request['REQUEST_URI'].sub(/#{request['REQUEST_PATH']}.*/, '') } - end - - after(:all) do - OmniAuth.config.full_host = @omniauth_config_full_host + around(:all) do |example| + with_omniauth_full_host { example.run } end def login_with_provider(provider, enter_two_factor: false) diff --git a/spec/frontend/boards/services/board_service_spec.js b/spec/frontend/boards/services/board_service_spec.js index de9fc998360..a8a322e7237 100644 --- a/spec/frontend/boards/services/board_service_spec.js +++ b/spec/frontend/boards/services/board_service_spec.js @@ -2,6 +2,7 @@ import BoardService from '~/boards/services/board_service'; import { TEST_HOST } from 'helpers/test_constants'; import AxiosMockAdapter from 'axios-mock-adapter'; import axios from '~/lib/utils/axios_utils'; +import boardsStore from '~/boards/stores/boards_store'; describe('BoardService', () => { const dummyResponse = "without type checking this doesn't matter"; @@ -18,10 +19,11 @@ describe('BoardService', () => { beforeEach(() => { axiosMock = new AxiosMockAdapter(axios); - service = new BoardService({ + boardsStore.setEndpoints({ ...endpoints, boardId, }); + service = new BoardService(); }); describe('all', () => { diff --git a/spec/javascripts/error_tracking_settings/components/app_spec.js b/spec/frontend/error_tracking_settings/components/app_spec.js index 2e52a45fd34..022f12ef191 100644 --- a/spec/javascripts/error_tracking_settings/components/app_spec.js +++ b/spec/frontend/error_tracking_settings/components/app_spec.js @@ -4,7 +4,7 @@ import ErrorTrackingSettings from '~/error_tracking_settings/components/app.vue' import ErrorTrackingForm from '~/error_tracking_settings/components/error_tracking_form.vue'; import ProjectDropdown from '~/error_tracking_settings/components/project_dropdown.vue'; import createStore from '~/error_tracking_settings/store'; -import { TEST_HOST } from 'spec/test_constants'; +import { TEST_HOST } from 'helpers/test_constants'; const localVue = createLocalVue(); localVue.use(Vuex); diff --git a/spec/javascripts/error_tracking_settings/components/error_tracking_form_spec.js b/spec/frontend/error_tracking_settings/components/error_tracking_form_spec.js index 23e57c4bbf1..23e57c4bbf1 100644 --- a/spec/javascripts/error_tracking_settings/components/error_tracking_form_spec.js +++ b/spec/frontend/error_tracking_settings/components/error_tracking_form_spec.js diff --git a/spec/javascripts/error_tracking_settings/components/project_dropdown_spec.js b/spec/frontend/error_tracking_settings/components/project_dropdown_spec.js index 8e5dbe28452..8e5dbe28452 100644 --- a/spec/javascripts/error_tracking_settings/components/project_dropdown_spec.js +++ b/spec/frontend/error_tracking_settings/components/project_dropdown_spec.js diff --git a/spec/javascripts/error_tracking_settings/mock.js b/spec/frontend/error_tracking_settings/mock.js index 32cdba33c14..42233f82d54 100644 --- a/spec/javascripts/error_tracking_settings/mock.js +++ b/spec/frontend/error_tracking_settings/mock.js @@ -1,5 +1,5 @@ import createStore from '~/error_tracking_settings/store'; -import { TEST_HOST } from 'spec/test_constants'; +import { TEST_HOST } from '../helpers/test_constants'; const defaultStore = createStore(); diff --git a/spec/javascripts/error_tracking_settings/store/actions_spec.js b/spec/frontend/error_tracking_settings/store/actions_spec.js index 0255b3a7aa4..1eab0f7470b 100644 --- a/spec/javascripts/error_tracking_settings/store/actions_spec.js +++ b/spec/frontend/error_tracking_settings/store/actions_spec.js @@ -1,13 +1,16 @@ import MockAdapter from 'axios-mock-adapter'; -import testAction from 'spec/helpers/vuex_action_helper'; -import { TEST_HOST } from 'spec/test_constants'; +import testAction from 'helpers/vuex_action_helper'; +import { TEST_HOST } from 'helpers/test_constants'; import axios from '~/lib/utils/axios_utils'; import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils'; -import actionsDefaultExport, * as actions from '~/error_tracking_settings/store/actions'; +import { refreshCurrentPage } from '~/lib/utils/url_utility'; +import * as actions from '~/error_tracking_settings/store/actions'; import * as types from '~/error_tracking_settings/store/mutation_types'; import defaultState from '~/error_tracking_settings/store/state'; import { projectList } from '../mock'; +jest.mock('~/lib/utils/url_utility'); + describe('error tracking settings actions', () => { let state; @@ -21,6 +24,7 @@ describe('error tracking settings actions', () => { afterEach(() => { mock.restore(); + refreshCurrentPage.mockClear(); }); it('should request and transform the project list', done => { @@ -111,7 +115,6 @@ describe('error tracking settings actions', () => { }); it('should save the page', done => { - const refreshCurrentPage = spyOnDependency(actionsDefaultExport, 'refreshCurrentPage'); mock.onPatch(TEST_HOST).reply(200); testAction(actions.updateSettings, null, state, [], [{ type: 'requestSettings' }], () => { expect(mock.history.patch.length).toBe(1); diff --git a/spec/javascripts/error_tracking_settings/store/getters_spec.js b/spec/frontend/error_tracking_settings/store/getters_spec.js index 2c5ff084b8a..2c5ff084b8a 100644 --- a/spec/javascripts/error_tracking_settings/store/getters_spec.js +++ b/spec/frontend/error_tracking_settings/store/getters_spec.js diff --git a/spec/javascripts/error_tracking_settings/store/mutation_spec.js b/spec/frontend/error_tracking_settings/store/mutation_spec.js index bb1f1da784e..fa188462c3f 100644 --- a/spec/javascripts/error_tracking_settings/store/mutation_spec.js +++ b/spec/frontend/error_tracking_settings/store/mutation_spec.js @@ -1,4 +1,4 @@ -import { TEST_HOST } from 'spec/test_constants'; +import { TEST_HOST } from 'helpers/test_constants'; import mutations from '~/error_tracking_settings/store/mutations'; import defaultState from '~/error_tracking_settings/store/state'; import * as types from '~/error_tracking_settings/store/mutation_types'; diff --git a/spec/javascripts/error_tracking_settings/utils_spec.js b/spec/frontend/error_tracking_settings/utils_spec.js index 4b144f7daf1..4b144f7daf1 100644 --- a/spec/javascripts/error_tracking_settings/utils_spec.js +++ b/spec/frontend/error_tracking_settings/utils_spec.js diff --git a/spec/frontend/monitoring/__snapshots__/dashboard_state_spec.js.snap b/spec/frontend/monitoring/__snapshots__/dashboard_state_spec.js.snap new file mode 100644 index 00000000000..5f24bab600c --- /dev/null +++ b/spec/frontend/monitoring/__snapshots__/dashboard_state_spec.js.snap @@ -0,0 +1,37 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`EmptyState shows gettingStarted state 1`] = ` +<glemptystate-stub + description="Stay updated about the performance and health of your environment by configuring Prometheus to monitor your deployments." + primarybuttonlink="/clustersPath" + primarybuttontext="Install on clusters" + secondarybuttonlink="/settingsPath" + secondarybuttontext="Configure existing installation" + svgpath="/path/to/getting-started.svg" + title="Get started with performance monitoring" +/> +`; + +exports[`EmptyState shows loading state 1`] = ` +<glemptystate-stub + description="Creating graphs uses the data from the Prometheus server. If this takes a long time, ensure that data is available." + primarybuttonlink="/documentationPath" + primarybuttontext="View documentation" + secondarybuttonlink="" + secondarybuttontext="" + svgpath="/path/to/loading.svg" + title="Waiting for performance data" +/> +`; + +exports[`EmptyState shows unableToConnect state 1`] = ` +<glemptystate-stub + description="Ensure connectivity is available from the GitLab server to the Prometheus server" + primarybuttonlink="/documentationPath" + primarybuttontext="View documentation" + secondarybuttonlink="/settingsPath" + secondarybuttontext="Configure Prometheus" + svgpath="/path/to/unable-to-connect.svg" + title="Unable to connect to Prometheus server" +/> +`; diff --git a/spec/frontend/monitoring/dashboard_state_spec.js b/spec/frontend/monitoring/dashboard_state_spec.js new file mode 100644 index 00000000000..950422911eb --- /dev/null +++ b/spec/frontend/monitoring/dashboard_state_spec.js @@ -0,0 +1,43 @@ +import { shallowMount } from '@vue/test-utils'; +import EmptyState from '~/monitoring/components/empty_state.vue'; + +function createComponent(props) { + return shallowMount(EmptyState, { + propsData: { + ...props, + settingsPath: '/settingsPath', + clustersPath: '/clustersPath', + documentationPath: '/documentationPath', + emptyGettingStartedSvgPath: '/path/to/getting-started.svg', + emptyLoadingSvgPath: '/path/to/loading.svg', + emptyNoDataSvgPath: '/path/to/no-data.svg', + emptyUnableToConnectSvgPath: '/path/to/unable-to-connect.svg', + }, + }); +} + +describe('EmptyState', () => { + it('shows gettingStarted state', () => { + const wrapper = createComponent({ + selectedState: 'gettingStarted', + }); + + expect(wrapper.element).toMatchSnapshot(); + }); + + it('shows loading state', () => { + const wrapper = createComponent({ + selectedState: 'loading', + }); + + expect(wrapper.element).toMatchSnapshot(); + }); + + it('shows unableToConnect state', () => { + const wrapper = createComponent({ + selectedState: 'unableToConnect', + }); + + expect(wrapper.element).toMatchSnapshot(); + }); +}); diff --git a/spec/helpers/preferences_helper_spec.rb b/spec/helpers/preferences_helper_spec.rb index db0d45c3692..554c08add2d 100644 --- a/spec/helpers/preferences_helper_spec.rb +++ b/spec/helpers/preferences_helper_spec.rb @@ -28,7 +28,7 @@ describe PreferencesHelper do ["Your Projects' Activity", 'project_activity'], ["Starred Projects' Activity", 'starred_project_activity'], ["Your Groups", 'groups'], - ["Your Todos", 'todos'], + ["Your To-Do List", 'todos'], ["Assigned Issues", 'issues'], ["Assigned Merge Requests", 'merge_requests'] ] diff --git a/spec/javascripts/boards/mock_data.js b/spec/javascripts/boards/mock_data.js index 9854cf49e97..ea22ae5c4e7 100644 --- a/spec/javascripts/boards/mock_data.js +++ b/spec/javascripts/boards/mock_data.js @@ -1,4 +1,5 @@ import BoardService from '~/boards/services/board_service'; +import boardsStore from '~/boards/stores/boards_store'; export const boardObj = { id: 1, @@ -76,12 +77,14 @@ export const mockBoardService = (opts = {}) => { const bulkUpdatePath = opts.bulkUpdatePath || ''; const boardId = opts.boardId || '1'; - return new BoardService({ + boardsStore.setEndpoints({ boardsEndpoint, listsEndpoint, bulkUpdatePath, boardId, }); + + return new BoardService(); }; export const mockAssigneesList = [ diff --git a/spec/javascripts/collapsed_sidebar_todo_spec.js b/spec/javascripts/collapsed_sidebar_todo_spec.js index bb90e53e525..f75d63c8f57 100644 --- a/spec/javascripts/collapsed_sidebar_todo_spec.js +++ b/spec/javascripts/collapsed_sidebar_todo_spec.js @@ -58,7 +58,7 @@ describe('Issuable right sidebar collapsed todo toggle', () => { it('sets default tooltip title', () => { expect( document.querySelector('.js-issuable-todo.sidebar-collapsed-icon').getAttribute('title'), - ).toBe('Add todo'); + ).toBe('Add a To Do'); }); it('toggle todo state', done => { @@ -85,7 +85,7 @@ describe('Issuable right sidebar collapsed todo toggle', () => { setTimeout(() => { expect( document.querySelector('.issuable-sidebar-header .js-issuable-todo').textContent.trim(), - ).toBe('Mark todo as done'); + ).toBe('Mark as done'); done(); }); @@ -99,7 +99,7 @@ describe('Issuable right sidebar collapsed todo toggle', () => { document .querySelector('.js-issuable-todo.sidebar-collapsed-icon') .getAttribute('data-original-title'), - ).toBe('Mark todo as done'); + ).toBe('Mark as done'); done(); }); @@ -124,13 +124,13 @@ describe('Issuable right sidebar collapsed todo toggle', () => { expect( document.querySelector('.issuable-sidebar-header .js-issuable-todo').textContent.trim(), - ).toBe('Add todo'); + ).toBe('Add a To Do'); }) .then(done) .catch(done.fail); }); - it('updates aria-label to mark todo as done', done => { + it('updates aria-label to Mark as done', done => { document.querySelector('.js-issuable-todo.sidebar-collapsed-icon').click(); setTimeout(() => { @@ -138,7 +138,7 @@ describe('Issuable right sidebar collapsed todo toggle', () => { document .querySelector('.js-issuable-todo.sidebar-collapsed-icon') .getAttribute('aria-label'), - ).toBe('Mark todo as done'); + ).toBe('Mark as done'); done(); }); @@ -153,7 +153,7 @@ describe('Issuable right sidebar collapsed todo toggle', () => { document .querySelector('.js-issuable-todo.sidebar-collapsed-icon') .getAttribute('aria-label'), - ).toBe('Mark todo as done'); + ).toBe('Mark as done'); document.querySelector('.js-issuable-todo.sidebar-collapsed-icon').click(); }) @@ -163,7 +163,7 @@ describe('Issuable right sidebar collapsed todo toggle', () => { document .querySelector('.js-issuable-todo.sidebar-collapsed-icon') .getAttribute('aria-label'), - ).toBe('Add todo'); + ).toBe('Add a To Do'); }) .then(done) .catch(done.fail); diff --git a/spec/javascripts/environments/environment_terminal_button_spec.js b/spec/javascripts/environments/environment_terminal_button_spec.js index 56e18db59c5..fc98e656efe 100644 --- a/spec/javascripts/environments/environment_terminal_button_spec.js +++ b/spec/javascripts/environments/environment_terminal_button_spec.js @@ -12,36 +12,24 @@ describe('Stop Component', () => { }).$mount(); }; - describe('enabled', () => { - beforeEach(() => { - mountWithProps({ terminalPath }); - }); - - describe('computed', () => { - it('title', () => { - expect(component.title).toEqual('Terminal'); - }); - }); - - it('should render a link to open a web terminal with the provided path', () => { - expect(component.$el.tagName).toEqual('A'); - expect(component.$el.getAttribute('data-original-title')).toEqual('Terminal'); - expect(component.$el.getAttribute('aria-label')).toEqual('Terminal'); - expect(component.$el.getAttribute('href')).toEqual(terminalPath); - }); + beforeEach(() => { + mountWithProps({ terminalPath }); + }); - it('should render a non-disabled button', () => { - expect(component.$el.classList).not.toContain('disabled'); + describe('computed', () => { + it('title', () => { + expect(component.title).toEqual('Terminal'); }); }); - describe('disabled', () => { - beforeEach(() => { - mountWithProps({ terminalPath, disabled: true }); - }); + it('should render a link to open a web terminal with the provided path', () => { + expect(component.$el.tagName).toEqual('A'); + expect(component.$el.getAttribute('data-original-title')).toEqual('Terminal'); + expect(component.$el.getAttribute('aria-label')).toEqual('Terminal'); + expect(component.$el.getAttribute('href')).toEqual(terminalPath); + }); - it('should render a disabled button', () => { - expect(component.$el.classList).toContain('disabled'); - }); + it('should render a non-disabled button', () => { + expect(component.$el.classList).not.toContain('disabled'); }); }); diff --git a/spec/javascripts/monitoring/dashboard_state_spec.js b/spec/javascripts/monitoring/dashboard_state_spec.js deleted file mode 100644 index 6b2be83aa8c..00000000000 --- a/spec/javascripts/monitoring/dashboard_state_spec.js +++ /dev/null @@ -1,101 +0,0 @@ -import Vue from 'vue'; -import EmptyState from '~/monitoring/components/empty_state.vue'; -import { statePaths } from './mock_data'; - -function createComponent(props) { - const Component = Vue.extend(EmptyState); - - return new Component({ - propsData: { - ...props, - settingsPath: statePaths.settingsPath, - clustersPath: statePaths.clustersPath, - documentationPath: statePaths.documentationPath, - emptyGettingStartedSvgPath: '/path/to/getting-started.svg', - emptyLoadingSvgPath: '/path/to/loading.svg', - emptyNoDataSvgPath: '/path/to/no-data.svg', - emptyUnableToConnectSvgPath: '/path/to/unable-to-connect.svg', - }, - }).$mount(); -} - -function getTextFromNode(component, selector) { - return component.$el.querySelector(selector).firstChild.nodeValue.trim(); -} - -describe('EmptyState', () => { - describe('Computed props', () => { - it('currentState', () => { - const component = createComponent({ - selectedState: 'gettingStarted', - }); - - expect(component.currentState).toBe(component.states.gettingStarted); - }); - - it('showButtonDescription returns a description with a link for the unableToConnect state', () => { - const component = createComponent({ - selectedState: 'unableToConnect', - }); - - expect(component.showButtonDescription).toEqual(true); - }); - - it('showButtonDescription returns the description without a link for any other state', () => { - const component = createComponent({ - selectedState: 'loading', - }); - - expect(component.showButtonDescription).toEqual(false); - }); - }); - - it('should show the gettingStarted state', () => { - const component = createComponent({ - selectedState: 'gettingStarted', - }); - - expect(component.$el.querySelector('svg')).toBeDefined(); - expect(getTextFromNode(component, '.state-title')).toEqual( - component.states.gettingStarted.title, - ); - - expect(getTextFromNode(component, '.state-description')).toEqual( - component.states.gettingStarted.description, - ); - - expect(getTextFromNode(component, '.btn-success')).toEqual( - component.states.gettingStarted.buttonText, - ); - }); - - it('should show the loading state', () => { - const component = createComponent({ - selectedState: 'loading', - }); - - expect(component.$el.querySelector('svg')).toBeDefined(); - expect(getTextFromNode(component, '.state-title')).toEqual(component.states.loading.title); - expect(getTextFromNode(component, '.state-description')).toEqual( - component.states.loading.description, - ); - - expect(getTextFromNode(component, '.btn-success')).toEqual(component.states.loading.buttonText); - }); - - it('should show the unableToConnect state', () => { - const component = createComponent({ - selectedState: 'unableToConnect', - }); - - expect(component.$el.querySelector('svg')).toBeDefined(); - expect(getTextFromNode(component, '.state-title')).toEqual( - component.states.unableToConnect.title, - ); - - expect(component.$el.querySelector('.state-description a')).toBeDefined(); - expect(getTextFromNode(component, '.btn-success')).toEqual( - component.states.unableToConnect.buttonText, - ); - }); -}); diff --git a/spec/javascripts/sidebar/todo_spec.js b/spec/javascripts/sidebar/todo_spec.js index f46ea5a0499..e7abd19c865 100644 --- a/spec/javascripts/sidebar/todo_spec.js +++ b/spec/javascripts/sidebar/todo_spec.js @@ -53,14 +53,14 @@ describe('SidebarTodo', () => { describe('buttonLabel', () => { it('returns todo button text for marking todo as done when `isTodo` prop is `true`', () => { - expect(vm.buttonLabel).toBe('Mark todo as done'); + expect(vm.buttonLabel).toBe('Mark as done'); }); it('returns todo button text for add todo when `isTodo` prop is `false`', done => { vm.isTodo = false; Vue.nextTick() .then(() => { - expect(vm.buttonLabel).toBe('Add todo'); + expect(vm.buttonLabel).toBe('Add a To Do'); }) .then(done) .catch(done.fail); @@ -131,14 +131,14 @@ describe('SidebarTodo', () => { }); it('check button label computed property', () => { - expect(vm.buttonLabel).toEqual('Mark todo as done'); + expect(vm.buttonLabel).toEqual('Mark as done'); }); it('renders button label element when `collapsed` prop is `false`', () => { const buttonLabelEl = vm.$el.querySelector('span.issuable-todo-inner'); expect(buttonLabelEl).not.toBeNull(); - expect(buttonLabelEl.innerText.trim()).toBe('Mark todo as done'); + expect(buttonLabelEl.innerText.trim()).toBe('Mark as done'); }); it('renders button icon when `collapsed` prop is `true`', done => { diff --git a/spec/lib/feature_spec.rb b/spec/lib/feature_spec.rb index 403e0785d1b..127463a57e8 100644 --- a/spec/lib/feature_spec.rb +++ b/spec/lib/feature_spec.rb @@ -144,6 +144,68 @@ describe Feature do expect(described_class.enabled?(:enabled_feature_flag)).to be_truthy end + it { expect(described_class.l1_cache_backend).to eq(Gitlab::ThreadMemoryCache.cache_backend) } + it { expect(described_class.l2_cache_backend).to eq(Rails.cache) } + + it 'caches the status in L1 and L2 caches', + :request_store, :use_clean_rails_memory_store_caching do + described_class.enable(:enabled_feature_flag) + flipper_key = "flipper/v1/feature/enabled_feature_flag" + + expect(described_class.l2_cache_backend) + .to receive(:fetch) + .once + .with(flipper_key, expires_in: 1.hour) + .and_call_original + + expect(described_class.l1_cache_backend) + .to receive(:fetch) + .once + .with(flipper_key, expires_in: 1.minute) + .and_call_original + + 2.times do + expect(described_class.enabled?(:enabled_feature_flag)).to be_truthy + end + end + + context 'cached feature flag', :request_store do + let(:flag) { :some_feature_flag } + + before do + described_class.flipper.memoize = false + described_class.enabled?(flag) + end + + it 'caches the status in L1 cache for the first minute' do + expect do + expect(described_class.l1_cache_backend).to receive(:fetch).once.and_call_original + expect(described_class.l2_cache_backend).not_to receive(:fetch) + expect(described_class.enabled?(flag)).to be_truthy + end.not_to exceed_query_limit(0) + end + + it 'caches the status in L2 cache after 2 minutes' do + Timecop.travel 2.minutes do + expect do + expect(described_class.l1_cache_backend).to receive(:fetch).once.and_call_original + expect(described_class.l2_cache_backend).to receive(:fetch).once.and_call_original + expect(described_class.enabled?(flag)).to be_truthy + end.not_to exceed_query_limit(0) + end + end + + it 'fetches the status after an hour' do + Timecop.travel 61.minutes do + expect do + expect(described_class.l1_cache_backend).to receive(:fetch).once.and_call_original + expect(described_class.l2_cache_backend).to receive(:fetch).once.and_call_original + expect(described_class.enabled?(flag)).to be_truthy + end.not_to exceed_query_limit(1) + end + end + end + context 'with an individual actor' do CustomActor = Struct.new(:flipper_id) diff --git a/spec/lib/gitlab/ci/reports/test_reports_comparer_spec.rb b/spec/lib/gitlab/ci/reports/test_reports_comparer_spec.rb index 71c61e0345f..36582204cc1 100644 --- a/spec/lib/gitlab/ci/reports/test_reports_comparer_spec.rb +++ b/spec/lib/gitlab/ci/reports/test_reports_comparer_spec.rb @@ -74,17 +74,11 @@ describe Gitlab::Ci::Reports::TestReportsComparer do subject { comparer.resolved_count } context 'when there is a resolved test case in head suites' do - let(:create_test_case_java_resolved) do - create_test_case_java_failed.tap do |test_case| - test_case.instance_variable_set("@status", Gitlab::Ci::Reports::TestCase::STATUS_SUCCESS) - end - end - before do base_reports.get_suite('rspec').add_test_case(create_test_case_rspec_success) base_reports.get_suite('junit').add_test_case(create_test_case_java_failed) head_reports.get_suite('rspec').add_test_case(create_test_case_rspec_success) - head_reports.get_suite('junit').add_test_case(create_test_case_java_resolved) + head_reports.get_suite('junit').add_test_case(create_test_case_java_success) end it 'returns the correct count' do diff --git a/spec/lib/gitlab/ci/reports/test_suite_comparer_spec.rb b/spec/lib/gitlab/ci/reports/test_suite_comparer_spec.rb index 6ab16e5518d..579b3e6fd24 100644 --- a/spec/lib/gitlab/ci/reports/test_suite_comparer_spec.rb +++ b/spec/lib/gitlab/ci/reports/test_suite_comparer_spec.rb @@ -10,12 +10,6 @@ describe Gitlab::Ci::Reports::TestSuiteComparer do let(:test_case_success) { create_test_case_rspec_success } let(:test_case_failed) { create_test_case_rspec_failed } - let(:test_case_resolved) do - create_test_case_rspec_failed.tap do |test_case| - test_case.instance_variable_set("@status", Gitlab::Ci::Reports::TestCase::STATUS_SUCCESS) - end - end - describe '#new_failures' do subject { comparer.new_failures } @@ -44,7 +38,7 @@ describe Gitlab::Ci::Reports::TestSuiteComparer do context 'when head sutie has a success test case which failed in base' do before do base_suite.add_test_case(test_case_failed) - head_suite.add_test_case(test_case_resolved) + head_suite.add_test_case(test_case_success) end it 'does not return the failed test case' do @@ -81,7 +75,7 @@ describe Gitlab::Ci::Reports::TestSuiteComparer do context 'when head sutie has a success test case which failed in base' do before do base_suite.add_test_case(test_case_failed) - head_suite.add_test_case(test_case_resolved) + head_suite.add_test_case(test_case_success) end it 'does not return the failed test case' do @@ -126,11 +120,11 @@ describe Gitlab::Ci::Reports::TestSuiteComparer do context 'when head sutie has a success test case which failed in base' do before do base_suite.add_test_case(test_case_failed) - head_suite.add_test_case(test_case_resolved) + head_suite.add_test_case(test_case_success) end it 'does not return the resolved test case' do - is_expected.to eq([test_case_resolved]) + is_expected.to eq([test_case_success]) end it 'returns the correct resolved count' do @@ -156,13 +150,8 @@ describe Gitlab::Ci::Reports::TestSuiteComparer do context 'when there are a new failure and an existing failure' do let(:test_case_1_success) { create_test_case_rspec_success } - let(:test_case_2_failed) { create_test_case_rspec_failed } - - let(:test_case_1_failed) do - create_test_case_rspec_success.tap do |test_case| - test_case.instance_variable_set("@status", Gitlab::Ci::Reports::TestCase::STATUS_FAILED) - end - end + let(:test_case_1_failed) { create_test_case_rspec_failed } + let(:test_case_2_failed) { create_test_case_rspec_failed('case2') } before do base_suite.add_test_case(test_case_1_success) diff --git a/spec/lib/gitlab/danger/helper_spec.rb b/spec/lib/gitlab/danger/helper_spec.rb index c8e65a3e59d..92d90ac2fef 100644 --- a/spec/lib/gitlab/danger/helper_spec.rb +++ b/spec/lib/gitlab/danger/helper_spec.rb @@ -87,13 +87,13 @@ describe Gitlab::Danger::Helper do describe '#changes_by_category' do it 'categorizes changed files' do - expect(fake_git).to receive(:added_files) { %w[foo foo.md foo.rb foo.js db/foo qa/foo ee/changelogs/foo.yml] } + expect(fake_git).to receive(:added_files) { %w[foo foo.md foo.rb foo.js db/foo lib/gitlab/database/foo.rb qa/foo ee/changelogs/foo.yml] } allow(fake_git).to receive(:modified_files) { [] } allow(fake_git).to receive(:renamed_files) { [] } expect(helper.changes_by_category).to eq( backend: %w[foo.rb], - database: %w[db/foo], + database: %w[db/foo lib/gitlab/database/foo.rb], frontend: %w[foo.js], none: %w[ee/changelogs/foo.yml foo.md], qa: %w[qa/foo], @@ -159,9 +159,22 @@ describe Gitlab::Danger::Helper do 'ee/FOO_VERSION' | :unknown - 'db/foo' | :database - 'qa/foo' | :qa + 'db/foo' | :database + 'ee/db/foo' | :database + 'app/models/project_authorization.rb' | :database + 'app/services/users/refresh_authorized_projects_service.rb' | :database + 'lib/gitlab/background_migration.rb' | :database + 'lib/gitlab/background_migration/foo' | :database + 'ee/lib/gitlab/background_migration/foo' | :database + 'lib/gitlab/database.rb' | :database + 'lib/gitlab/database/foo' | :database + 'ee/lib/gitlab/database/foo' | :database + 'lib/gitlab/github_import.rb' | :database + 'lib/gitlab/github_import/foo' | :database + 'lib/gitlab/sql/foo' | :database + 'rubocop/cop/migration/foo' | :database + 'qa/foo' | :qa 'ee/qa/foo' | :qa 'changelogs/foo' | :none diff --git a/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb b/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb index aaf8c9fa2a0..4d93b70e6e3 100644 --- a/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb +++ b/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb @@ -8,12 +8,19 @@ describe Gitlab::Metrics::Samplers::RubySampler do allow(Gitlab::Metrics::NullMetric).to receive(:instance).and_return(null_metric) end + describe '#initialize' do + it 'sets process_start_time_seconds' do + Timecop.freeze do + expect(sampler.metrics[:process_start_time_seconds].get).to eq(Time.now.to_i) + end + end + end + describe '#sample' do it 'samples various statistics' do expect(Gitlab::Metrics::System).to receive(:cpu_time) expect(Gitlab::Metrics::System).to receive(:file_descriptor_count) expect(Gitlab::Metrics::System).to receive(:memory_usage) - expect(Gitlab::Metrics::System).to receive(:process_start_time) expect(Gitlab::Metrics::System).to receive(:max_open_file_descriptors) expect(sampler).to receive(:sample_gc) @@ -44,13 +51,6 @@ describe Gitlab::Metrics::Samplers::RubySampler do sampler.sample end - it 'adds a metric containing the process start time' do - expect(Gitlab::Metrics::System).to receive(:process_start_time).and_return(12345) - expect(sampler.metrics[:process_start_time_seconds]).to receive(:set).with({}, 12345) - - sampler.sample - end - it 'adds a metric containing the process max file descriptors' do expect(Gitlab::Metrics::System).to receive(:max_open_file_descriptors).and_return(1024) expect(sampler.metrics[:process_max_fds]).to receive(:set).with({}, 1024) diff --git a/spec/lib/gitlab/metrics/system_spec.rb b/spec/lib/gitlab/metrics/system_spec.rb index da87df15746..3b434a02f63 100644 --- a/spec/lib/gitlab/metrics/system_spec.rb +++ b/spec/lib/gitlab/metrics/system_spec.rb @@ -19,12 +19,6 @@ describe Gitlab::Metrics::System do expect(described_class.max_open_file_descriptors).to be > 0 end end - - describe '.process_start_time' do - it 'returns the process start time' do - expect(described_class.process_start_time).to be > 0 - end - end else describe '.memory_usage' do it 'returns 0.0' do @@ -43,12 +37,6 @@ describe Gitlab::Metrics::System do expect(described_class.max_open_file_descriptors).to eq(0) end end - - describe 'process_start_time' do - it 'returns 0' do - expect(described_class.process_start_time).to eq(0) - end - end end describe '.cpu_time' do diff --git a/spec/lib/gitlab/performance_bar_spec.rb b/spec/lib/gitlab/performance_bar_spec.rb index f480376acb4..ee3c571c9c0 100644 --- a/spec/lib/gitlab/performance_bar_spec.rb +++ b/spec/lib/gitlab/performance_bar_spec.rb @@ -3,17 +3,42 @@ require 'spec_helper' describe Gitlab::PerformanceBar do shared_examples 'allowed user IDs are cached' do before do - # Warm the Redis cache + # Warm the caches described_class.enabled?(user) end it 'caches the allowed user IDs in cache', :use_clean_rails_memory_store_caching do expect do + expect(described_class.l1_cache_backend).to receive(:fetch).and_call_original + expect(described_class.l2_cache_backend).not_to receive(:fetch) expect(described_class.enabled?(user)).to be_truthy end.not_to exceed_query_limit(0) end + + it 'caches the allowed user IDs in L1 cache for 1 minute', :use_clean_rails_memory_store_caching do + Timecop.travel 2.minutes do + expect do + expect(described_class.l1_cache_backend).to receive(:fetch).and_call_original + expect(described_class.l2_cache_backend).to receive(:fetch).and_call_original + expect(described_class.enabled?(user)).to be_truthy + end.not_to exceed_query_limit(0) + end + end + + it 'caches the allowed user IDs in L2 cache for 5 minutes', :use_clean_rails_memory_store_caching do + Timecop.travel 6.minutes do + expect do + expect(described_class.l1_cache_backend).to receive(:fetch).and_call_original + expect(described_class.l2_cache_backend).to receive(:fetch).and_call_original + expect(described_class.enabled?(user)).to be_truthy + end.not_to exceed_query_limit(2) + end + end end + it { expect(described_class.l1_cache_backend).to eq(Gitlab::ThreadMemoryCache.cache_backend) } + it { expect(described_class.l2_cache_backend).to eq(Rails.cache) } + describe '.enabled?' do let(:user) { create(:user) } diff --git a/spec/lib/gitlab/user_extractor_spec.rb b/spec/lib/gitlab/user_extractor_spec.rb deleted file mode 100644 index b86ec5445b8..00000000000 --- a/spec/lib/gitlab/user_extractor_spec.rb +++ /dev/null @@ -1,78 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -describe Gitlab::UserExtractor do - let(:text) do - <<~TXT - This is a long texth that mentions some users. - @user-1, @user-2 and user@gitlab.org take a walk in the park. - There they meet @user-4 that was out with other-user@gitlab.org. - @user-1 thought it was late, so went home straight away - TXT - end - subject(:extractor) { described_class.new(text) } - - describe '#users' do - it 'returns an empty relation when nil was passed' do - extractor = described_class.new(nil) - - expect(extractor.users).to be_empty - expect(extractor.users).to be_a(ActiveRecord::Relation) - end - - it 'returns the user case insensitive for usernames' do - user = create(:user, username: "USER-4") - - expect(extractor.users).to include(user) - end - - it 'returns users by primary email' do - user = create(:user, email: 'user@gitlab.org') - - expect(extractor.users).to include(user) - end - - it 'returns users by secondary email' do - user = create(:email, email: 'other-user@gitlab.org').user - - expect(extractor.users).to include(user) - end - - context 'input as array of strings' do - it 'is treated as one string' do - extractor = described_class.new(text.lines) - - user_1 = create(:user, username: "USER-1") - user_4 = create(:user, username: "USER-4") - user_email = create(:user, email: 'user@gitlab.org') - - expect(extractor.users).to contain_exactly(user_1, user_4, user_email) - end - end - end - - describe '#matches' do - it 'includes all mentioned email adresses' do - expect(extractor.matches[:emails]).to contain_exactly('user@gitlab.org', 'other-user@gitlab.org') - end - - it 'includes all mentioned usernames' do - expect(extractor.matches[:usernames]).to contain_exactly('user-1', 'user-2', 'user-4') - end - - context 'input has no matching e-mail or usernames' do - it 'returns an empty list of users' do - extractor = described_class.new('My test') - - expect(extractor.users).to be_empty - end - end - end - - describe '#references' do - it 'includes all user-references once' do - expect(extractor.references).to contain_exactly('user-1', 'user-2', 'user@gitlab.org', 'user-4', 'other-user@gitlab.org') - end - end -end diff --git a/spec/migrations/backfill_store_project_full_path_in_repo_spec.rb b/spec/migrations/backfill_store_project_full_path_in_repo_spec.rb index 34f4a36d63d..65a918d5440 100644 --- a/spec/migrations/backfill_store_project_full_path_in_repo_spec.rb +++ b/spec/migrations/backfill_store_project_full_path_in_repo_spec.rb @@ -13,7 +13,7 @@ describe BackfillStoreProjectFullPathInRepo, :migration do subject(:migration) { described_class.new } around do |example| - Sidekiq::Testing.inline! do + perform_enqueued_jobs do example.run end end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 55cea48b641..e24bbc39761 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -2998,4 +2998,28 @@ describe Ci::Pipeline, :mailer do end end end + + describe '#error_messages' do + subject { pipeline.error_messages } + + before do + pipeline.valid? + end + + context 'when pipeline has errors' do + let(:pipeline) { build(:ci_pipeline, sha: nil, ref: nil) } + + it 'returns the full error messages' do + is_expected.to eq("Sha can't be blank and Ref can't be blank") + end + end + + context 'when pipeline does not have errors' do + let(:pipeline) { build(:ci_pipeline) } + + it 'returns empty string' do + is_expected.to be_empty + end + end + end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index a2547755510..fe6d68aff3f 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -7,6 +7,8 @@ describe MergeRequest do include ProjectForksHelper include ReactiveCachingHelpers + using RSpec::Parameterized::TableSyntax + subject { create(:merge_request) } describe 'associations' do @@ -1996,6 +1998,47 @@ describe MergeRequest do end end + describe '#rebase_async' do + let(:merge_request) { create(:merge_request) } + let(:user_id) { double(:user_id) } + let(:rebase_jid) { 'rebase-jid' } + + subject(:execute) { merge_request.rebase_async(user_id) } + + it 'atomically enqueues a RebaseWorker job and updates rebase_jid' do + expect(RebaseWorker) + .to receive(:perform_async) + .with(merge_request.id, user_id) + .and_return(rebase_jid) + + expect(merge_request).to receive(:lock!).and_call_original + + execute + + expect(merge_request.rebase_jid).to eq(rebase_jid) + end + + it 'refuses to enqueue a job if a rebase is in progress' do + merge_request.update_column(:rebase_jid, rebase_jid) + + expect(RebaseWorker).not_to receive(:perform_async) + expect(Gitlab::SidekiqStatus) + .to receive(:running?) + .with(rebase_jid) + .and_return(true) + + expect { execute }.to raise_error(ActiveRecord::StaleObjectError) + end + + it 'refuses to enqueue a job if the MR is not open' do + merge_request.update_column(:state, 'foo') + + expect(RebaseWorker).not_to receive(:perform_async) + + expect { execute }.to raise_error(ActiveRecord::StaleObjectError) + end + end + describe '#mergeable?' do let(:project) { create(:project) } @@ -2946,40 +2989,64 @@ describe MergeRequest do end describe '#rebase_in_progress?' do - shared_examples 'checking whether a rebase is in progress' do - let(:repo_path) do - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - subject.source_project.repository.path - end - end - let(:rebase_path) { File.join(repo_path, "gitlab-worktree", "rebase-#{subject.id}") } + where(:rebase_jid, :jid_valid, :result) do + 'foo' | true | true + 'foo' | false | false + '' | true | false + nil | true | false + end - before do - system(*%W(#{Gitlab.config.git.bin_path} -C #{repo_path} worktree add --detach #{rebase_path} master)) + with_them do + let(:merge_request) { create(:merge_request) } + + subject { merge_request.rebase_in_progress? } + + it do + # Stub out the legacy gitaly implementation + allow(merge_request).to receive(:gitaly_rebase_in_progress?) { false } + + allow(Gitlab::SidekiqStatus).to receive(:running?).with(rebase_jid) { jid_valid } + + merge_request.rebase_jid = rebase_jid + + is_expected.to eq(result) end + end + end - it 'returns true when there is a current rebase directory' do - expect(subject.rebase_in_progress?).to be_truthy + describe '#gitaly_rebase_in_progress?' do + let(:repo_path) do + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + subject.source_project.repository.path end + end + let(:rebase_path) { File.join(repo_path, "gitlab-worktree", "rebase-#{subject.id}") } + + before do + system(*%W(#{Gitlab.config.git.bin_path} -C #{repo_path} worktree add --detach #{rebase_path} master)) + end - it 'returns false when there is no rebase directory' do - FileUtils.rm_rf(rebase_path) + it 'returns true when there is a current rebase directory' do + expect(subject.rebase_in_progress?).to be_truthy + end - expect(subject.rebase_in_progress?).to be_falsey - end + it 'returns false when there is no rebase directory' do + FileUtils.rm_rf(rebase_path) - it 'returns false when the rebase directory has expired' do - time = 20.minutes.ago.to_time - File.utime(time, time, rebase_path) + expect(subject.rebase_in_progress?).to be_falsey + end - expect(subject.rebase_in_progress?).to be_falsey - end + it 'returns false when the rebase directory has expired' do + time = 20.minutes.ago.to_time + File.utime(time, time, rebase_path) - it 'returns false when the source project has been removed' do - allow(subject).to receive(:source_project).and_return(nil) + expect(subject.rebase_in_progress?).to be_falsey + end - expect(subject.rebase_in_progress?).to be_falsey - end + it 'returns false when the source project has been removed' do + allow(subject).to receive(:source_project).and_return(nil) + + expect(subject.rebase_in_progress?).to be_falsey end end diff --git a/spec/models/namespace/aggregation_schedule_spec.rb b/spec/models/namespace/aggregation_schedule_spec.rb index 8ed0248e1b2..0f1283717e0 100644 --- a/spec/models/namespace/aggregation_schedule_spec.rb +++ b/spec/models/namespace/aggregation_schedule_spec.rb @@ -53,10 +53,39 @@ RSpec.describe Namespace::AggregationSchedule, :clean_gitlab_redis_shared_state, expect(Namespaces::RootStatisticsWorker) .to receive(:perform_in).once - .with(described_class::DEFAULT_LEASE_TIMEOUT, aggregation_schedule.namespace_id ) + .with(described_class::DEFAULT_LEASE_TIMEOUT, aggregation_schedule.namespace_id) aggregation_schedule.save! end + + it 'does not release the lease' do + stub_exclusive_lease(lease_key, timeout: described_class::DEFAULT_LEASE_TIMEOUT) + + aggregation_schedule.save! + + exclusive_lease = aggregation_schedule.exclusive_lease + expect(exclusive_lease.exists?).to be_truthy + end + + it 'only executes the workers once' do + # Avoid automatic deletion of Namespace::AggregationSchedule + # for testing purposes. + expect(Namespaces::RootStatisticsWorker) + .to receive(:perform_async).once + .and_return(nil) + + expect(Namespaces::RootStatisticsWorker) + .to receive(:perform_in).once + .with(described_class::DEFAULT_LEASE_TIMEOUT, aggregation_schedule.namespace_id) + .and_return(nil) + + # Scheduling workers for the first time + aggregation_schedule.schedule_root_storage_statistics + + # Executing again, this time workers should not be scheduled + # due to the lease not been released. + aggregation_schedule.schedule_root_storage_statistics + end end context 'with a personalized lease timeout' do diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index a82ecb4fd63..ced853caab4 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -2033,6 +2033,9 @@ describe API::MergeRequests do expect(response).to have_gitlab_http_status(202) expect(RebaseWorker.jobs.size).to eq(1) + + expect(merge_request.reload).to be_rebase_in_progress + expect(json_response['rebase_in_progress']).to be(true) end it 'returns 403 if the user cannot push to the branch' do @@ -2043,6 +2046,16 @@ describe API::MergeRequests do expect(response).to have_gitlab_http_status(403) end + + it 'returns 409 if a rebase is already in progress' do + Sidekiq::Testing.fake! do + merge_request.rebase_async(user.id) + + put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/rebase", user) + end + + expect(response).to have_gitlab_http_status(409) + end end describe 'Time tracking' do diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 5f7d2fa6d9c..c67412a44c1 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -2428,7 +2428,7 @@ describe API::Projects do let(:housekeeping) { Projects::HousekeepingService.new(project) } before do - allow(Projects::HousekeepingService).to receive(:new).with(project).and_return(housekeeping) + allow(Projects::HousekeepingService).to receive(:new).with(project, :gc).and_return(housekeeping) end context 'when authenticated as owner' do diff --git a/spec/serializers/test_case_entity_spec.rb b/spec/serializers/test_case_entity_spec.rb index 986c9feb07b..cc5f086ca4e 100644 --- a/spec/serializers/test_case_entity_spec.rb +++ b/spec/serializers/test_case_entity_spec.rb @@ -24,7 +24,7 @@ describe TestCaseEntity do it 'contains correct test case details' do expect(subject[:status]).to eq('failed') - expect(subject[:name]).to eq('Test#sum when a is 2 and b is 2 returns summary') + expect(subject[:name]).to eq('Test#sum when a is 1 and b is 3 returns summary') expect(subject[:classname]).to eq('spec.test_spec') expect(subject[:execution_time]).to eq(2.22) end diff --git a/spec/serializers/test_reports_comparer_entity_spec.rb b/spec/serializers/test_reports_comparer_entity_spec.rb index 59c058fe368..4a951bbbde4 100644 --- a/spec/serializers/test_reports_comparer_entity_spec.rb +++ b/spec/serializers/test_reports_comparer_entity_spec.rb @@ -53,13 +53,7 @@ describe TestReportsComparerEntity do base_reports.get_suite('rspec').add_test_case(create_test_case_rspec_success) base_reports.get_suite('junit').add_test_case(create_test_case_java_failed) head_reports.get_suite('rspec').add_test_case(create_test_case_rspec_success) - head_reports.get_suite('junit').add_test_case(create_test_case_java_resolved) - end - - let(:create_test_case_java_resolved) do - create_test_case_java_failed.tap do |test_case| - test_case.instance_variable_set("@status", Gitlab::Ci::Reports::TestCase::STATUS_SUCCESS) - end + head_reports.get_suite('junit').add_test_case(create_test_case_java_success) end it 'contains correct compared test reports details' do diff --git a/spec/serializers/test_reports_comparer_serializer_spec.rb b/spec/serializers/test_reports_comparer_serializer_spec.rb index 9ea86c0dd83..62dc6f486c5 100644 --- a/spec/serializers/test_reports_comparer_serializer_spec.rb +++ b/spec/serializers/test_reports_comparer_serializer_spec.rb @@ -44,13 +44,7 @@ describe TestReportsComparerSerializer do base_reports.get_suite('rspec').add_test_case(create_test_case_rspec_success) base_reports.get_suite('junit').add_test_case(create_test_case_java_failed) head_reports.get_suite('rspec').add_test_case(create_test_case_rspec_success) - head_reports.get_suite('junit').add_test_case(create_test_case_java_resolved) - end - - let(:create_test_case_java_resolved) do - create_test_case_java_failed.tap do |test_case| - test_case.instance_variable_set("@status", Gitlab::Ci::Reports::TestCase::STATUS_SUCCESS) - end + head_reports.get_suite('junit').add_test_case(create_test_case_java_success) end it 'matches the schema' do diff --git a/spec/serializers/test_suite_comparer_entity_spec.rb b/spec/serializers/test_suite_comparer_entity_spec.rb index f61331f53a0..4b2cca2c68c 100644 --- a/spec/serializers/test_suite_comparer_entity_spec.rb +++ b/spec/serializers/test_suite_comparer_entity_spec.rb @@ -11,16 +11,10 @@ describe TestSuiteComparerEntity do let(:test_case_success) { create_test_case_rspec_success } let(:test_case_failed) { create_test_case_rspec_failed } - let(:test_case_resolved) do - create_test_case_rspec_failed.tap do |test_case| - test_case.instance_variable_set("@status", Gitlab::Ci::Reports::TestCase::STATUS_SUCCESS) - end - end - describe '#as_json' do subject { entity.as_json } - context 'when head sutie has a newly failed test case which does not exist in base' do + context 'when head suite has a newly failed test case which does not exist in base' do before do base_suite.add_test_case(test_case_success) head_suite.add_test_case(test_case_failed) @@ -41,7 +35,7 @@ describe TestSuiteComparerEntity do end end - context 'when head sutie still has a failed test case which failed in base' do + context 'when head suite still has a failed test case which failed in base' do before do base_suite.add_test_case(test_case_failed) head_suite.add_test_case(test_case_failed) @@ -62,10 +56,10 @@ describe TestSuiteComparerEntity do end end - context 'when head sutie has a success test case which failed in base' do + context 'when head suite has a success test case which failed in base' do before do base_suite.add_test_case(test_case_failed) - head_suite.add_test_case(test_case_resolved) + head_suite.add_test_case(test_case_success) end it 'contains correct compared test suite details' do @@ -74,13 +68,83 @@ describe TestSuiteComparerEntity do expect(subject[:summary]).to include(total: 1, resolved: 1, failed: 0) expect(subject[:new_failures]).to be_empty subject[:resolved_failures].first.tap do |resolved_failure| - expect(resolved_failure[:status]).to eq(test_case_resolved.status) - expect(resolved_failure[:name]).to eq(test_case_resolved.name) - expect(resolved_failure[:execution_time]).to eq(test_case_resolved.execution_time) - expect(resolved_failure[:system_output]).to eq(test_case_resolved.system_output) + expect(resolved_failure[:status]).to eq(test_case_success.status) + expect(resolved_failure[:name]).to eq(test_case_success.name) + expect(resolved_failure[:execution_time]).to eq(test_case_success.execution_time) + expect(resolved_failure[:system_output]).to eq(test_case_success.system_output) end expect(subject[:existing_failures]).to be_empty end end + + context 'limits amount of tests returned' do + before do + stub_const('TestSuiteComparerEntity::DEFAULT_MAX_TESTS', 2) + stub_const('TestSuiteComparerEntity::DEFAULT_MIN_TESTS', 1) + end + + context 'prefers new over existing and resolved' do + before do + 3.times { add_new_failure } + 3.times { add_existing_failure } + 3.times { add_resolved_failure } + end + + it 'returns 2 new failures, and 1 of resolved and existing' do + expect(subject[:summary]).to include(total: 9, resolved: 3, failed: 6) + expect(subject[:new_failures].count).to eq(2) + expect(subject[:existing_failures].count).to eq(1) + expect(subject[:resolved_failures].count).to eq(1) + end + end + + context 'prefers existing over resolved' do + before do + 3.times { add_existing_failure } + 3.times { add_resolved_failure } + end + + it 'returns 2 existing failures, and 1 resolved' do + expect(subject[:summary]).to include(total: 6, resolved: 3, failed: 3) + expect(subject[:new_failures].count).to eq(0) + expect(subject[:existing_failures].count).to eq(2) + expect(subject[:resolved_failures].count).to eq(1) + end + end + + context 'limits amount of resolved' do + before do + 3.times { add_resolved_failure } + end + + it 'returns 2 resolved failures' do + expect(subject[:summary]).to include(total: 3, resolved: 3, failed: 0) + expect(subject[:new_failures].count).to eq(0) + expect(subject[:existing_failures].count).to eq(0) + expect(subject[:resolved_failures].count).to eq(2) + end + end + + private + + def add_new_failure + failed_case = create_test_case_rspec_failed(SecureRandom.hex) + head_suite.add_test_case(failed_case) + end + + def add_existing_failure + failed_case = create_test_case_rspec_failed(SecureRandom.hex) + base_suite.add_test_case(failed_case) + head_suite.add_test_case(failed_case) + end + + def add_resolved_failure + case_name = SecureRandom.hex + failed_case = create_test_case_rspec_failed(case_name) + success_case = create_test_case_rspec_success(case_name) + base_suite.add_test_case(failed_case) + head_suite.add_test_case(success_case) + end + end end end diff --git a/spec/services/auto_merge/base_service_spec.rb b/spec/services/auto_merge/base_service_spec.rb index cd08e0b6f32..24cb63a0d61 100644 --- a/spec/services/auto_merge/base_service_spec.rb +++ b/spec/services/auto_merge/base_service_spec.rb @@ -59,6 +59,11 @@ describe AutoMerge::BaseService do context 'when strategy is merge when pipeline succeeds' do let(:service) { AutoMerge::MergeWhenPipelineSucceedsService.new(project, user) } + before do + pipeline = build(:ci_pipeline) + allow(merge_request).to receive(:actual_head_pipeline) { pipeline } + end + it 'sets the auto merge strategy' do subject diff --git a/spec/services/auto_merge/merge_when_pipeline_succeeds_service_spec.rb b/spec/services/auto_merge/merge_when_pipeline_succeeds_service_spec.rb index a20bf8e17e4..5e84ef052ce 100644 --- a/spec/services/auto_merge/merge_when_pipeline_succeeds_service_spec.rb +++ b/spec/services/auto_merge/merge_when_pipeline_succeeds_service_spec.rb @@ -64,8 +64,11 @@ describe AutoMerge::MergeWhenPipelineSucceedsService do end it 'creates a system note' do + pipeline = build(:ci_pipeline) + allow(merge_request).to receive(:actual_head_pipeline) { pipeline } + note = merge_request.notes.last - expect(note.note).to match %r{enabled an automatic merge when the pipeline for (\w+/\w+@)?\h{8}} + expect(note.note).to match "enabled an automatic merge when the pipeline for #{pipeline.sha}" end end diff --git a/spec/services/merge_requests/rebase_service_spec.rb b/spec/services/merge_requests/rebase_service_spec.rb index 7e2f03d1097..ee9caaf2f47 100644 --- a/spec/services/merge_requests/rebase_service_spec.rb +++ b/spec/services/merge_requests/rebase_service_spec.rb @@ -6,10 +6,12 @@ describe MergeRequests::RebaseService do include ProjectForksHelper let(:user) { create(:user) } + let(:rebase_jid) { 'fake-rebase-jid' } let(:merge_request) do - create(:merge_request, + create :merge_request, source_branch: 'feature_conflict', - target_branch: 'master') + target_branch: 'master', + rebase_jid: rebase_jid end let(:project) { merge_request.project } let(:repository) { project.repository.raw } @@ -23,11 +25,11 @@ describe MergeRequests::RebaseService do describe '#execute' do context 'when another rebase is already in progress' do before do - allow(merge_request).to receive(:rebase_in_progress?).and_return(true) + allow(merge_request).to receive(:gitaly_rebase_in_progress?).and_return(true) end it 'saves the error message' do - subject.execute(merge_request) + service.execute(merge_request) expect(merge_request.reload.merge_error).to eq 'Rebase task canceled: Another rebase is already in progress' end @@ -36,6 +38,13 @@ describe MergeRequests::RebaseService do expect(service.execute(merge_request)).to match(status: :error, message: described_class::REBASE_ERROR) end + + it 'clears rebase_jid' do + expect { service.execute(merge_request) } + .to change { merge_request.rebase_jid } + .from(rebase_jid) + .to(nil) + end end shared_examples 'sequence of failure and success' do @@ -43,14 +52,19 @@ describe MergeRequests::RebaseService do allow(repository).to receive(:gitaly_operation_client).and_raise('Something went wrong') service.execute(merge_request) + merge_request.reload - expect(merge_request.reload.merge_error).to eq described_class::REBASE_ERROR + expect(merge_request.reload.merge_error).to eq(described_class::REBASE_ERROR) + expect(merge_request.rebase_jid).to eq(nil) allow(repository).to receive(:gitaly_operation_client).and_call_original + merge_request.update!(rebase_jid: rebase_jid) service.execute(merge_request) + merge_request.reload - expect(merge_request.reload.merge_error).to eq nil + expect(merge_request.merge_error).to eq(nil) + expect(merge_request.rebase_jid).to eq(nil) end end @@ -72,7 +86,7 @@ describe MergeRequests::RebaseService do it 'saves a generic error message' do subject.execute(merge_request) - expect(merge_request.reload.merge_error).to eq described_class::REBASE_ERROR + expect(merge_request.reload.merge_error).to eq(described_class::REBASE_ERROR) end it 'returns an error' do diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index 2a2547f9400..9f60e49290e 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -332,7 +332,7 @@ describe SystemNoteService do create(:merge_request, source_project: project, target_project: project) end - subject { described_class.merge_when_pipeline_succeeds(noteable, project, author, noteable.diff_head_commit) } + subject { described_class.merge_when_pipeline_succeeds(noteable, project, author, pipeline.sha) } it_behaves_like 'a system note' do let(:action) { 'merge' } diff --git a/spec/support/helpers/devise_helpers.rb b/spec/support/helpers/devise_helpers.rb index d32bc2424c0..fb2a110422a 100644 --- a/spec/support/helpers/devise_helpers.rb +++ b/spec/support/helpers/devise_helpers.rb @@ -21,4 +21,16 @@ module DeviseHelpers context.env end end + + def with_omniauth_full_host(&block) + # The OmniAuth `full_host` parameter doesn't get set correctly (it gets set to something like `http://localhost` + # here), and causes integration tests to fail with 404s. We set the `full_host` by removing the request path (and + # anything after it) from the request URI. + omniauth_config_full_host = OmniAuth.config.full_host + OmniAuth.config.full_host = ->(request) { ActionDispatch::Request.new(request).base_url } + + yield + + OmniAuth.config.full_host = omniauth_config_full_host + end end diff --git a/spec/support/helpers/fake_u2f_device.rb b/spec/support/helpers/fake_u2f_device.rb index a7605cd483a..22cd8152d77 100644 --- a/spec/support/helpers/fake_u2f_device.rb +++ b/spec/support/helpers/fake_u2f_device.rb @@ -32,6 +32,10 @@ class FakeU2fDevice ") end + def fake_u2f_authentication + @page.execute_script("window.gl.u2fAuthenticate.renderAuthenticated('abc');") + end + private def u2f_device(app_id) diff --git a/spec/support/helpers/login_helpers.rb b/spec/support/helpers/login_helpers.rb index 0bb2d2510c2..0cb99b4e087 100644 --- a/spec/support/helpers/login_helpers.rb +++ b/spec/support/helpers/login_helpers.rb @@ -87,12 +87,17 @@ module LoginHelpers click_link "oauth-login-#{provider}" end + def fake_successful_u2f_authentication + allow(U2fRegistration).to receive(:authenticate).and_return(true) + FakeU2fDevice.new(page, nil).fake_u2f_authentication + end + def mock_auth_hash_with_saml_xml(provider, uid, email, saml_response) response_object = { document: saml_xml(saml_response) } mock_auth_hash(provider, uid, email, response_object: response_object) end - def mock_auth_hash(provider, uid, email, response_object: nil) + def configure_mock_auth(provider, uid, email, response_object: nil) # The mock_auth configuration allows you to set per-provider (or default) # authentication hashes to return during integration testing. OmniAuth.config.mock_auth[provider.to_sym] = OmniAuth::AuthHash.new({ @@ -118,6 +123,11 @@ module LoginHelpers response_object: response_object } }) + end + + def mock_auth_hash(provider, uid, email, response_object: nil) + configure_mock_auth(provider, uid, email, response_object: response_object) + original_env_config_omniauth_auth = Rails.application.env_config['omniauth.auth'] Rails.application.env_config['omniauth.auth'] = OmniAuth.config.mock_auth[provider.to_sym] diff --git a/spec/support/test_reports/test_reports_helper.rb b/spec/support/test_reports/test_reports_helper.rb index 45c6e04dbf3..6840fb9a860 100644 --- a/spec/support/test_reports/test_reports_helper.rb +++ b/spec/support/test_reports/test_reports_helper.rb @@ -1,36 +1,36 @@ module TestReportsHelper - def create_test_case_rspec_success + def create_test_case_rspec_success(name = 'test_spec') Gitlab::Ci::Reports::TestCase.new( name: 'Test#sum when a is 1 and b is 3 returns summary', - classname: 'spec.test_spec', + classname: "spec.#{name}", file: './spec/test_spec.rb', execution_time: 1.11, status: Gitlab::Ci::Reports::TestCase::STATUS_SUCCESS) end - def create_test_case_rspec_failed + def create_test_case_rspec_failed(name = 'test_spec') Gitlab::Ci::Reports::TestCase.new( - name: 'Test#sum when a is 2 and b is 2 returns summary', - classname: 'spec.test_spec', + name: 'Test#sum when a is 1 and b is 3 returns summary', + classname: "spec.#{name}", file: './spec/test_spec.rb', execution_time: 2.22, system_output: sample_rspec_failed_message, status: Gitlab::Ci::Reports::TestCase::STATUS_FAILED) end - def create_test_case_rspec_skipped + def create_test_case_rspec_skipped(name = 'test_spec') Gitlab::Ci::Reports::TestCase.new( name: 'Test#sum when a is 3 and b is 3 returns summary', - classname: 'spec.test_spec', + classname: "spec.#{name}", file: './spec/test_spec.rb', execution_time: 3.33, status: Gitlab::Ci::Reports::TestCase::STATUS_SKIPPED) end - def create_test_case_rspec_error + def create_test_case_rspec_error(name = 'test_spec') Gitlab::Ci::Reports::TestCase.new( name: 'Test#sum when a is 4 and b is 4 returns summary', - classname: 'spec.test_spec', + classname: "spec.#{name}", file: './spec/test_spec.rb', execution_time: 4.44, status: Gitlab::Ci::Reports::TestCase::STATUS_ERROR) @@ -48,34 +48,34 @@ module TestReportsHelper EOF end - def create_test_case_java_success + def create_test_case_java_success(name = 'addTest') Gitlab::Ci::Reports::TestCase.new( - name: 'addTest', + name: name, classname: 'CalculatorTest', execution_time: 5.55, status: Gitlab::Ci::Reports::TestCase::STATUS_SUCCESS) end - def create_test_case_java_failed + def create_test_case_java_failed(name = 'addTest') Gitlab::Ci::Reports::TestCase.new( - name: 'subtractTest', + name: name, classname: 'CalculatorTest', execution_time: 6.66, system_output: sample_java_failed_message, status: Gitlab::Ci::Reports::TestCase::STATUS_FAILED) end - def create_test_case_java_skipped + def create_test_case_java_skipped(name = 'addTest') Gitlab::Ci::Reports::TestCase.new( - name: 'multiplyTest', + name: name, classname: 'CalculatorTest', execution_time: 7.77, status: Gitlab::Ci::Reports::TestCase::STATUS_SKIPPED) end - def create_test_case_java_error + def create_test_case_java_error(name = 'addTest') Gitlab::Ci::Reports::TestCase.new( - name: 'divideTest', + name: name, classname: 'CalculatorTest', execution_time: 8.88, status: Gitlab::Ci::Reports::TestCase::STATUS_ERROR) diff --git a/spec/uploaders/file_mover_spec.rb b/spec/uploaders/file_mover_spec.rb index a9e03f3d4e5..5ee0a10f38d 100644 --- a/spec/uploaders/file_mover_spec.rb +++ b/spec/uploaders/file_mover_spec.rb @@ -85,8 +85,7 @@ describe FileMover do context 'when tmp uploader is not local storage' do before do - allow(PersonalFileUploader).to receive(:object_store_enabled?) { true } - tmp_uploader.object_store = ObjectStorage::Store::REMOTE + stub_uploads_object_storage(uploader: PersonalFileUploader) allow_any_instance_of(PersonalFileUploader).to receive(:file_storage?) { false } end diff --git a/spec/uploaders/file_uploader_spec.rb b/spec/uploaders/file_uploader_spec.rb index 185c62491ce..04206de3dc6 100644 --- a/spec/uploaders/file_uploader_spec.rb +++ b/spec/uploaders/file_uploader_spec.rb @@ -184,40 +184,37 @@ describe FileUploader do end end - describe '#cache!' do - subject do - uploader.store!(uploaded_file) - end + context 'when remote file is used' do + let(:temp_file) { Tempfile.new("test") } - context 'when remote file is used' do - let(:temp_file) { Tempfile.new("test") } + let!(:fog_connection) do + stub_uploads_object_storage(described_class) + end - let!(:fog_connection) do - stub_uploads_object_storage(described_class) - end + let(:filename) { "my file.txt" } + let(:uploaded_file) do + UploadedFile.new(temp_file.path, filename: filename, remote_id: "test/123123") + end - let(:uploaded_file) do - UploadedFile.new(temp_file.path, filename: "my file.txt", remote_id: "test/123123") - end + let!(:fog_file) do + fog_connection.directories.new(key: 'uploads').files.create( + key: 'tmp/uploads/test/123123', + body: 'content' + ) + end - let!(:fog_file) do - fog_connection.directories.new(key: 'uploads').files.create( - key: 'tmp/uploads/test/123123', - body: 'content' - ) - end + before do + FileUtils.touch(temp_file) - before do - FileUtils.touch(temp_file) - end + uploader.store!(uploaded_file) + end - after do - FileUtils.rm_f(temp_file) - end + after do + FileUtils.rm_f(temp_file) + end + describe '#cache!' do it 'file is stored remotely in permament location with sanitized name' do - subject - expect(uploader).to be_exists expect(uploader).not_to be_cached expect(uploader).not_to be_file_storage @@ -228,5 +225,18 @@ describe FileUploader do expect(uploader.object_store).to eq(described_class::Store::REMOTE) end end + + describe '#to_h' do + subject { uploader.to_h } + + let(:filename) { 'my+file.txt' } + + it 'generates URL using original file name instead of filename returned by object storage' do + # GCS returns a URL with a `+` instead of `%2B` + allow(uploader.file).to receive(:url).and_return('https://storage.googleapis.com/gitlab-test-uploads/@hashed/6b/86/6b86b273ff34fce19d6b804eff5a3f5747ada4eaa22f1d49c01e52ddb7875b4b/64c5065e62100b1a12841644256a98be/my+file.txt') + + expect(subject[:url]).to end_with(filename) + end + end end end diff --git a/spec/workers/namespaces/schedule_aggregation_worker_spec.rb b/spec/workers/namespaces/schedule_aggregation_worker_spec.rb index 7432ca12f2a..d4a49a3f53a 100644 --- a/spec/workers/namespaces/schedule_aggregation_worker_spec.rb +++ b/spec/workers/namespaces/schedule_aggregation_worker_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Namespaces::ScheduleAggregationWorker, '#perform' do +describe Namespaces::ScheduleAggregationWorker, '#perform', :clean_gitlab_redis_shared_state do let(:group) { create(:group) } subject(:worker) { described_class.new } @@ -10,6 +10,8 @@ describe Namespaces::ScheduleAggregationWorker, '#perform' do context 'when group is the root ancestor' do context 'when aggregation schedule exists' do it 'does not create a new one' do + stub_aggregation_schedule_statistics + Namespace::AggregationSchedule.safe_find_or_create_by!(namespace_id: group.id) expect do @@ -18,26 +20,25 @@ describe Namespaces::ScheduleAggregationWorker, '#perform' do end end - context 'when update_statistics_namespace is off' do - it 'does not create a new one' do - stub_feature_flags(update_statistics_namespace: false, namespace: group) + context 'when aggregation schedule does not exist' do + it 'creates one' do + stub_aggregation_schedule_statistics expect do worker.perform(group.id) - end.not_to change(Namespace::AggregationSchedule, :count) + end.to change(Namespace::AggregationSchedule, :count).by(1) + + expect(group.aggregation_schedule).to be_present end end - context 'when aggregation schedule does not exist' do - it 'creates one' do - allow_any_instance_of(Namespace::AggregationSchedule) - .to receive(:schedule_root_storage_statistics).and_return(nil) + context 'when update_statistics_namespace is off' do + it 'does not create a new one' do + stub_feature_flags(update_statistics_namespace: false, namespace: group) expect do worker.perform(group.id) - end.to change(Namespace::AggregationSchedule, :count).by(1) - - expect(group.aggregation_schedule).to be_present + end.not_to change(Namespace::AggregationSchedule, :count) end end end @@ -47,8 +48,7 @@ describe Namespaces::ScheduleAggregationWorker, '#perform' do let(:group) { create(:group, parent: parent_group) } it 'creates an aggregation schedule for the root' do - allow_any_instance_of(Namespace::AggregationSchedule) - .to receive(:schedule_root_storage_statistics).and_return(nil) + stub_aggregation_schedule_statistics worker.perform(group.id) @@ -63,4 +63,15 @@ describe Namespaces::ScheduleAggregationWorker, '#perform' do worker.perform(12345) end end + + def stub_aggregation_schedule_statistics + # Namespace::Aggregations are deleted by + # Namespace::AggregationSchedule::schedule_root_storage_statistics, + # which is executed async. Stubing the service so instances are not deleted + # while still running the specs. + expect_next_instance_of(Namespace::AggregationSchedule) do |aggregation_schedule| + expect(aggregation_schedule) + .to receive(:schedule_root_storage_statistics) + end + end end |