diff options
Diffstat (limited to 'spec')
128 files changed, 4944 insertions, 1304 deletions
diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 81cbccd5436..760f33b09c1 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -100,8 +100,6 @@ describe ApplicationController do end describe '#route_not_found' do - let(:controller) { ApplicationController.new } - it 'renders 404 if authenticated' do allow(controller).to receive(:current_user).and_return(user) expect(controller).to receive(:not_found) @@ -115,4 +113,203 @@ describe ApplicationController do controller.send(:route_not_found) end end + + context 'two-factor authentication' do + let(:controller) { ApplicationController.new } + + describe '#check_two_factor_requirement' do + subject { controller.send :check_two_factor_requirement } + + it 'does not redirect if 2FA is not required' do + allow(controller).to receive(:two_factor_authentication_required?).and_return(false) + expect(controller).not_to receive(:redirect_to) + + subject + end + + it 'does not redirect if user is not logged in' do + allow(controller).to receive(:two_factor_authentication_required?).and_return(true) + allow(controller).to receive(:current_user).and_return(nil) + expect(controller).not_to receive(:redirect_to) + + subject + end + + it 'does not redirect if user has 2FA enabled' do + allow(controller).to receive(:two_factor_authentication_required?).and_return(true) + allow(controller).to receive(:current_user).twice.and_return(user) + allow(user).to receive(:two_factor_enabled?).and_return(true) + expect(controller).not_to receive(:redirect_to) + + subject + end + + it 'does not redirect if 2FA setup can be skipped' do + allow(controller).to receive(:two_factor_authentication_required?).and_return(true) + allow(controller).to receive(:current_user).twice.and_return(user) + allow(user).to receive(:two_factor_enabled?).and_return(false) + allow(controller).to receive(:skip_two_factor?).and_return(true) + expect(controller).not_to receive(:redirect_to) + + subject + end + + it 'redirects to 2FA setup otherwise' do + allow(controller).to receive(:two_factor_authentication_required?).and_return(true) + allow(controller).to receive(:current_user).twice.and_return(user) + allow(user).to receive(:two_factor_enabled?).and_return(false) + allow(controller).to receive(:skip_two_factor?).and_return(false) + allow(controller).to receive(:profile_two_factor_auth_path) + expect(controller).to receive(:redirect_to) + + subject + end + end + + describe '#two_factor_authentication_required?' do + subject { controller.send :two_factor_authentication_required? } + + it 'returns false if no 2FA requirement is present' do + allow(controller).to receive(:current_user).and_return(nil) + + expect(subject).to be_falsey + end + + it 'returns true if a 2FA requirement is set in the application settings' do + stub_application_setting require_two_factor_authentication: true + allow(controller).to receive(:current_user).and_return(nil) + + expect(subject).to be_truthy + end + + it 'returns true if a 2FA requirement is set on the user' do + user.require_two_factor_authentication_from_group = true + allow(controller).to receive(:current_user).and_return(user) + + expect(subject).to be_truthy + end + end + + describe '#two_factor_grace_period' do + subject { controller.send :two_factor_grace_period } + + it 'returns the grace period from the application settings' do + stub_application_setting two_factor_grace_period: 23 + allow(controller).to receive(:current_user).and_return(nil) + + expect(subject).to eq 23 + end + + context 'with a 2FA requirement set on the user' do + let(:user) { create :user, require_two_factor_authentication_from_group: true, two_factor_grace_period: 23 } + + it 'returns the user grace period if lower than the application grace period' do + stub_application_setting two_factor_grace_period: 24 + allow(controller).to receive(:current_user).and_return(user) + + expect(subject).to eq 23 + end + + it 'returns the application grace period if lower than the user grace period' do + stub_application_setting two_factor_grace_period: 22 + allow(controller).to receive(:current_user).and_return(user) + + expect(subject).to eq 22 + end + end + end + + describe '#two_factor_grace_period_expired?' do + subject { controller.send :two_factor_grace_period_expired? } + + before do + allow(controller).to receive(:current_user).and_return(user) + end + + it 'returns false if the user has not started their grace period yet' do + expect(subject).to be_falsey + end + + context 'with grace period started' do + let(:user) { create :user, otp_grace_period_started_at: 2.hours.ago } + + it 'returns true if the grace period has expired' do + allow(controller).to receive(:two_factor_grace_period).and_return(1) + + expect(subject).to be_truthy + end + + it 'returns false if the grace period is still active' do + allow(controller).to receive(:two_factor_grace_period).and_return(3) + + expect(subject).to be_falsey + end + end + end + + describe '#two_factor_skippable' do + subject { controller.send :two_factor_skippable? } + + before do + allow(controller).to receive(:current_user).and_return(user) + end + + it 'returns false if 2FA is not required' do + allow(controller).to receive(:two_factor_authentication_required?).and_return(false) + + expect(subject).to be_falsey + end + + it 'returns false if the user has already enabled 2FA' do + allow(controller).to receive(:two_factor_authentication_required?).and_return(true) + allow(user).to receive(:two_factor_enabled?).and_return(true) + + expect(subject).to be_falsey + end + + it 'returns false if the 2FA grace period has expired' do + allow(controller).to receive(:two_factor_authentication_required?).and_return(true) + allow(user).to receive(:two_factor_enabled?).and_return(false) + allow(controller).to receive(:two_factor_grace_period_expired?).and_return(true) + + expect(subject).to be_falsey + end + + it 'returns true otherwise' do + allow(controller).to receive(:two_factor_authentication_required?).and_return(true) + allow(user).to receive(:two_factor_enabled?).and_return(false) + allow(controller).to receive(:two_factor_grace_period_expired?).and_return(false) + + expect(subject).to be_truthy + end + end + + describe '#skip_two_factor?' do + subject { controller.send :skip_two_factor? } + + it 'returns false if 2FA setup was not skipped' do + allow(controller).to receive(:session).and_return({}) + + expect(subject).to be_falsey + end + + context 'with 2FA setup skipped' do + before do + allow(controller).to receive(:session).and_return({ skip_two_factor: 2.hours.from_now }) + end + + it 'returns false if the grace period has expired' do + Timecop.freeze(3.hours.from_now) do + expect(subject).to be_falsey + end + end + + it 'returns true if the grace period is still active' do + Timecop.freeze(1.hour.from_now) do + expect(subject).to be_truthy + end + end + end + end + end end diff --git a/spec/controllers/projects/blob_controller_spec.rb b/spec/controllers/projects/blob_controller_spec.rb index ec36a64b415..3e9f272a0d8 100644 --- a/spec/controllers/projects/blob_controller_spec.rb +++ b/spec/controllers/projects/blob_controller_spec.rb @@ -2,15 +2,10 @@ require 'rails_helper' describe Projects::BlobController do let(:project) { create(:project, :public, :repository) } - let(:user) { create(:user) } - - before do - project.team << [user, :master] - - sign_in(user) - end describe 'GET diff' do + let(:user) { create(:user) } + render_views def do_get(opts = {}) @@ -20,6 +15,12 @@ describe Projects::BlobController do get :diff, params.merge(opts) end + before do + project.team << [user, :master] + + sign_in(user) + end + context 'when essential params are missing' do it 'renders nothing' do do_get @@ -37,7 +38,69 @@ describe Projects::BlobController do end end + describe 'GET edit' do + let(:default_params) do + { + namespace_id: project.namespace, + project_id: project, + id: 'master/CHANGELOG' + } + end + + context 'anonymous' do + before do + get :edit, default_params + end + + it 'redirects to sign in and returns' do + expect(response).to redirect_to(new_user_session_path) + end + end + + context 'as guest' do + let(:guest) { create(:user) } + + before do + sign_in(guest) + get :edit, default_params + end + + it 'redirects to blob show' do + expect(response).to redirect_to(namespace_project_blob_path(project.namespace, project, 'master/CHANGELOG')) + end + end + + context 'as developer' do + let(:developer) { create(:user) } + + before do + project.team << [developer, :developer] + sign_in(developer) + get :edit, default_params + end + + it 'redirects to blob show' do + expect(response).to have_http_status(200) + end + end + + context 'as master' do + let(:master) { create(:user) } + + before do + project.team << [master, :master] + sign_in(master) + get :edit, default_params + end + + it 'redirects to blob show' do + expect(response).to have_http_status(200) + end + end + end + describe 'PUT update' do + let(:user) { create(:user) } let(:default_params) do { namespace_id: project.namespace, @@ -53,6 +116,12 @@ describe Projects::BlobController do namespace_project_blob_path(project.namespace, project, 'master/CHANGELOG') end + before do + project.team << [user, :master] + + sign_in(user) + end + it 'redirects to blob' do put :update, default_params diff --git a/spec/controllers/projects/registry/repositories_controller_spec.rb b/spec/controllers/projects/registry/repositories_controller_spec.rb new file mode 100644 index 00000000000..464302824a8 --- /dev/null +++ b/spec/controllers/projects/registry/repositories_controller_spec.rb @@ -0,0 +1,84 @@ +require 'spec_helper' + +describe Projects::Registry::RepositoriesController do + let(:user) { create(:user) } + let(:project) { create(:empty_project, :private) } + + before do + sign_in(user) + stub_container_registry_config(enabled: true) + end + + context 'when user has access to registry' do + before do + project.add_developer(user) + end + + describe 'GET index' do + context 'when root container repository exists' do + before do + create(:container_repository, :root, project: project) + end + + it 'does not create root container repository' do + expect { go_to_index }.not_to change { ContainerRepository.all.count } + end + end + + context 'when root container repository is not created' do + context 'when there are tags for this repository' do + before do + stub_container_registry_tags(repository: project.full_path, + tags: %w[rc1 latest]) + end + + it 'successfully renders container repositories' do + go_to_index + + expect(response).to have_http_status(:ok) + end + + it 'creates a root container repository' do + expect { go_to_index }.to change { ContainerRepository.all.count }.by(1) + expect(ContainerRepository.first).to be_root_repository + end + end + + context 'when there are no tags for this repository' do + before do + stub_container_registry_tags(repository: :any, tags: []) + end + + it 'successfully renders container repositories' do + go_to_index + + expect(response).to have_http_status(:ok) + end + + it 'does not ensure root container repository' do + expect { go_to_index }.not_to change { ContainerRepository.all.count } + end + end + end + end + end + + context 'when user does not have access to registry' do + describe 'GET index' do + it 'responds with 404' do + go_to_index + + expect(response).to have_http_status(:not_found) + end + + it 'does not ensure root container repository' do + expect { go_to_index }.not_to change { ContainerRepository.all.count } + end + end + end + + def go_to_index + get :index, namespace_id: project.namespace, + project_id: project + end +end diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb index 87a0c95c4dc..b62def83ee4 100644 --- a/spec/factories/ci/builds.rb +++ b/spec/factories/ci/builds.rb @@ -111,7 +111,7 @@ FactoryGirl.define do trait :trace do after(:create) do |build, evaluator| - build.trace = 'BUILD TRACE' + build.trace.set('BUILD TRACE') end end diff --git a/spec/factories/ci/trigger_schedules.rb b/spec/factories/ci/trigger_schedules.rb new file mode 100644 index 00000000000..315bce16995 --- /dev/null +++ b/spec/factories/ci/trigger_schedules.rb @@ -0,0 +1,26 @@ +FactoryGirl.define do + factory :ci_trigger_schedule, class: Ci::TriggerSchedule do + trigger factory: :ci_trigger_for_trigger_schedule + cron '0 1 * * *' + cron_timezone Gitlab::Ci::CronParser::VALID_SYNTAX_SAMPLE_TIME_ZONE + + after(:build) do |trigger_schedule, evaluator| + trigger_schedule.update!(project: trigger_schedule.trigger.project) + end + + trait :nightly do + cron '0 1 * * *' + cron_timezone Gitlab::Ci::CronParser::VALID_SYNTAX_SAMPLE_TIME_ZONE + end + + trait :weekly do + cron '0 1 * * 6' + cron_timezone Gitlab::Ci::CronParser::VALID_SYNTAX_SAMPLE_TIME_ZONE + end + + trait :monthly do + cron '0 1 22 * *' + cron_timezone Gitlab::Ci::CronParser::VALID_SYNTAX_SAMPLE_TIME_ZONE + end + end +end diff --git a/spec/factories/ci/triggers.rb b/spec/factories/ci/triggers.rb index a27b04424e5..c3a29d8bf04 100644 --- a/spec/factories/ci/triggers.rb +++ b/spec/factories/ci/triggers.rb @@ -1,7 +1,14 @@ FactoryGirl.define do factory :ci_trigger_without_token, class: Ci::Trigger do factory :ci_trigger do - token 'token' + sequence(:token) { |n| "token#{n}" } + + factory :ci_trigger_for_trigger_schedule do + token { SecureRandom.hex(15) } + owner factory: :user + project factory: :project + ref 'master' + end end end end diff --git a/spec/factories/container_repositories.rb b/spec/factories/container_repositories.rb new file mode 100644 index 00000000000..3fcad9fd4b3 --- /dev/null +++ b/spec/factories/container_repositories.rb @@ -0,0 +1,33 @@ +FactoryGirl.define do + factory :container_repository do + name 'test_container_image' + project + + transient do + tags [] + end + + trait :root do + name '' + end + + after(:build) do |repository, evaluator| + next if evaluator.tags.to_a.none? + + allow(repository.client) + .to receive(:repository_tags) + .and_return({ + 'name' => repository.path, + 'tags' => evaluator.tags + }) + + evaluator.tags.each do |tag| + allow(repository.client) + .to receive(:repository_tag_digest) + .with(repository.path, tag) + .and_return('sha256:4c8e63ca4cb663ce6c688cb06f1c3' \ + '72b088dac5b6d7ad7d49cd620d85cf72a15') + end + end + end +end diff --git a/spec/features/admin/admin_deploy_keys_spec.rb b/spec/features/admin/admin_deploy_keys_spec.rb index 7ce6cce0a5c..c0b6995a84a 100644 --- a/spec/features/admin/admin_deploy_keys_spec.rb +++ b/spec/features/admin/admin_deploy_keys_spec.rb @@ -18,7 +18,7 @@ RSpec.describe 'admin deploy keys', type: :feature do describe 'create new deploy key' do before do visit admin_deploy_keys_path - click_link 'New Deploy Key' + click_link 'New deploy key' end it 'creates new deploy key' do diff --git a/spec/features/admin/admin_groups_spec.rb b/spec/features/admin/admin_groups_spec.rb index a871e370ba2..56d5c7c327e 100644 --- a/spec/features/admin/admin_groups_spec.rb +++ b/spec/features/admin/admin_groups_spec.rb @@ -24,7 +24,7 @@ feature 'Admin Groups', feature: true do it 'creates new group' do visit admin_groups_path - click_link "New Group" + click_link "New group" fill_in 'group_path', with: 'gitlab' fill_in 'group_description', with: 'Group description' click_button "Create group" diff --git a/spec/features/admin/admin_hooks_spec.rb b/spec/features/admin/admin_hooks_spec.rb index 570c374a89b..fb519a9bf12 100644 --- a/spec/features/admin/admin_hooks_spec.rb +++ b/spec/features/admin/admin_hooks_spec.rb @@ -33,7 +33,7 @@ describe "Admin::Hooks", feature: true do fill_in 'hook_url', with: url check 'Enable SSL verification' - expect { click_button 'Add System Hook' }.to change(SystemHook, :count).by(1) + expect { click_button 'Add system hook' }.to change(SystemHook, :count).by(1) expect(page).to have_content 'SSL Verification: enabled' expect(current_path).to eq(admin_hooks_path) expect(page).to have_content(url) @@ -44,7 +44,7 @@ describe "Admin::Hooks", feature: true do before do WebMock.stub_request(:post, @system_hook.url) visit admin_hooks_path - click_link "Test Hook" + click_link "Test hook" end it { expect(current_path).to eq(admin_hooks_path) } diff --git a/spec/features/admin/admin_manage_applications_spec.rb b/spec/features/admin/admin_manage_applications_spec.rb index c2c618b5659..0079125889b 100644 --- a/spec/features/admin/admin_manage_applications_spec.rb +++ b/spec/features/admin/admin_manage_applications_spec.rb @@ -8,7 +8,7 @@ RSpec.describe 'admin manage applications', feature: true do it do visit admin_applications_path - click_on 'New Application' + click_on 'New application' expect(page).to have_content('New application') fill_in :doorkeeper_application_name, with: 'test' diff --git a/spec/features/admin/admin_users_impersonation_tokens_spec.rb b/spec/features/admin/admin_users_impersonation_tokens_spec.rb index ff23d486355..0fb4baeb71c 100644 --- a/spec/features/admin/admin_users_impersonation_tokens_spec.rb +++ b/spec/features/admin/admin_users_impersonation_tokens_spec.rb @@ -30,7 +30,7 @@ describe 'Admin > Users > Impersonation Tokens', feature: true, js: true do check "api" check "read_user" - expect { click_on "Create Impersonation Token" }.to change { PersonalAccessTokensFinder.new(impersonation: true).execute.count } + expect { click_on "Create impersonation token" }.to change { PersonalAccessTokensFinder.new(impersonation: true).execute.count } expect(active_impersonation_tokens).to have_text(name) expect(active_impersonation_tokens).to have_text('In') expect(active_impersonation_tokens).to have_text('api') diff --git a/spec/features/auto_deploy_spec.rb b/spec/features/auto_deploy_spec.rb index 009e9c6b04c..6f36c74c911 100644 --- a/spec/features/auto_deploy_spec.rb +++ b/spec/features/auto_deploy_spec.rb @@ -56,7 +56,7 @@ describe 'Auto deploy' do click_on 'OpenShift' end wait_for_ajax - click_button 'Commit Changes' + click_button 'Commit changes' expect(page).to have_content('New Merge Request From auto-deploy into master') end diff --git a/spec/features/boards/add_issues_modal_spec.rb b/spec/features/boards/add_issues_modal_spec.rb index 1c0f97d8a1c..248c31115ad 100644 --- a/spec/features/boards/add_issues_modal_spec.rb +++ b/spec/features/boards/add_issues_modal_spec.rb @@ -145,7 +145,7 @@ describe 'Issue Boards add issue modal', :feature, :js do context 'selecing issues' do it 'selects single issue' do page.within('.add-issues-modal') do - first('.card').click + first('.card .card-number').click page.within('.nav-links') do expect(page).to have_content('Selected issues 1') @@ -155,7 +155,7 @@ describe 'Issue Boards add issue modal', :feature, :js do it 'changes button text' do page.within('.add-issues-modal') do - first('.card').click + first('.card .card-number').click expect(first('.add-issues-footer .btn')).to have_content('Add 1 issue') end @@ -163,7 +163,7 @@ describe 'Issue Boards add issue modal', :feature, :js do it 'changes button text with plural' do page.within('.add-issues-modal') do - all('.card').each do |el| + all('.card .card-number').each do |el| el.click end @@ -173,7 +173,7 @@ describe 'Issue Boards add issue modal', :feature, :js do it 'shows only selected issues on selected tab' do page.within('.add-issues-modal') do - first('.card').click + first('.card .card-number').click click_link 'Selected issues' @@ -203,7 +203,7 @@ describe 'Issue Boards add issue modal', :feature, :js do it 'selects all that arent already selected' do page.within('.add-issues-modal') do - first('.card').click + first('.card .card-number').click expect(page).to have_selector('.is-active', count: 1) @@ -215,11 +215,11 @@ describe 'Issue Boards add issue modal', :feature, :js do it 'unselects from selected tab' do page.within('.add-issues-modal') do - first('.card').click + first('.card .card-number').click click_link 'Selected issues' - first('.card').click + first('.card .card-number').click expect(page).not_to have_selector('.is-active') end @@ -229,7 +229,7 @@ describe 'Issue Boards add issue modal', :feature, :js do context 'adding issues' do it 'adds to board' do page.within('.add-issues-modal') do - first('.card').click + first('.card .card-number').click click_button 'Add 1 issue' end @@ -241,7 +241,7 @@ describe 'Issue Boards add issue modal', :feature, :js do it 'adds to second list' do page.within('.add-issues-modal') do - first('.card').click + first('.card .card-number').click click_button planning.title diff --git a/spec/features/boards/boards_spec.rb b/spec/features/boards/boards_spec.rb index e168585534d..30ad169e30e 100644 --- a/spec/features/boards/boards_spec.rb +++ b/spec/features/boards/boards_spec.rb @@ -590,7 +590,7 @@ describe 'Issue Boards', feature: true, js: true do end def click_filter_link(link_text) - page.within('.filtered-search-input-container') do + page.within('.filtered-search-box') do expect(page).to have_button(link_text) click_button(link_text) diff --git a/spec/features/boards/keyboard_shortcut_spec.rb b/spec/features/boards/keyboard_shortcut_spec.rb index a5fc766401f..a9cc6c49f8e 100644 --- a/spec/features/boards/keyboard_shortcut_spec.rb +++ b/spec/features/boards/keyboard_shortcut_spec.rb @@ -14,7 +14,7 @@ describe 'Issue Boards shortcut', feature: true, js: true do end it 'takes user to issue board index' do - find('body').native.send_keys('gl') + find('body').native.send_keys('gb') expect(page).to have_selector('.boards-list') wait_for_vue_resource diff --git a/spec/features/boards/modal_filter_spec.rb b/spec/features/boards/modal_filter_spec.rb index e2281a7da55..4a4c13e79c8 100644 --- a/spec/features/boards/modal_filter_spec.rb +++ b/spec/features/boards/modal_filter_spec.rb @@ -219,7 +219,7 @@ describe 'Issue Boards add issue modal filtering', :feature, :js do end def click_filter_link(link_text) - page.within('.add-issues-modal .filtered-search-input-container') do + page.within('.add-issues-modal .filtered-search-box') do expect(page).to have_button(link_text) click_button(link_text) diff --git a/spec/features/container_registry_spec.rb b/spec/features/container_registry_spec.rb index 203e55a36f2..b86609e07c5 100644 --- a/spec/features/container_registry_spec.rb +++ b/spec/features/container_registry_spec.rb @@ -1,45 +1,61 @@ require 'spec_helper' describe "Container Registry" do + let(:user) { create(:user) } let(:project) { create(:empty_project) } - let(:repository) { project.container_registry_repository } - let(:tag_name) { 'latest' } - let(:tags) { [tag_name] } + + let(:container_repository) do + create(:container_repository, name: 'my/image') + end before do - login_as(:user) - project.team << [@user, :developer] - stub_container_registry_tags(*tags) + login_as(user) + project.add_developer(user) stub_container_registry_config(enabled: true) - allow(Auth::ContainerRegistryAuthenticationService).to receive(:full_access_token).and_return('token') + stub_container_registry_tags(repository: :any, tags: []) end - describe 'GET /:project/container_registry' do + context 'when there are no image repositories' do + scenario 'user visits container registry main page' do + visit_container_registry + + expect(page).to have_content 'No container image repositories' + end + end + + context 'when there are image repositories' do before do - visit namespace_project_container_registry_index_path(project.namespace, project) + stub_container_registry_tags(repository: %r{my/image}, tags: %w[latest]) + project.container_repositories << container_repository end - context 'when no tags' do - let(:tags) { [] } + scenario 'user wants to see multi-level container repository' do + visit_container_registry - it { expect(page).to have_content('No images in Container Registry for this project') } + expect(page).to have_content('my/image') end - context 'when there are tags' do - it { expect(page).to have_content(tag_name) } - it { expect(page).to have_content('d7a513a66') } - end - end + scenario 'user removes entire container repository' do + visit_container_registry - describe 'DELETE /:project/container_registry/tag' do - before do - visit namespace_project_container_registry_index_path(project.namespace, project) + expect_any_instance_of(ContainerRepository) + .to receive(:delete_tags!).and_return(true) + + click_on 'Remove repository' end - it do - expect_any_instance_of(::ContainerRegistry::Tag).to receive(:delete).and_return(true) + scenario 'user removes a specific tag from container repository' do + visit_container_registry - click_on 'Remove' + expect_any_instance_of(ContainerRegistry::Tag) + .to receive(:delete).and_return(true) + + click_on 'Remove tag' end end + + def visit_container_registry + visit namespace_project_container_registry_index_path( + project.namespace, project) + end end diff --git a/spec/features/dashboard/group_spec.rb b/spec/features/dashboard/group_spec.rb index d5f8470fab0..19f5d1b0f30 100644 --- a/spec/features/dashboard/group_spec.rb +++ b/spec/features/dashboard/group_spec.rb @@ -7,7 +7,7 @@ RSpec.describe 'Dashboard Group', feature: true do it 'creates new grpup' do visit dashboard_groups_path - click_link 'New Group' + click_link 'New group' fill_in 'group_path', with: 'Samurai' fill_in 'group_description', with: 'Tokugawa Shogunate' diff --git a/spec/features/dashboard/project_member_activity_index_spec.rb b/spec/features/dashboard/project_member_activity_index_spec.rb index 49d93db58a9..d62839a09ef 100644 --- a/spec/features/dashboard/project_member_activity_index_spec.rb +++ b/spec/features/dashboard/project_member_activity_index_spec.rb @@ -21,20 +21,20 @@ feature 'Project member activity', feature: true, js: true do context 'when a user joins the project' do before { visit_activities_and_wait_with_event(Event::JOINED) } - it { is_expected.to eq("#{user.name} joined project") } + it { is_expected.to eq("joined project") } end context 'when a user leaves the project' do before { visit_activities_and_wait_with_event(Event::LEFT) } - it { is_expected.to eq("#{user.name} left project") } + it { is_expected.to eq("left project") } end context 'when a users membership expires for the project' do before { visit_activities_and_wait_with_event(Event::EXPIRED) } it "presents the correct message" do - message = "#{user.name} removed due to membership expiration from project" + message = "removed due to membership expiration from project" is_expected.to eq(message) end end diff --git a/spec/features/dashboard/shortcuts_spec.rb b/spec/features/dashboard/shortcuts_spec.rb index 3642c0bfb5b..fa5524e18d8 100644 --- a/spec/features/dashboard/shortcuts_spec.rb +++ b/spec/features/dashboard/shortcuts_spec.rb @@ -3,27 +3,23 @@ require 'spec_helper' feature 'Dashboard shortcuts', feature: true, js: true do before do login_as :user - visit dashboard_projects_path + visit root_dashboard_path end scenario 'Navigate to tabs' do - find('body').native.send_key('g') - find('body').native.send_key('p') + find('body').native.send_keys([:shift, 'P']) check_page_title('Projects') - find('body').native.send_key('g') - find('body').native.send_key('i') + find('body').native.send_key([:shift, 'I']) check_page_title('Issues') - find('body').native.send_key('g') - find('body').native.send_key('m') + find('body').native.send_key([:shift, 'M']) check_page_title('Merge Requests') - find('body').native.send_key('g') - find('body').native.send_key('t') + find('body').native.send_keys([:shift, 'T']) check_page_title('Todos') end diff --git a/spec/features/groups_spec.rb b/spec/features/groups_spec.rb index c90cc06a8f5..7670c4caea4 100644 --- a/spec/features/groups_spec.rb +++ b/spec/features/groups_spec.rb @@ -86,17 +86,40 @@ feature 'Group', feature: true do describe 'create a nested group' do let(:group) { create(:group, path: 'foo') } - before do - visit subgroups_group_path(group) - click_link 'New Subgroup' + context 'as admin' do + before do + visit subgroups_group_path(group) + click_link 'New Subgroup' + end + + it 'creates a nested group' do + fill_in 'Group path', with: 'bar' + click_button 'Create group' + + expect(current_path).to eq(group_path('foo/bar')) + expect(page).to have_content("Group 'bar' was successfully created.") + end end - it 'creates a nested group' do - fill_in 'Group path', with: 'bar' - click_button 'Create group' + context 'as group owner' do + let(:user) { create(:user) } - expect(current_path).to eq(group_path('foo/bar')) - expect(page).to have_content("Group 'bar' was successfully created.") + before do + group.add_owner(user) + logout + login_as(user) + + visit subgroups_group_path(group) + click_link 'New Subgroup' + end + + it 'creates a nested group' do + fill_in 'Group path', with: 'bar' + click_button 'Create group' + + expect(current_path).to eq(group_path('foo/bar')) + expect(page).to have_content("Group 'bar' was successfully created.") + end end end @@ -130,7 +153,7 @@ feature 'Group', feature: true do end it 'removes group' do - click_link 'Remove Group' + click_link 'Remove group' expect(page).to have_content "scheduled for deletion" end diff --git a/spec/features/issues/filtered_search/dropdown_assignee_spec.rb b/spec/features/issues/filtered_search/dropdown_assignee_spec.rb index 4dcc56a97d1..3d1a9ed1722 100644 --- a/spec/features/issues/filtered_search/dropdown_assignee_spec.rb +++ b/spec/features/issues/filtered_search/dropdown_assignee_spec.rb @@ -194,7 +194,7 @@ describe 'Dropdown assignee', :feature, :js do new_user = create(:user) project.team << [new_user, :master] - find('.filtered-search-input-container .clear-search').click + find('.filtered-search-box .clear-search').click filtered_search.set('assignee') filtered_search.send_keys(':') diff --git a/spec/features/issues/filtered_search/dropdown_author_spec.rb b/spec/features/issues/filtered_search/dropdown_author_spec.rb index 1772a120045..990e3b3e60c 100644 --- a/spec/features/issues/filtered_search/dropdown_author_spec.rb +++ b/spec/features/issues/filtered_search/dropdown_author_spec.rb @@ -172,7 +172,7 @@ describe 'Dropdown author', js: true, feature: true do new_user = create(:user) project.team << [new_user, :master] - find('.filtered-search-input-container .clear-search').click + find('.filtered-search-box .clear-search').click filtered_search.set('author') send_keys_to_filtered_search(':') diff --git a/spec/features/issues/filtered_search/dropdown_label_spec.rb b/spec/features/issues/filtered_search/dropdown_label_spec.rb index d7d71b0eba9..abe5d61e38c 100644 --- a/spec/features/issues/filtered_search/dropdown_label_spec.rb +++ b/spec/features/issues/filtered_search/dropdown_label_spec.rb @@ -29,7 +29,7 @@ describe 'Dropdown label', js: true, feature: true do end def clear_search_field - find('.filtered-search-input-container .clear-search').click + find('.filtered-search-box .clear-search').click end before do diff --git a/spec/features/issues/filtered_search/dropdown_milestone_spec.rb b/spec/features/issues/filtered_search/dropdown_milestone_spec.rb index cf5259b878d..448259057b0 100644 --- a/spec/features/issues/filtered_search/dropdown_milestone_spec.rb +++ b/spec/features/issues/filtered_search/dropdown_milestone_spec.rb @@ -252,7 +252,7 @@ describe 'Dropdown milestone', :feature, :js do expect(initial_size).to be > 0 create(:milestone, project: project) - find('.filtered-search-input-container .clear-search').click + find('.filtered-search-box .clear-search').click filtered_search.set('milestone:') expect(dropdown_milestone_size).to eq(initial_size) diff --git a/spec/features/issues/filtered_search/filter_issues_spec.rb b/spec/features/issues/filtered_search/filter_issues_spec.rb index 2f880c926e7..6f00066de4d 100644 --- a/spec/features/issues/filtered_search/filter_issues_spec.rb +++ b/spec/features/issues/filtered_search/filter_issues_spec.rb @@ -758,10 +758,10 @@ describe 'Filter issues', js: true, feature: true do expect_issues_list_count(2) - sort_toggle = find('.filtered-search-container .dropdown-toggle') + sort_toggle = find('.filtered-search-wrapper .dropdown-toggle') sort_toggle.click - find('.filtered-search-container .dropdown-menu li a', text: 'Oldest updated').click + find('.filtered-search-wrapper .dropdown-menu li a', text: 'Oldest updated').click wait_for_ajax expect(find('.issues-list .issue:first-of-type .issue-title-text a')).to have_content(old_issue.title) diff --git a/spec/features/issues/filtered_search/recent_searches_spec.rb b/spec/features/issues/filtered_search/recent_searches_spec.rb new file mode 100644 index 00000000000..f506065a242 --- /dev/null +++ b/spec/features/issues/filtered_search/recent_searches_spec.rb @@ -0,0 +1,92 @@ +require 'spec_helper' + +describe 'Recent searches', js: true, feature: true do + include FilteredSearchHelpers + include WaitForAjax + + let!(:group) { create(:group) } + let!(:project) { create(:project, group: group) } + let!(:user) { create(:user) } + + before do + Capybara.ignore_hidden_elements = false + project.add_master(user) + group.add_developer(user) + create(:issue, project: project) + login_as(user) + + remove_recent_searches + end + + after do + Capybara.ignore_hidden_elements = true + end + + it 'searching adds to recent searches' do + visit namespace_project_issues_path(project.namespace, project) + + input_filtered_search('foo', submit: true) + input_filtered_search('bar', submit: true) + + items = all('.filtered-search-history-dropdown-item', visible: false) + + expect(items.count).to eq(2) + expect(items[0].text).to eq('bar') + expect(items[1].text).to eq('foo') + end + + it 'visiting URL with search params adds to recent searches' do + visit namespace_project_issues_path(project.namespace, project, label_name: 'foo', search: 'bar') + visit namespace_project_issues_path(project.namespace, project, label_name: 'qux', search: 'garply') + + items = all('.filtered-search-history-dropdown-item', visible: false) + + expect(items.count).to eq(2) + expect(items[0].text).to eq('label:~qux garply') + expect(items[1].text).to eq('label:~foo bar') + end + + it 'saved recent searches are restored last on the list' do + set_recent_searches('["saved1", "saved2"]') + + visit namespace_project_issues_path(project.namespace, project, search: 'foo') + + items = all('.filtered-search-history-dropdown-item', visible: false) + + expect(items.count).to eq(3) + expect(items[0].text).to eq('foo') + expect(items[1].text).to eq('saved1') + expect(items[2].text).to eq('saved2') + end + + it 'clicking item fills search input' do + set_recent_searches('["foo", "bar"]') + visit namespace_project_issues_path(project.namespace, project) + + all('.filtered-search-history-dropdown-item', visible: false)[0].trigger('click') + wait_for_filtered_search('foo') + + expect(find('.filtered-search').value.strip).to eq('foo') + end + + it 'clear recent searches button, clears recent searches' do + set_recent_searches('["foo"]') + visit namespace_project_issues_path(project.namespace, project) + + items_before = all('.filtered-search-history-dropdown-item', visible: false) + + expect(items_before.count).to eq(1) + + find('.filtered-search-history-clear-button', visible: false).trigger('click') + items_after = all('.filtered-search-history-dropdown-item', visible: false) + + expect(items_after.count).to eq(0) + end + + it 'shows flash error when failed to parse saved history' do + set_recent_searches('fail') + visit namespace_project_issues_path(project.namespace, project) + + expect(find('.flash-alert')).to have_text('An error occured while parsing recent searches') + end +end diff --git a/spec/features/issues/filtered_search/search_bar_spec.rb b/spec/features/issues/filtered_search/search_bar_spec.rb index 3b2b6347bc0..28137f11b92 100644 --- a/spec/features/issues/filtered_search/search_bar_spec.rb +++ b/spec/features/issues/filtered_search/search_bar_spec.rb @@ -44,7 +44,7 @@ describe 'Search bar', js: true, feature: true do filtered_search.set(search_text) expect(filtered_search.value).to eq(search_text) - find('.filtered-search-input-container .clear-search').click + find('.filtered-search-box .clear-search').click expect(filtered_search.value).to eq('') end @@ -55,7 +55,7 @@ describe 'Search bar', js: true, feature: true do it 'hides after clicked' do filtered_search.set('a') - find('.filtered-search-input-container .clear-search').click + find('.filtered-search-box .clear-search').click expect(page).to have_css('.clear-search', visible: false) end @@ -81,7 +81,7 @@ describe 'Search bar', js: true, feature: true do expect(find('#js-dropdown-hint')).to have_selector('.filter-dropdown .filter-dropdown-item', count: 1) - find('.filtered-search-input-container .clear-search').click + find('.filtered-search-box .clear-search').click filtered_search.click expect(find('#js-dropdown-hint')).to have_selector('.filter-dropdown .filter-dropdown-item', count: original_size) @@ -98,7 +98,7 @@ describe 'Search bar', js: true, feature: true do find('#js-dropdown-hint', visible: false) - find('.filtered-search-input-container .clear-search').click + find('.filtered-search-box .clear-search').click filtered_search.click expect(find('#js-dropdown-hint')).to have_selector('.filter-dropdown .filter-dropdown-item', count: 4) diff --git a/spec/features/login_spec.rb b/spec/features/login_spec.rb index f32d1f78b40..11d417c253d 100644 --- a/spec/features/login_spec.rb +++ b/spec/features/login_spec.rb @@ -199,52 +199,125 @@ feature 'Login', feature: true do describe 'with required two-factor authentication enabled' do let(:user) { create(:user) } - before(:each) { stub_application_setting(require_two_factor_authentication: true) } + # TODO: otp_grace_period_started_at - context 'with grace period defined' do - before(:each) do - stub_application_setting(two_factor_grace_period: 48) - login_with(user) - end + context 'global setting' do + before(:each) { stub_application_setting(require_two_factor_authentication: true) } - context 'within the grace period' do - it 'redirects to two-factor configuration page' do - expect(current_path).to eq profile_two_factor_auth_path - expect(page).to have_content('You must enable Two-Factor Authentication for your account before') + context 'with grace period defined' do + before(:each) do + stub_application_setting(two_factor_grace_period: 48) + login_with(user) end - it 'allows skipping two-factor configuration', js: true do - expect(current_path).to eq profile_two_factor_auth_path + context 'within the grace period' do + it 'redirects to two-factor configuration page' do + expect(current_path).to eq profile_two_factor_auth_path + expect(page).to have_content('The global settings require you to enable Two-Factor Authentication for your account. You need to do this before ') + end - click_link 'Configure it later' - expect(current_path).to eq root_path + it 'allows skipping two-factor configuration', js: true do + expect(current_path).to eq profile_two_factor_auth_path + + click_link 'Configure it later' + expect(current_path).to eq root_path + end end - end - context 'after the grace period' do - let(:user) { create(:user, otp_grace_period_started_at: 9999.hours.ago) } + context 'after the grace period' do + let(:user) { create(:user, otp_grace_period_started_at: 9999.hours.ago) } - it 'redirects to two-factor configuration page' do - expect(current_path).to eq profile_two_factor_auth_path - expect(page).to have_content('You must enable Two-Factor Authentication for your account.') + it 'redirects to two-factor configuration page' do + expect(current_path).to eq profile_two_factor_auth_path + expect(page).to have_content( + 'The global settings require you to enable Two-Factor Authentication for your account.' + ) + end + + it 'disallows skipping two-factor configuration', js: true do + expect(current_path).to eq profile_two_factor_auth_path + expect(page).not_to have_link('Configure it later') + end + end + end + + context 'without grace period defined' do + before(:each) do + stub_application_setting(two_factor_grace_period: 0) + login_with(user) end - it 'disallows skipping two-factor configuration', js: true do + it 'redirects to two-factor configuration page' do expect(current_path).to eq profile_two_factor_auth_path - expect(page).not_to have_link('Configure it later') + expect(page).to have_content( + 'The global settings require you to enable Two-Factor Authentication for your account.' + ) end end end - context 'without grace period defined' do - before(:each) do - stub_application_setting(two_factor_grace_period: 0) - login_with(user) + context 'group setting' do + before do + group1 = create :group, name: 'Group 1', require_two_factor_authentication: true + group1.add_user(user, GroupMember::DEVELOPER) + group2 = create :group, name: 'Group 2', require_two_factor_authentication: true + group2.add_user(user, GroupMember::DEVELOPER) end - it 'redirects to two-factor configuration page' do - expect(current_path).to eq profile_two_factor_auth_path - expect(page).to have_content('You must enable Two-Factor Authentication for your account.') + context 'with grace period defined' do + before(:each) do + stub_application_setting(two_factor_grace_period: 48) + login_with(user) + end + + context 'within the grace period' do + it 'redirects to two-factor configuration page' do + expect(current_path).to eq profile_two_factor_auth_path + expect(page).to have_content( + 'The group settings for Group 1 and Group 2 require you to enable ' \ + 'Two-Factor Authentication for your account. You need to do this ' \ + 'before ') + end + + it 'allows skipping two-factor configuration', js: true do + expect(current_path).to eq profile_two_factor_auth_path + + click_link 'Configure it later' + expect(current_path).to eq root_path + end + end + + context 'after the grace period' do + let(:user) { create(:user, otp_grace_period_started_at: 9999.hours.ago) } + + it 'redirects to two-factor configuration page' do + expect(current_path).to eq profile_two_factor_auth_path + expect(page).to have_content( + 'The group settings for Group 1 and Group 2 require you to enable ' \ + 'Two-Factor Authentication for your account.' + ) + end + + it 'disallows skipping two-factor configuration', js: true do + expect(current_path).to eq profile_two_factor_auth_path + expect(page).not_to have_link('Configure it later') + end + end + end + + context 'without grace period defined' do + before(:each) do + stub_application_setting(two_factor_grace_period: 0) + login_with(user) + end + + it 'redirects to two-factor configuration page' do + expect(current_path).to eq profile_two_factor_auth_path + expect(page).to have_content( + 'The group settings for Group 1 and Group 2 require you to enable ' \ + 'Two-Factor Authentication for your account.' + ) + end end end end diff --git a/spec/features/merge_requests/check_if_mergeable_with_unresolved_discussions_spec.rb b/spec/features/merge_requests/check_if_mergeable_with_unresolved_discussions_spec.rb index 7f11db3c417..77b7ba4ac7a 100644 --- a/spec/features/merge_requests/check_if_mergeable_with_unresolved_discussions_spec.rb +++ b/spec/features/merge_requests/check_if_mergeable_with_unresolved_discussions_spec.rb @@ -19,7 +19,7 @@ feature 'Check if mergeable with unresolved discussions', js: true, feature: tru it 'does not allow to merge' do visit_merge_request(merge_request) - expect(page).not_to have_button 'Accept Merge Request' + expect(page).not_to have_button 'Accept merge request' expect(page).to have_content('This merge request has unresolved discussions') end end @@ -32,7 +32,7 @@ feature 'Check if mergeable with unresolved discussions', js: true, feature: tru it 'allows MR to be merged' do visit_merge_request(merge_request) - expect(page).to have_button 'Accept Merge Request' + expect(page).to have_button 'Accept merge request' end end end @@ -46,7 +46,7 @@ feature 'Check if mergeable with unresolved discussions', js: true, feature: tru it 'does not allow to merge' do visit_merge_request(merge_request) - expect(page).to have_button 'Accept Merge Request' + expect(page).to have_button 'Accept merge request' end end @@ -58,7 +58,7 @@ feature 'Check if mergeable with unresolved discussions', js: true, feature: tru it 'allows MR to be merged' do visit_merge_request(merge_request) - expect(page).to have_button 'Accept Merge Request' + expect(page).to have_button 'Accept merge request' end end end diff --git a/spec/features/merge_requests/create_new_mr_spec.rb b/spec/features/merge_requests/create_new_mr_spec.rb index d4fe67c224f..3a4ec07b2b0 100644 --- a/spec/features/merge_requests/create_new_mr_spec.rb +++ b/spec/features/merge_requests/create_new_mr_spec.rb @@ -26,7 +26,7 @@ feature 'Create New Merge Request', feature: true, js: true do end it 'selects the target branch sha when a tag with the same name exists' do - visit namespace_project_merge_requests_path(project.namespace, project) + visit namespace_project_merge_requests_path(project.namespace, project) click_link 'New merge request' diff --git a/spec/features/merge_requests/merge_immediately_with_pipeline_spec.rb b/spec/features/merge_requests/merge_immediately_with_pipeline_spec.rb index 79105b1ee46..497240803d4 100644 --- a/spec/features/merge_requests/merge_immediately_with_pipeline_spec.rb +++ b/spec/features/merge_requests/merge_immediately_with_pipeline_spec.rb @@ -32,7 +32,7 @@ feature 'Merge immediately', :feature, :js do page.within '.mr-widget-body' do find('.dropdown-toggle').click - click_link 'Merge Immediately' + click_link 'Merge immediately' expect(find('.js-merge-when-pipeline-succeeds-button')).to have_content('Merge in progress') diff --git a/spec/features/merge_requests/merge_when_pipeline_succeeds_spec.rb b/spec/features/merge_requests/merge_when_pipeline_succeeds_spec.rb index ed7193b9777..646e7bab265 100644 --- a/spec/features/merge_requests/merge_when_pipeline_succeeds_spec.rb +++ b/spec/features/merge_requests/merge_when_pipeline_succeeds_spec.rb @@ -28,25 +28,25 @@ feature 'Merge When Pipeline Succeeds', :feature, :js do visit_merge_request(merge_request) end - it 'displays the Merge When Pipeline Succeeds button' do - expect(page).to have_button "Merge When Pipeline Succeeds" + it 'displays the Merge when pipeline succeeds button' do + expect(page).to have_button "Merge when pipeline succeeds" end - describe 'enabling Merge When Pipeline Succeeds' do - shared_examples 'Merge When Pipeline Succeeds activator' do - it 'activates the Merge When Pipeline Succeeds feature' do - click_button "Merge When Pipeline Succeeds" + describe 'enabling Merge when pipeline succeeds' do + shared_examples 'Merge when pipeline succeeds activator' do + it 'activates the Merge when pipeline succeeds feature' do + click_button "Merge when pipeline succeeds" expect(page).to have_content "Set by #{user.name} to be merged automatically when the pipeline succeeds." expect(page).to have_content "The source branch will not be removed." - expect(page).to have_link "Cancel Automatic Merge" + expect(page).to have_link "Cancel automatic merge" visit_merge_request(merge_request) # Needed to refresh the page expect(page).to have_content /enabled an automatic merge when the pipeline for \h{8} succeeds/i end end context "when enabled immediately" do - it_behaves_like 'Merge When Pipeline Succeeds activator' + it_behaves_like 'Merge when pipeline succeeds activator' end context 'when enabled after pipeline status changed' do @@ -60,16 +60,16 @@ feature 'Merge When Pipeline Succeeds', :feature, :js do expect(page).to have_content "Pipeline ##{pipeline.id} running" end - it_behaves_like 'Merge When Pipeline Succeeds activator' + it_behaves_like 'Merge when pipeline succeeds activator' end context 'when enabled after it was previously canceled' do before do - click_button "Merge When Pipeline Succeeds" - click_link "Cancel Automatic Merge" + click_button "Merge when pipeline succeeds" + click_link "Cancel automatic merge" end - it_behaves_like 'Merge When Pipeline Succeeds activator' + it_behaves_like 'Merge when pipeline succeeds activator' end context 'when it was enabled and then canceled' do @@ -83,10 +83,10 @@ feature 'Merge When Pipeline Succeeds', :feature, :js do end before do - click_link "Cancel Automatic Merge" + click_link "Cancel automatic merge" end - it_behaves_like 'Merge When Pipeline Succeeds activator' + it_behaves_like 'Merge when pipeline succeeds activator' end end end @@ -110,18 +110,18 @@ feature 'Merge When Pipeline Succeeds', :feature, :js do end it 'allows to cancel the automatic merge' do - click_link "Cancel Automatic Merge" + click_link "Cancel automatic merge" - expect(page).to have_button "Merge When Pipeline Succeeds" + expect(page).to have_button "Merge when pipeline succeeds" visit_merge_request(merge_request) # refresh the page expect(page).to have_content "canceled the automatic merge" end it "allows the user to remove the source branch" do - expect(page).to have_link "Remove Source Branch When Merged" + expect(page).to have_link "Remove source branch when merged" - click_link "Remove Source Branch When Merged" + click_link "Remove source branch when merged" expect(page).to have_content "The source branch will be removed" end @@ -141,7 +141,7 @@ feature 'Merge When Pipeline Succeeds', :feature, :js do it "does not allow to enable merge when pipeline succeeds" do visit_merge_request(merge_request) - expect(page).not_to have_link 'Merge When Pipeline Succeeds' + expect(page).not_to have_link 'Merge when pipeline succeeds' end end diff --git a/spec/features/merge_requests/only_allow_merge_if_build_succeeds_spec.rb b/spec/features/merge_requests/only_allow_merge_if_build_succeeds_spec.rb index 447764566e0..4a590e3bf68 100644 --- a/spec/features/merge_requests/only_allow_merge_if_build_succeeds_spec.rb +++ b/spec/features/merge_requests/only_allow_merge_if_build_succeeds_spec.rb @@ -14,7 +14,7 @@ feature 'Only allow merge requests to be merged if the pipeline succeeds', featu it 'allows MR to be merged' do visit_merge_request(merge_request) - expect(page).to have_button 'Accept Merge Request' + expect(page).to have_button 'Accept merge request' end end @@ -38,8 +38,8 @@ feature 'Only allow merge requests to be merged if the pipeline succeeds', featu it 'does not allow to merge immediately' do visit_merge_request(merge_request) - expect(page).to have_button 'Merge When Pipeline Succeeds' - expect(page).not_to have_button 'Select Merge Moment' + expect(page).to have_button 'Merge when pipeline succeeds' + expect(page).not_to have_button 'Select merge moment' end end @@ -49,7 +49,7 @@ feature 'Only allow merge requests to be merged if the pipeline succeeds', featu it 'does not allow MR to be merged' do visit_merge_request(merge_request) - expect(page).not_to have_button 'Accept Merge Request' + expect(page).not_to have_button 'Accept merge request' expect(page).to have_content('Please retry the job or push a new commit to fix the failure.') end end @@ -60,7 +60,7 @@ feature 'Only allow merge requests to be merged if the pipeline succeeds', featu it 'does not allow MR to be merged' do visit_merge_request(merge_request) - expect(page).not_to have_button 'Accept Merge Request' + expect(page).not_to have_button 'Accept merge request' expect(page).to have_content('Please retry the job or push a new commit to fix the failure.') end end @@ -71,7 +71,7 @@ feature 'Only allow merge requests to be merged if the pipeline succeeds', featu it 'allows MR to be merged' do visit_merge_request(merge_request) - expect(page).to have_button 'Accept Merge Request' + expect(page).to have_button 'Accept merge request' end end @@ -81,7 +81,7 @@ feature 'Only allow merge requests to be merged if the pipeline succeeds', featu it 'allows MR to be merged' do visit_merge_request(merge_request) - expect(page).to have_button 'Accept Merge Request' + expect(page).to have_button 'Accept merge request' end end end @@ -97,10 +97,10 @@ feature 'Only allow merge requests to be merged if the pipeline succeeds', featu it 'allows MR to be merged immediately', js: true do visit_merge_request(merge_request) - expect(page).to have_button 'Merge When Pipeline Succeeds' + expect(page).to have_button 'Merge when pipeline succeeds' - click_button 'Select Merge Moment' - expect(page).to have_content 'Merge Immediately' + click_button 'Select merge moment' + expect(page).to have_content 'Merge immediately' end end @@ -110,7 +110,7 @@ feature 'Only allow merge requests to be merged if the pipeline succeeds', featu it 'allows MR to be merged' do visit_merge_request(merge_request) - expect(page).to have_button 'Accept Merge Request' + expect(page).to have_button 'Accept merge request' end end @@ -120,7 +120,7 @@ feature 'Only allow merge requests to be merged if the pipeline succeeds', featu it 'allows MR to be merged' do visit_merge_request(merge_request) - expect(page).to have_button 'Accept Merge Request' + expect(page).to have_button 'Accept merge request' end end end diff --git a/spec/features/merge_requests/reset_filters_spec.rb b/spec/features/merge_requests/reset_filters_spec.rb index 14511707af4..df5943f9136 100644 --- a/spec/features/merge_requests/reset_filters_spec.rb +++ b/spec/features/merge_requests/reset_filters_spec.rb @@ -14,7 +14,7 @@ feature 'Merge requests filter clear button', feature: true, js: true do let!(:mr2) { create(:merge_request, title: "Bugfix1", source_project: project, target_project: project, source_branch: "Bugfix1") } let(:merge_request_css) { '.merge-request' } - let(:clear_search_css) { '.filtered-search-input-container .clear-search' } + let(:clear_search_css) { '.filtered-search-box .clear-search' } before do mr2.labels << bug diff --git a/spec/features/merge_requests/widget_spec.rb b/spec/features/merge_requests/widget_spec.rb index c2db7d8da3c..a62c5435748 100644 --- a/spec/features/merge_requests/widget_spec.rb +++ b/spec/features/merge_requests/widget_spec.rb @@ -145,7 +145,7 @@ describe 'Merge request', :feature, :js do before do allow_any_instance_of(Repository).to receive(:merge).and_return(false) visit namespace_project_merge_request_path(project.namespace, project, merge_request) - click_button 'Accept Merge Request' + click_button 'Accept merge request' wait_for_ajax end diff --git a/spec/features/profiles/personal_access_tokens_spec.rb b/spec/features/profiles/personal_access_tokens_spec.rb index 99fba594651..27a20e78a43 100644 --- a/spec/features/profiles/personal_access_tokens_spec.rb +++ b/spec/features/profiles/personal_access_tokens_spec.rb @@ -41,7 +41,7 @@ describe 'Profile > Personal Access Tokens', feature: true, js: true do check "api" check "read_user" - click_on "Create Personal Access Token" + click_on "Create personal access token" expect(active_personal_access_tokens).to have_text(name) expect(active_personal_access_tokens).to have_text('In') expect(active_personal_access_tokens).to have_text('api') @@ -54,7 +54,7 @@ describe 'Profile > Personal Access Tokens', feature: true, js: true do visit profile_personal_access_tokens_path fill_in "Name", with: 'My PAT' - expect { click_on "Create Personal Access Token" }.not_to change { PersonalAccessToken.count } + expect { click_on "Create personal access token" }.not_to change { PersonalAccessToken.count } expect(page).to have_content("Name cannot be nil") end end diff --git a/spec/features/projects/blobs/blob_show_spec.rb b/spec/features/projects/blobs/blob_show_spec.rb new file mode 100644 index 00000000000..01cd268ffe8 --- /dev/null +++ b/spec/features/projects/blobs/blob_show_spec.rb @@ -0,0 +1,23 @@ +require 'spec_helper' + +feature 'File blob', feature: true do + include WaitForAjax + include TreeHelper + + let(:project) { create(:project, :public, :test_repo) } + let(:merge_request) { create(:merge_request, source_project: project, source_branch: 'feature', target_branch: 'master') } + let(:branch) { 'master' } + let(:file_path) { project.repository.ls_files(project.repository.root_ref)[1] } + + context 'anonymous' do + context 'from blob file path' do + before do + visit namespace_project_blob_path(project.namespace, project, tree_join(branch, file_path)) + end + + it 'updates content' do + expect(page).to have_link 'Edit' + end + end + end +end diff --git a/spec/features/projects/blobs/edit_spec.rb b/spec/features/projects/blobs/edit_spec.rb index a820d07ab3b..aab5a72678e 100644 --- a/spec/features/projects/blobs/edit_spec.rb +++ b/spec/features/projects/blobs/edit_spec.rb @@ -2,44 +2,135 @@ require 'spec_helper' feature 'Editing file blob', feature: true, js: true do include WaitForAjax + include TreeHelper - given(:user) { create(:user) } - given(:role) { :developer } - given(:merge_request) { create(:merge_request, source_branch: 'feature', target_branch: 'master') } - given(:project) { merge_request.target_project } + let(:project) { create(:project, :public, :test_repo) } + let(:merge_request) { create(:merge_request, source_project: project, source_branch: 'feature', target_branch: 'master') } + let(:branch) { 'master' } + let(:file_path) { project.repository.ls_files(project.repository.root_ref)[1] } - background do - login_as(user) - project.team << [user, role] - end - - def edit_and_commit - wait_for_ajax - first('.file-actions').click_link 'Edit' - execute_script('ace.edit("editor").setValue("class NextFeature\nend\n")') - click_button 'Commit Changes' - end + context 'as a developer' do + let(:user) { create(:user) } + let(:role) { :developer } - context 'from MR diff' do before do - visit diffs_namespace_project_merge_request_path(project.namespace, project, merge_request) - edit_and_commit + project.team << [user, role] + login_as(user) + end + + def edit_and_commit + wait_for_ajax + find('.js-edit-blob').click + execute_script('ace.edit("editor").setValue("class NextFeature\nend\n")') + click_button 'Commit changes' + end + + context 'from MR diff' do + before do + visit diffs_namespace_project_merge_request_path(project.namespace, project, merge_request) + edit_and_commit + end + + it 'returns me to the mr' do + expect(page).to have_content(merge_request.title) + end end - scenario 'returns me to the mr' do - expect(page).to have_content(merge_request.title) + context 'from blob file path' do + before do + visit namespace_project_blob_path(project.namespace, project, tree_join(branch, file_path)) + edit_and_commit + end + + it 'updates content' do + expect(page).to have_content 'successfully committed' + expect(page).to have_content 'NextFeature' + end end end - context 'from blob file path' do - before do - visit namespace_project_blob_path(project.namespace, project, '/feature/files/ruby/feature.rb') - edit_and_commit + context 'visit blob edit' do + context 'redirects to sign in and returns' do + context 'as developer' do + let(:user) { create(:user) } + + before do + project.team << [user, :developer] + visit namespace_project_edit_blob_path(project.namespace, project, tree_join(branch, file_path)) + end + + it 'redirects to sign in and returns' do + expect(page).to have_current_path(new_user_session_path) + + login_as(user) + + expect(page).to have_current_path(namespace_project_edit_blob_path(project.namespace, project, tree_join(branch, file_path))) + end + end + + context 'as guest' do + let(:user) { create(:user) } + + before do + visit namespace_project_edit_blob_path(project.namespace, project, tree_join(branch, file_path)) + end + + it 'redirects to sign in and returns' do + expect(page).to have_current_path(new_user_session_path) + + login_as(user) + + expect(page).to have_current_path(namespace_project_blob_path(project.namespace, project, tree_join(branch, file_path))) + end + end + end + + context 'as developer' do + let(:user) { create(:user) } + let(:protected_branch) { 'protected-branch' } + + before do + project.team << [user, :developer] + project.repository.add_branch(user, protected_branch, 'master') + create(:protected_branch, project: project, name: protected_branch) + login_as(user) + end + + context 'on some branch' do + before do + visit namespace_project_edit_blob_path(project.namespace, project, tree_join(branch, file_path)) + end + + it 'shows blob editor with same branch' do + expect(page).to have_current_path(namespace_project_edit_blob_path(project.namespace, project, tree_join(branch, file_path))) + expect(find('.js-target-branch .dropdown-toggle-text').text).to eq(branch) + end + end + + context 'with protected branch' do + before do + visit namespace_project_edit_blob_path(project.namespace, project, tree_join(protected_branch, file_path)) + end + + it 'shows blob editor with patch branch' do + expect(find('.js-target-branch .dropdown-toggle-text').text).to eq('patch-1') + end + end end - scenario 'updates content' do - expect(page).to have_content 'successfully committed' - expect(page).to have_content 'NextFeature' + context 'as master' do + let(:user) { create(:user) } + + before do + project.team << [user, :master] + login_as(user) + visit namespace_project_edit_blob_path(project.namespace, project, tree_join(branch, file_path)) + end + + it 'shows blob editor with same branch' do + expect(page).to have_current_path(namespace_project_edit_blob_path(project.namespace, project, tree_join(branch, file_path))) + expect(find('.js-target-branch .dropdown-toggle-text').text).to eq(branch) + end end end end diff --git a/spec/features/projects/blobs/user_create_spec.rb b/spec/features/projects/blobs/user_create_spec.rb index d214a531138..fa1a753afcb 100644 --- a/spec/features/projects/blobs/user_create_spec.rb +++ b/spec/features/projects/blobs/user_create_spec.rb @@ -22,7 +22,7 @@ feature 'New blob creation', feature: true, js: true do end def commit_file - click_button 'Commit Changes' + click_button 'Commit changes' end context 'with default target branch' do diff --git a/spec/features/projects/builds_spec.rb b/spec/features/projects/builds_spec.rb index 2116721b224..ab10434e10c 100644 --- a/spec/features/projects/builds_spec.rb +++ b/spec/features/projects/builds_spec.rb @@ -205,21 +205,13 @@ feature 'Builds', :feature do it 'loads job trace' do expect(page).to have_content 'BUILD TRACE' - build.append_trace(' and more trace', 11) + build.trace.write do |stream| + stream.append(' and more trace', 11) + end expect(page).to have_content 'BUILD TRACE and more trace' end end - - context 'when build does not have an initial trace' do - let(:build) { create(:ci_build, pipeline: pipeline) } - - it 'loads new trace' do - build.append_trace('build trace', 0) - - expect(page).to have_content 'build trace' - end - end end feature 'Variables' do @@ -390,7 +382,7 @@ feature 'Builds', :feature do it 'sends the right headers' do expect(page.status_code).to eq(200) expect(page.response_headers['Content-Type']).to eq('text/plain; charset=utf-8') - expect(page.response_headers['X-Sendfile']).to eq(build.path_to_trace) + expect(page.response_headers['X-Sendfile']).to eq(build.trace.send(:current_path)) end end @@ -409,43 +401,24 @@ feature 'Builds', :feature do context 'storage form' do let(:existing_file) { Tempfile.new('existing-trace-file').path } - let(:non_existing_file) do - file = Tempfile.new('non-existing-trace-file') - path = file.path - file.unlink - path - end - context 'when build has trace in file' do - before do - Capybara.current_session.driver.header('X-Sendfile-Type', 'X-Sendfile') - build.run! - visit namespace_project_build_path(project.namespace, project, build) + before do + Capybara.current_session.driver.header('X-Sendfile-Type', 'X-Sendfile') - allow_any_instance_of(Project).to receive(:ci_id).and_return(nil) - allow_any_instance_of(Ci::Build).to receive(:path_to_trace).and_return(existing_file) - allow_any_instance_of(Ci::Build).to receive(:old_path_to_trace).and_return(non_existing_file) + build.run! - page.within('.js-build-sidebar') { click_link 'Raw' } - end + allow_any_instance_of(Gitlab::Ci::Trace).to receive(:paths) + .and_return(paths) - it 'sends the right headers' do - expect(page.status_code).to eq(200) - expect(page.response_headers['Content-Type']).to eq('text/plain; charset=utf-8') - expect(page.response_headers['X-Sendfile']).to eq(existing_file) - end + visit namespace_project_build_path(project.namespace, project, build) end - context 'when build has trace in old file' do - before do - Capybara.current_session.driver.header('X-Sendfile-Type', 'X-Sendfile') - build.run! - visit namespace_project_build_path(project.namespace, project, build) - - allow_any_instance_of(Project).to receive(:ci_id).and_return(999) - allow_any_instance_of(Ci::Build).to receive(:path_to_trace).and_return(non_existing_file) - allow_any_instance_of(Ci::Build).to receive(:old_path_to_trace).and_return(existing_file) + context 'when build has trace in file' do + let(:paths) do + [existing_file] + end + before do page.within('.js-build-sidebar') { click_link 'Raw' } end @@ -457,20 +430,10 @@ feature 'Builds', :feature do end context 'when build has trace in DB' do - before do - Capybara.current_session.driver.header('X-Sendfile-Type', 'X-Sendfile') - build.run! - visit namespace_project_build_path(project.namespace, project, build) - - allow_any_instance_of(Project).to receive(:ci_id).and_return(nil) - allow_any_instance_of(Ci::Build).to receive(:path_to_trace).and_return(non_existing_file) - allow_any_instance_of(Ci::Build).to receive(:old_path_to_trace).and_return(non_existing_file) - - page.within('.js-build-sidebar') { click_link 'Raw' } - end + let(:paths) { [] } it 'sends the right headers' do - expect(page.status_code).to eq(404) + expect(page.status_code).not_to have_link('Raw') end end end diff --git a/spec/features/projects/files/creating_a_file_spec.rb b/spec/features/projects/files/creating_a_file_spec.rb index ae448706130..5d7bd3dc4ce 100644 --- a/spec/features/projects/files/creating_a_file_spec.rb +++ b/spec/features/projects/files/creating_a_file_spec.rb @@ -19,7 +19,7 @@ feature 'User wants to create a file', feature: true do file_content = find('#file-content') file_content.set options[:file_content] || 'Some content' - click_button 'Commit Changes' + click_button 'Commit changes' end scenario 'file name contains Chinese characters' do diff --git a/spec/features/projects/files/editing_a_file_spec.rb b/spec/features/projects/files/editing_a_file_spec.rb index 36a80d7575d..3e544316f28 100644 --- a/spec/features/projects/files/editing_a_file_spec.rb +++ b/spec/features/projects/files/editing_a_file_spec.rb @@ -27,7 +27,7 @@ feature 'User wants to edit a file', feature: true do scenario 'file has been updated since the user opened the edit page' do Files::UpdateService.new(project, user, commit_params).execute - click_button 'Commit Changes' + click_button 'Commit changes' expect(page).to have_content 'Someone edited the file the same time you did.' end diff --git a/spec/features/projects/files/project_owner_creates_license_file_spec.rb b/spec/features/projects/files/project_owner_creates_license_file_spec.rb index 6b281e6d21d..8ff0f5898ec 100644 --- a/spec/features/projects/files/project_owner_creates_license_file_spec.rb +++ b/spec/features/projects/files/project_owner_creates_license_file_spec.rb @@ -29,7 +29,7 @@ feature 'project owner creates a license file', feature: true, js: true do expect(file_content).to have_content("Copyright (c) #{Time.now.year} #{project.namespace.human_name}") fill_in :commit_message, with: 'Add a LICENSE file', visible: true - click_button 'Commit Changes' + click_button 'Commit changes' expect(current_path).to eq( namespace_project_blob_path(project.namespace, project, 'master/LICENSE')) @@ -53,7 +53,7 @@ feature 'project owner creates a license file', feature: true, js: true do expect(file_content).to have_content("Copyright (c) #{Time.now.year} #{project.namespace.human_name}") fill_in :commit_message, with: 'Add a LICENSE file', visible: true - click_button 'Commit Changes' + click_button 'Commit changes' expect(current_path).to eq( namespace_project_blob_path(project.namespace, project, 'master/LICENSE')) @@ -63,7 +63,7 @@ feature 'project owner creates a license file', feature: true, js: true do def select_template(template) page.within('.js-license-selector-wrap') do - click_button 'Apply a License template' + click_button 'Apply a license template' click_link template wait_for_ajax end diff --git a/spec/features/projects/files/project_owner_sees_link_to_create_license_file_in_empty_project_spec.rb b/spec/features/projects/files/project_owner_sees_link_to_create_license_file_in_empty_project_spec.rb index 87322ac2584..1a1910455a1 100644 --- a/spec/features/projects/files/project_owner_sees_link_to_create_license_file_in_empty_project_spec.rb +++ b/spec/features/projects/files/project_owner_sees_link_to_create_license_file_in_empty_project_spec.rb @@ -30,7 +30,7 @@ feature 'project owner sees a link to create a license file in empty project', f fill_in :commit_message, with: 'Add a LICENSE file', visible: true # Remove pre-receive hook so we can push without auth FileUtils.rm_f(File.join(project.repository.path, 'hooks', 'pre-receive')) - click_button 'Commit Changes' + click_button 'Commit changes' expect(current_path).to eq( namespace_project_blob_path(project.namespace, project, 'master/LICENSE')) @@ -40,7 +40,7 @@ feature 'project owner sees a link to create a license file in empty project', f def select_template(template) page.within('.js-license-selector-wrap') do - click_button 'Apply a License template' + click_button 'Apply a license template' click_link template wait_for_ajax end diff --git a/spec/features/projects/files/template_type_dropdown_spec.rb b/spec/features/projects/files/template_type_dropdown_spec.rb index 5ee5e5b4c4e..9fcf12e6cb9 100644 --- a/spec/features/projects/files/template_type_dropdown_spec.rb +++ b/spec/features/projects/files/template_type_dropdown_spec.rb @@ -48,7 +48,7 @@ feature 'Template type dropdown selector', js: true do context 'user previews changes' do before do - click_link 'Preview Changes' + click_link 'Preview changes' end scenario 'type selector is hidden and shown correctly' do @@ -102,7 +102,7 @@ def check_type_selector_display(is_visible) end def try_selecting_all_types - try_selecting_template_type('LICENSE', 'Apply a License template') + try_selecting_template_type('LICENSE', 'Apply a license template') try_selecting_template_type('Dockerfile', 'Apply a Dockerfile template') try_selecting_template_type('.gitlab-ci.yml', 'Apply a GitLab CI Yaml template') try_selecting_template_type('.gitignore', 'Apply a .gitignore template') @@ -130,6 +130,6 @@ end def create_and_edit_file(file_name) visit namespace_project_new_blob_path(project.namespace, project, 'master', file_name: file_name) - click_button "Commit Changes" + click_button "Commit changes" visit namespace_project_edit_blob_path(project.namespace, project, File.join(project.default_branch, file_name)) end diff --git a/spec/features/projects/files/undo_template_spec.rb b/spec/features/projects/files/undo_template_spec.rb index 5479ea34610..c51851d3f94 100644 --- a/spec/features/projects/files/undo_template_spec.rb +++ b/spec/features/projects/files/undo_template_spec.rb @@ -17,7 +17,7 @@ feature 'Template Undo Button', js: true do end scenario 'reverts template application' do - try_template_undo('http://www.apache.org/licenses/', 'Apply a License template') + try_template_undo('http://www.apache.org/licenses/', 'Apply a license template') end end @@ -29,7 +29,7 @@ feature 'Template Undo Button', js: true do end scenario 'reverts template application' do - try_template_undo('http://www.apache.org/licenses/', 'Apply a License template') + try_template_undo('http://www.apache.org/licenses/', 'Apply a license template') end end end diff --git a/spec/features/projects/merge_request_button_spec.rb b/spec/features/projects/merge_request_button_spec.rb index b6728960fb8..05f3162f13c 100644 --- a/spec/features/projects/merge_request_button_spec.rb +++ b/spec/features/projects/merge_request_button_spec.rb @@ -1,13 +1,13 @@ require 'spec_helper' feature 'Merge Request button', feature: true do - shared_examples 'Merge Request button only shown when allowed' do + shared_examples 'Merge request button only shown when allowed' do let(:user) { create(:user) } let(:project) { create(:project, :public) } let(:forked_project) { create(:project, :public, forked_from_project: project) } context 'not logged in' do - it 'does not show Create Merge Request button' do + it 'does not show Create merge request button' do visit url within("#content-body") do @@ -22,7 +22,7 @@ feature 'Merge Request button', feature: true do project.team << [user, :developer] end - it 'shows Create Merge Request button' do + it 'shows Create merge request button' do href = new_namespace_project_merge_request_path(project.namespace, project, merge_request: { source_branch: 'feature', @@ -40,7 +40,7 @@ feature 'Merge Request button', feature: true do project.project_feature.update!(merge_requests_access_level: ProjectFeature::DISABLED) end - it 'does not show Create Merge Request button' do + it 'does not show Create merge request button' do visit url within("#content-body") do @@ -55,7 +55,7 @@ feature 'Merge Request button', feature: true do login_as(user) end - it 'does not show Create Merge Request button' do + it 'does not show Create merge request button' do visit url within("#content-body") do @@ -66,7 +66,7 @@ feature 'Merge Request button', feature: true do context 'on own fork of project' do let(:user) { forked_project.owner } - it 'shows Create Merge Request button' do + it 'shows Create merge request button' do href = new_namespace_project_merge_request_path(forked_project.namespace, forked_project, merge_request: { source_branch: 'feature', @@ -83,24 +83,24 @@ feature 'Merge Request button', feature: true do end context 'on branches page' do - it_behaves_like 'Merge Request button only shown when allowed' do - let(:label) { 'Merge Request' } + it_behaves_like 'Merge request button only shown when allowed' do + let(:label) { 'Merge request' } let(:url) { namespace_project_branches_path(project.namespace, project) } let(:fork_url) { namespace_project_branches_path(forked_project.namespace, forked_project) } end end context 'on compare page' do - it_behaves_like 'Merge Request button only shown when allowed' do - let(:label) { 'Create Merge Request' } + it_behaves_like 'Merge request button only shown when allowed' do + let(:label) { 'Create merge request' } let(:url) { namespace_project_compare_path(project.namespace, project, from: 'master', to: 'feature') } let(:fork_url) { namespace_project_compare_path(forked_project.namespace, forked_project, from: 'master', to: 'feature') } end end context 'on commits page' do - it_behaves_like 'Merge Request button only shown when allowed' do - let(:label) { 'Create Merge Request' } + it_behaves_like 'Merge request button only shown when allowed' do + let(:label) { 'Create merge request' } let(:url) { namespace_project_commits_path(project.namespace, project, 'feature') } let(:fork_url) { namespace_project_commits_path(forked_project.namespace, forked_project, 'feature') } end diff --git a/spec/features/projects/new_project_spec.rb b/spec/features/projects/new_project_spec.rb index 52196ce49bd..c66b9a34b86 100644 --- a/spec/features/projects/new_project_spec.rb +++ b/spec/features/projects/new_project_spec.rb @@ -71,6 +71,22 @@ feature "New project", feature: true do end end end + + context "with subgroup namespace" do + let(:group) { create(:group, :private, owner: user) } + let(:subgroup) { create(:group, parent: group) } + + before do + group.add_master(user) + visit new_project_path(namespace_id: subgroup.id) + end + + it "selects the group namespace" do + namespace = find("#project_namespace_id option[selected]") + + expect(namespace.text).to eq subgroup.full_path + end + end end context 'Import project options' do diff --git a/spec/features/projects/settings/pipelines_settings_spec.rb b/spec/features/projects/settings/pipelines_settings_spec.rb index 76cb240ea98..035c57eaa47 100644 --- a/spec/features/projects/settings/pipelines_settings_spec.rb +++ b/spec/features/projects/settings/pipelines_settings_spec.rb @@ -32,5 +32,16 @@ feature "Pipelines settings", feature: true do expect(page).to have_button('Save changes', disabled: false) expect(page).to have_field('Test coverage parsing', with: 'coverage_regex') end + + scenario 'updates auto_cancel_pending_pipelines' do + page.check('Auto-cancel redundant, pending pipelines') + click_on 'Save changes' + + expect(page.status_code).to eq(200) + expect(page).to have_button('Save changes', disabled: false) + + checkbox = find_field('project_auto_cancel_pending_pipelines') + expect(checkbox).to be_checked + end end end diff --git a/spec/features/projects/wiki/markdown_preview_spec.rb b/spec/features/projects/wiki/markdown_preview_spec.rb index a1c386ddc18..43d8b45669e 100644 --- a/spec/features/projects/wiki/markdown_preview_spec.rb +++ b/spec/features/projects/wiki/markdown_preview_spec.rb @@ -24,12 +24,16 @@ feature 'Projects > Wiki > User previews markdown changes', feature: true, js: t context "while creating a new wiki page" do context "when there are no spaces or hyphens in the page name" do it "rewrites relative links as expected" do - click_link 'New Page' - fill_in :new_wiki_path, with: 'a/b/c/d' - click_button 'Create Page' + click_link 'New page' + page.within '#modal-new-wiki' do + fill_in :new_wiki_path, with: 'a/b/c/d' + click_button 'Create page' + end - fill_in :wiki_content, with: wiki_content - click_on "Preview" + page.within '.wiki-form' do + fill_in :wiki_content, with: wiki_content + click_on "Preview" + end expect(page).to have_content("regular link") @@ -42,12 +46,16 @@ feature 'Projects > Wiki > User previews markdown changes', feature: true, js: t context "when there are spaces in the page name" do it "rewrites relative links as expected" do - click_link 'New Page' - fill_in :new_wiki_path, with: 'a page/b page/c page/d page' - click_button 'Create Page' + click_link 'New page' + page.within '#modal-new-wiki' do + fill_in :new_wiki_path, with: 'a page/b page/c page/d page' + click_button 'Create page' + end - fill_in :wiki_content, with: wiki_content - click_on "Preview" + page.within '.wiki-form' do + fill_in :wiki_content, with: wiki_content + click_on "Preview" + end expect(page).to have_content("regular link") @@ -60,12 +68,16 @@ feature 'Projects > Wiki > User previews markdown changes', feature: true, js: t context "when there are hyphens in the page name" do it "rewrites relative links as expected" do - click_link 'New Page' - fill_in :new_wiki_path, with: 'a-page/b-page/c-page/d-page' - click_button 'Create Page' - - fill_in :wiki_content, with: wiki_content - click_on "Preview" + click_link 'New page' + page.within '#modal-new-wiki' do + fill_in :new_wiki_path, with: 'a-page/b-page/c-page/d-page' + click_button 'Create page' + end + + page.within '.wiki-form' do + fill_in :wiki_content, with: wiki_content + click_on "Preview" + end expect(page).to have_content("regular link") @@ -79,11 +91,17 @@ feature 'Projects > Wiki > User previews markdown changes', feature: true, js: t context "while editing a wiki page" do def create_wiki_page(path) - click_link 'New Page' - fill_in :new_wiki_path, with: path - click_button 'Create Page' - fill_in :wiki_content, with: 'content' - click_on "Create page" + click_link 'New page' + + page.within '#modal-new-wiki' do + fill_in :new_wiki_path, with: path + click_button 'Create page' + end + + page.within '.wiki-form' do + fill_in :wiki_content, with: 'content' + click_on "Create page" + end end context "when there are no spaces or hyphens in the page name" do diff --git a/spec/features/projects/wiki/user_creates_wiki_page_spec.rb b/spec/features/projects/wiki/user_creates_wiki_page_spec.rb index 7bdaafd1beb..1ffac8cd542 100644 --- a/spec/features/projects/wiki/user_creates_wiki_page_spec.rb +++ b/spec/features/projects/wiki/user_creates_wiki_page_spec.rb @@ -21,8 +21,9 @@ feature 'Projects > Wiki > User creates wiki page', feature: true do scenario 'directly from the wiki home page' do fill_in :wiki_content, with: 'My awesome wiki!' - click_button 'Create page' - + page.within '.wiki-form' do + click_button 'Create page' + end expect(page).to have_content('Home') expect(page).to have_content("Last edited by #{user.name}") expect(page).to have_content('My awesome wiki!') @@ -36,16 +37,20 @@ feature 'Projects > Wiki > User creates wiki page', feature: true do context 'via the "new wiki page" page' do scenario 'when the wiki page has a single word name', js: true do - click_link 'New Page' + click_link 'New page' - fill_in :new_wiki_path, with: 'foo' - click_button 'Create Page' + page.within '#modal-new-wiki' do + fill_in :new_wiki_path, with: 'foo' + click_button 'Create page' + end # Commit message field should have correct value. expect(page).to have_field('wiki[message]', with: 'Create foo') - fill_in :wiki_content, with: 'My awesome wiki!' - click_button 'Create page' + page.within '.wiki-form' do + fill_in :wiki_content, with: 'My awesome wiki!' + click_button 'Create page' + end expect(page).to have_content('Foo') expect(page).to have_content("Last edited by #{user.name}") @@ -53,16 +58,20 @@ feature 'Projects > Wiki > User creates wiki page', feature: true do end scenario 'when the wiki page has spaces in the name', js: true do - click_link 'New Page' + click_link 'New page' - fill_in :new_wiki_path, with: 'Spaces in the name' - click_button 'Create Page' + page.within '#modal-new-wiki' do + fill_in :new_wiki_path, with: 'Spaces in the name' + click_button 'Create page' + end # Commit message field should have correct value. expect(page).to have_field('wiki[message]', with: 'Create spaces in the name') - fill_in :wiki_content, with: 'My awesome wiki!' - click_button 'Create page' + page.within '.wiki-form' do + fill_in :wiki_content, with: 'My awesome wiki!' + click_button 'Create page' + end expect(page).to have_content('Spaces in the name') expect(page).to have_content("Last edited by #{user.name}") @@ -70,16 +79,20 @@ feature 'Projects > Wiki > User creates wiki page', feature: true do end scenario 'when the wiki page has hyphens in the name', js: true do - click_link 'New Page' + click_link 'New page' - fill_in :new_wiki_path, with: 'hyphens-in-the-name' - click_button 'Create Page' + page.within '#modal-new-wiki' do + fill_in :new_wiki_path, with: 'hyphens-in-the-name' + click_button 'Create page' + end # Commit message field should have correct value. expect(page).to have_field('wiki[message]', with: 'Create hyphens in the name') - fill_in :wiki_content, with: 'My awesome wiki!' - click_button 'Create page' + page.within '.wiki-form' do + fill_in :wiki_content, with: 'My awesome wiki!' + click_button 'Create page' + end expect(page).to have_content('Hyphens in the name') expect(page).to have_content("Last edited by #{user.name}") @@ -99,7 +112,9 @@ feature 'Projects > Wiki > User creates wiki page', feature: true do scenario 'directly from the wiki home page' do fill_in :wiki_content, with: 'My awesome wiki!' - click_button 'Create page' + page.within '.wiki-form' do + click_button 'Create page' + end expect(page).to have_content('Home') expect(page).to have_content("Last edited by #{user.name}") @@ -113,16 +128,20 @@ feature 'Projects > Wiki > User creates wiki page', feature: true do end scenario 'via the "new wiki page" page', js: true do - click_link 'New Page' + click_link 'New page' - fill_in :new_wiki_path, with: 'foo' - click_button 'Create Page' + page.within '#modal-new-wiki' do + fill_in :new_wiki_path, with: 'foo' + click_button 'Create page' + end # Commit message field should have correct value. expect(page).to have_field('wiki[message]', with: 'Create foo') - fill_in :wiki_content, with: 'My awesome wiki!' - click_button 'Create page' + page.within '.wiki-form' do + fill_in :wiki_content, with: 'My awesome wiki!' + click_button 'Create page' + end expect(page).to have_content('Foo') expect(page).to have_content("Last edited by #{user.name}") diff --git a/spec/features/security/project/internal_access_spec.rb b/spec/features/security/project/internal_access_spec.rb index 1a66d1a6a1e..6ecdc8cbb71 100644 --- a/spec/features/security/project/internal_access_spec.rb +++ b/spec/features/security/project/internal_access_spec.rb @@ -443,9 +443,12 @@ describe "Internal Project Access", feature: true do end describe "GET /:project_path/container_registry" do + let(:container_repository) { create(:container_repository) } + before do - stub_container_registry_tags('latest') + stub_container_registry_tags(repository: :any, tags: ['latest']) stub_container_registry_config(enabled: true) + project.container_repositories << container_repository end subject { namespace_project_container_registry_index_path(project.namespace, project) } diff --git a/spec/features/security/project/private_access_spec.rb b/spec/features/security/project/private_access_spec.rb index ad3bd60a313..a8fc0624588 100644 --- a/spec/features/security/project/private_access_spec.rb +++ b/spec/features/security/project/private_access_spec.rb @@ -432,9 +432,12 @@ describe "Private Project Access", feature: true do end describe "GET /:project_path/container_registry" do + let(:container_repository) { create(:container_repository) } + before do - stub_container_registry_tags('latest') + stub_container_registry_tags(repository: :any, tags: ['latest']) stub_container_registry_config(enabled: true) + project.container_repositories << container_repository end subject { namespace_project_container_registry_index_path(project.namespace, project) } diff --git a/spec/features/security/project/public_access_spec.rb b/spec/features/security/project/public_access_spec.rb index e06aab4e0b2..c4d2f50ca14 100644 --- a/spec/features/security/project/public_access_spec.rb +++ b/spec/features/security/project/public_access_spec.rb @@ -443,9 +443,12 @@ describe "Public Project Access", feature: true do end describe "GET /:project_path/container_registry" do + let(:container_repository) { create(:container_repository) } + before do - stub_container_registry_tags('latest') + stub_container_registry_tags(repository: :any, tags: ['latest']) stub_container_registry_config(enabled: true) + project.container_repositories << container_repository end subject { namespace_project_container_registry_index_path(project.namespace, project) } diff --git a/spec/features/tags/master_views_tags_spec.rb b/spec/features/tags/master_views_tags_spec.rb index 555f84c4772..922ac15a2eb 100644 --- a/spec/features/tags/master_views_tags_spec.rb +++ b/spec/features/tags/master_views_tags_spec.rb @@ -16,7 +16,7 @@ feature 'Master views tags', feature: true do fill_in :commit_message, with: 'Add a README file', visible: true # Remove pre-receive hook so we can push without auth FileUtils.rm_f(File.join(project.repository.path, 'hooks', 'pre-receive')) - click_button 'Commit Changes' + click_button 'Commit changes' visit namespace_project_tags_path(project.namespace, project) end diff --git a/spec/features/u2f_spec.rb b/spec/features/u2f_spec.rb index 28373098123..c877cfdd978 100644 --- a/spec/features/u2f_spec.rb +++ b/spec/features/u2f_spec.rb @@ -6,18 +6,18 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', :js do before { allow_any_instance_of(U2fHelper).to receive(:inject_u2f_api?).and_return(true) } def manage_two_factor_authentication - click_on 'Manage Two-Factor Authentication' - expect(page).to have_content("Setup New U2F Device") + click_on 'Manage two-factor authentication' + expect(page).to have_content("Setup new U2F device") wait_for_ajax end def register_u2f_device(u2f_device = nil, name: 'My device') u2f_device ||= FakeU2fDevice.new(page, name) u2f_device.respond_to_u2f_registration - click_on 'Setup New U2F Device' + click_on 'Setup new U2F device' expect(page).to have_content('Your device was successfully set up') fill_in "Pick a name", with: name - click_on 'Register U2F Device' + click_on 'Register U2F device' u2f_device end @@ -34,9 +34,9 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', :js do it 'does not allow registering a new device' do visit profile_account_path - click_on 'Enable Two-Factor Authentication' + click_on 'Enable two-factor authentication' - expect(page).to have_button('Setup New U2F Device', disabled: true) + expect(page).to have_button('Setup new U2F device', disabled: true) end end @@ -111,9 +111,9 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', :js do # Have the "u2f device" respond with bad data page.execute_script("u2f.register = function(_,_,_,callback) { callback('bad response'); };") - click_on 'Setup New U2F Device' + click_on 'Setup new U2F device' expect(page).to have_content('Your device was successfully set up') - click_on 'Register U2F Device' + click_on 'Register U2F device' expect(U2fRegistration.count).to eq(0) expect(page).to have_content("The form contains the following error") @@ -126,9 +126,9 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', :js do # Failed registration page.execute_script("u2f.register = function(_,_,_,callback) { callback('bad response'); };") - click_on 'Setup New U2F Device' + click_on 'Setup new U2F device' expect(page).to have_content('Your device was successfully set up') - click_on 'Register U2F Device' + click_on 'Register U2F device' expect(page).to have_content("The form contains the following error") # Successful registration diff --git a/spec/helpers/blob_helper_spec.rb b/spec/helpers/blob_helper_spec.rb index bead7948486..508aeb7cf67 100644 --- a/spec/helpers/blob_helper_spec.rb +++ b/spec/helpers/blob_helper_spec.rb @@ -73,7 +73,7 @@ describe BlobHelper do let(:project) { create(:project, :repository, namespace: namespace) } before do - allow(self).to receive(:current_user).and_return(double) + allow(self).to receive(:current_user).and_return(nil) allow(self).to receive(:can_collaborate_with_project?).and_return(true) end diff --git a/spec/javascripts/build_spec.js b/spec/javascripts/build_spec.js index beee6cb2969..cb48f14fd9a 100644 --- a/spec/javascripts/build_spec.js +++ b/spec/javascripts/build_spec.js @@ -72,12 +72,14 @@ describe('Build', () => { it('displays the initial build trace', () => { expect($.ajax.calls.count()).toBe(1); const [{ url, dataType, success, context }] = $.ajax.calls.argsFor(0); - expect(url).toBe(`${BUILD_URL}.json`); + expect(url).toBe( + `${BUILD_URL}/trace.json`, + ); expect(dataType).toBe('json'); expect(success).toEqual(jasmine.any(Function)); spyOn(gl.utils, 'setCiStatusFavicon').and.callFake(() => {}); - success.call(context, { trace_html: '<span>Example</span>', status: 'running' }); + success.call(context, { html: '<span>Example</span>', status: 'running' }); expect($('#build-trace .js-build-output').text()).toMatch(/Example/); }); diff --git a/spec/javascripts/filtered_search/components/recent_searches_dropdown_content_spec.js b/spec/javascripts/filtered_search/components/recent_searches_dropdown_content_spec.js new file mode 100644 index 00000000000..2722882375f --- /dev/null +++ b/spec/javascripts/filtered_search/components/recent_searches_dropdown_content_spec.js @@ -0,0 +1,166 @@ +import Vue from 'vue'; +import eventHub from '~/filtered_search/event_hub'; +import RecentSearchesDropdownContent from '~/filtered_search/components/recent_searches_dropdown_content'; + +const createComponent = (propsData) => { + const Component = Vue.extend(RecentSearchesDropdownContent); + + return new Component({ + el: document.createElement('div'), + propsData, + }); +}; + +// Remove all the newlines and whitespace from the formatted markup +const trimMarkupWhitespace = text => text.replace(/(\n|\s)+/gm, ' ').trim(); + +describe('RecentSearchesDropdownContent', () => { + const propsDataWithoutItems = { + items: [], + }; + const propsDataWithItems = { + items: [ + 'foo', + 'author:@root label:~foo bar', + ], + }; + + let vm; + afterEach(() => { + if (vm) { + vm.$destroy(); + } + }); + + describe('with no items', () => { + let el; + + beforeEach(() => { + vm = createComponent(propsDataWithoutItems); + el = vm.$el; + }); + + it('should render empty state', () => { + expect(el.querySelector('.dropdown-info-note')).toBeDefined(); + + const items = el.querySelectorAll('.filtered-search-history-dropdown-item'); + expect(items.length).toEqual(propsDataWithoutItems.items.length); + }); + }); + + describe('with items', () => { + let el; + + beforeEach(() => { + vm = createComponent(propsDataWithItems); + el = vm.$el; + }); + + it('should render clear recent searches button', () => { + expect(el.querySelector('.filtered-search-history-clear-button')).toBeDefined(); + }); + + it('should render recent search items', () => { + const items = el.querySelectorAll('.filtered-search-history-dropdown-item'); + expect(items.length).toEqual(propsDataWithItems.items.length); + + expect(trimMarkupWhitespace(items[0].querySelector('.filtered-search-history-dropdown-search-token').textContent)).toEqual('foo'); + + const item1Tokens = items[1].querySelectorAll('.filtered-search-history-dropdown-token'); + expect(item1Tokens.length).toEqual(2); + expect(item1Tokens[0].querySelector('.name').textContent).toEqual('author:'); + expect(item1Tokens[0].querySelector('.value').textContent).toEqual('@root'); + expect(item1Tokens[1].querySelector('.name').textContent).toEqual('label:'); + expect(item1Tokens[1].querySelector('.value').textContent).toEqual('~foo'); + expect(trimMarkupWhitespace(items[1].querySelector('.filtered-search-history-dropdown-search-token').textContent)).toEqual('bar'); + }); + }); + + describe('computed', () => { + describe('processedItems', () => { + it('with items', () => { + vm = createComponent(propsDataWithItems); + const processedItems = vm.processedItems; + + expect(processedItems.length).toEqual(2); + + expect(processedItems[0].text).toEqual(propsDataWithItems.items[0]); + expect(processedItems[0].tokens).toEqual([]); + expect(processedItems[0].searchToken).toEqual('foo'); + + expect(processedItems[1].text).toEqual(propsDataWithItems.items[1]); + expect(processedItems[1].tokens.length).toEqual(2); + expect(processedItems[1].tokens[0].prefix).toEqual('author:'); + expect(processedItems[1].tokens[0].suffix).toEqual('@root'); + expect(processedItems[1].tokens[1].prefix).toEqual('label:'); + expect(processedItems[1].tokens[1].suffix).toEqual('~foo'); + expect(processedItems[1].searchToken).toEqual('bar'); + }); + + it('with no items', () => { + vm = createComponent(propsDataWithoutItems); + const processedItems = vm.processedItems; + + expect(processedItems.length).toEqual(0); + }); + }); + + describe('hasItems', () => { + it('with items', () => { + vm = createComponent(propsDataWithItems); + const hasItems = vm.hasItems; + expect(hasItems).toEqual(true); + }); + + it('with no items', () => { + vm = createComponent(propsDataWithoutItems); + const hasItems = vm.hasItems; + expect(hasItems).toEqual(false); + }); + }); + }); + + describe('methods', () => { + describe('onItemActivated', () => { + let onRecentSearchesItemSelectedSpy; + + beforeEach(() => { + onRecentSearchesItemSelectedSpy = jasmine.createSpy('spy'); + eventHub.$on('recentSearchesItemSelected', onRecentSearchesItemSelectedSpy); + + vm = createComponent(propsDataWithItems); + }); + + afterEach(() => { + eventHub.$off('recentSearchesItemSelected', onRecentSearchesItemSelectedSpy); + }); + + it('emits event', () => { + expect(onRecentSearchesItemSelectedSpy).not.toHaveBeenCalled(); + vm.onItemActivated('something'); + expect(onRecentSearchesItemSelectedSpy).toHaveBeenCalledWith('something'); + }); + }); + + describe('onRequestClearRecentSearches', () => { + let onRequestClearRecentSearchesSpy; + + beforeEach(() => { + onRequestClearRecentSearchesSpy = jasmine.createSpy('spy'); + eventHub.$on('requestClearRecentSearches', onRequestClearRecentSearchesSpy); + + vm = createComponent(propsDataWithItems); + }); + + afterEach(() => { + eventHub.$off('requestClearRecentSearches', onRequestClearRecentSearchesSpy); + }); + + it('emits event', () => { + expect(onRequestClearRecentSearchesSpy).not.toHaveBeenCalled(); + vm.onRequestClearRecentSearches({ stopPropagation: () => {} }); + expect(onRequestClearRecentSearchesSpy).toHaveBeenCalled(); + }); + }); + }); +}); diff --git a/spec/javascripts/filtered_search/filtered_search_manager_spec.js b/spec/javascripts/filtered_search/filtered_search_manager_spec.js index 5f7c05e9014..97af681429b 100644 --- a/spec/javascripts/filtered_search/filtered_search_manager_spec.js +++ b/spec/javascripts/filtered_search/filtered_search_manager_spec.js @@ -29,7 +29,7 @@ const FilteredSearchSpecHelper = require('../helpers/filtered_search_spec_helper beforeEach(() => { setFixtures(` - <div class="filtered-search-input-container"> + <div class="filtered-search-box"> <form> <ul class="tokens-container list-unstyled"> ${FilteredSearchSpecHelper.createInputHTML(placeholder)} @@ -264,12 +264,12 @@ const FilteredSearchSpecHelper = require('../helpers/filtered_search_spec_helper describe('toggleInputContainerFocus', () => { it('toggles on focus', () => { input.focus(); - expect(document.querySelector('.filtered-search-input-container').classList.contains('focus')).toEqual(true); + expect(document.querySelector('.filtered-search-box').classList.contains('focus')).toEqual(true); }); it('toggles on blur', () => { input.blur(); - expect(document.querySelector('.filtered-search-input-container').classList.contains('focus')).toEqual(false); + expect(document.querySelector('.filtered-search-box').classList.contains('focus')).toEqual(false); }); }); }); diff --git a/spec/javascripts/filtered_search/services/recent_searches_service_spec.js b/spec/javascripts/filtered_search/services/recent_searches_service_spec.js new file mode 100644 index 00000000000..2a58fb3a7df --- /dev/null +++ b/spec/javascripts/filtered_search/services/recent_searches_service_spec.js @@ -0,0 +1,56 @@ +import RecentSearchesService from '~/filtered_search/services/recent_searches_service'; + +describe('RecentSearchesService', () => { + let service; + + beforeEach(() => { + service = new RecentSearchesService(); + window.localStorage.removeItem(service.localStorageKey); + }); + + describe('fetch', () => { + it('should default to empty array', (done) => { + const fetchItemsPromise = service.fetch(); + + fetchItemsPromise + .then((items) => { + expect(items).toEqual([]); + done(); + }) + .catch((err) => { + done.fail('Shouldn\'t reject with empty localStorage key', err); + }); + }); + + it('should reject when unable to parse', (done) => { + window.localStorage.setItem(service.localStorageKey, 'fail'); + const fetchItemsPromise = service.fetch(); + + fetchItemsPromise + .catch(() => { + done(); + }); + }); + + it('should return items from localStorage', (done) => { + window.localStorage.setItem(service.localStorageKey, '["foo", "bar"]'); + const fetchItemsPromise = service.fetch(); + + fetchItemsPromise + .then((items) => { + expect(items).toEqual(['foo', 'bar']); + done(); + }); + }); + }); + + describe('setRecentSearches', () => { + it('should save things in localStorage', () => { + const items = ['foo', 'bar']; + service.save(items); + const newLocalStorageValue = + window.localStorage.getItem(service.localStorageKey); + expect(JSON.parse(newLocalStorageValue)).toEqual(items); + }); + }); +}); diff --git a/spec/javascripts/filtered_search/stores/recent_searches_store_spec.js b/spec/javascripts/filtered_search/stores/recent_searches_store_spec.js new file mode 100644 index 00000000000..1eebc6f2367 --- /dev/null +++ b/spec/javascripts/filtered_search/stores/recent_searches_store_spec.js @@ -0,0 +1,59 @@ +import RecentSearchesStore from '~/filtered_search/stores/recent_searches_store'; + +describe('RecentSearchesStore', () => { + let store; + + beforeEach(() => { + store = new RecentSearchesStore(); + }); + + describe('addRecentSearch', () => { + it('should add to the front of the list', () => { + store.addRecentSearch('foo'); + store.addRecentSearch('bar'); + + expect(store.state.recentSearches).toEqual(['bar', 'foo']); + }); + + it('should deduplicate', () => { + store.addRecentSearch('foo'); + store.addRecentSearch('bar'); + store.addRecentSearch('foo'); + + expect(store.state.recentSearches).toEqual(['foo', 'bar']); + }); + + it('only keeps track of 5 items', () => { + store.addRecentSearch('1'); + store.addRecentSearch('2'); + store.addRecentSearch('3'); + store.addRecentSearch('4'); + store.addRecentSearch('5'); + store.addRecentSearch('6'); + store.addRecentSearch('7'); + + expect(store.state.recentSearches).toEqual(['7', '6', '5', '4', '3']); + }); + }); + + describe('setRecentSearches', () => { + it('should override list', () => { + store.setRecentSearches([ + 'foo', + 'bar', + ]); + store.setRecentSearches([ + 'baz', + 'qux', + ]); + + expect(store.state.recentSearches).toEqual(['baz', 'qux']); + }); + + it('only keeps track of 5 items', () => { + store.setRecentSearches(['1', '2', '3', '4', '5', '6', '7']); + + expect(store.state.recentSearches).toEqual(['1', '2', '3', '4', '5']); + }); + }); +}); diff --git a/spec/javascripts/fixtures/environments/metrics.html.haml b/spec/javascripts/fixtures/environments/metrics.html.haml index 483063fb889..e2dd9519898 100644 --- a/spec/javascripts/fixtures/environments/metrics.html.haml +++ b/spec/javascripts/fixtures/environments/metrics.html.haml @@ -1,12 +1,62 @@ -%div +.prometheus-container{ 'data-has-metrics': "false", 'data-doc-link': '/help/administration/monitoring/prometheus/index.md', 'data-prometheus-integration': '/root/hello-prometheus/services/prometheus/edit' } .top-area .row .col-sm-6 %h3.page-title Metrics for environment - .row - .col-sm-12 - %svg.prometheus-graph{ 'graph-type' => 'cpu_values' } - .row - .col-sm-12 - %svg.prometheus-graph{ 'graph-type' => 'memory_values' }
\ No newline at end of file + .prometheus-state + .js-getting-started.hidden + .row + .col-md-4.col-md-offset-4.state-svg + %svg + .row + .col-md-6.col-md-offset-3 + %h4.text-center.state-title + Get started with performance monitoring + .row + .col-md-6.col-md-offset-3 + .description-text.text-center.state-description + Stay updated about the performance and health of your environment by configuring Prometheus to monitor your deployments. Learn more about performance monitoring + .row.state-button-section + .col-md-4.col-md-offset-4.text-center.state-button + %a.btn.btn-success + Configure Prometheus + .js-loading.hidden + .row + .col-md-4.col-md-offset-4.state-svg + %svg + .row + .col-md-6.col-md-offset-3 + %h4.text-center.state-title + Waiting for performance data + .row + .col-md-6.col-md-offset-3 + .description-text.text-center.state-description + Creating graphs uses the data from the Prometheus server. If this takes a long time, ensure that data is available. + .row.state-button-section + .col-md-4.col-md-offset-4.text-center.state-button + %a.btn.btn-success + View documentation + .js-unable-to-connect.hidden + .row + .col-md-4.col-md-offset-4.state-svg + %svg + .row + .col-md-6.col-md-offset-3 + %h4.text-center.state-title + Unable to connect to Prometheus server + .row + .col-md-6.col-md-offset-3 + .description-text.text-center.state-description + Ensure connectivity is available from the GitLab server to the Prometheus server + .row.state-button-section + .col-md-4.col-md-offset-4.text-center.state-button + %a.btn.btn-success + View documentation + .prometheus-graphs + .row + .col-sm-12 + %svg.prometheus-graph{ 'graph-type' => 'cpu_values' } + .row + .col-sm-12 + %svg.prometheus-graph{ 'graph-type' => 'memory_values' } diff --git a/spec/javascripts/merge_request_tabs_spec.js b/spec/javascripts/merge_request_tabs_spec.js index 7b9632be84e..5354e536edc 100644 --- a/spec/javascripts/merge_request_tabs_spec.js +++ b/spec/javascripts/merge_request_tabs_spec.js @@ -3,6 +3,9 @@ require('~/merge_request_tabs'); require('~/breakpoints'); require('~/lib/utils/common_utils'); +require('~/diff'); +require('~/single_file_diff'); +require('~/files_comment_button'); require('vendor/jquery.scrollTo'); (function () { @@ -39,7 +42,8 @@ require('vendor/jquery.scrollTo'); }); afterEach(function () { - this.class.destroy(); + this.class.unbindEvents(); + this.class.destroyPipelinesView(); }); describe('#activateTab', function () { @@ -65,6 +69,7 @@ require('vendor/jquery.scrollTo'); expect($('#diffs')).toHaveClass('active'); }); }); + describe('#opensInNewTab', function () { var tabUrl; var windowTarget = '_blank'; @@ -116,6 +121,7 @@ require('vendor/jquery.scrollTo'); stopImmediatePropagation: function () {} }); }); + it('opens page tab in a new browser tab with Cmd+Click - Mac', function () { spyOn(window, 'open').and.callFake(function (url, name) { expect(url).toEqual(tabUrl); @@ -129,6 +135,7 @@ require('vendor/jquery.scrollTo'); stopImmediatePropagation: function () {} }); }); + it('opens page tab in a new browser tab with Middle-click - Mac/PC', function () { spyOn(window, 'open').and.callFake(function (url, name) { expect(url).toEqual(tabUrl); @@ -149,6 +156,7 @@ require('vendor/jquery.scrollTo'); spyOn($, 'ajax').and.callFake(function () {}); this.subject = this.class.setCurrentAction; }); + it('changes from commits', function () { setLocation({ pathname: '/foo/bar/merge_requests/1/commits' @@ -156,13 +164,16 @@ require('vendor/jquery.scrollTo'); expect(this.subject('notes')).toBe('/foo/bar/merge_requests/1'); expect(this.subject('diffs')).toBe('/foo/bar/merge_requests/1/diffs'); }); + it('changes from diffs', function () { setLocation({ pathname: '/foo/bar/merge_requests/1/diffs' }); + expect(this.subject('notes')).toBe('/foo/bar/merge_requests/1'); expect(this.subject('commits')).toBe('/foo/bar/merge_requests/1/commits'); }); + it('changes from diffs.html', function () { setLocation({ pathname: '/foo/bar/merge_requests/1/diffs.html' @@ -170,6 +181,7 @@ require('vendor/jquery.scrollTo'); expect(this.subject('notes')).toBe('/foo/bar/merge_requests/1'); expect(this.subject('commits')).toBe('/foo/bar/merge_requests/1/commits'); }); + it('changes from notes', function () { setLocation({ pathname: '/foo/bar/merge_requests/1' @@ -177,6 +189,7 @@ require('vendor/jquery.scrollTo'); expect(this.subject('diffs')).toBe('/foo/bar/merge_requests/1/diffs'); expect(this.subject('commits')).toBe('/foo/bar/merge_requests/1/commits'); }); + it('includes search parameters and hash string', function () { setLocation({ pathname: '/foo/bar/merge_requests/1/diffs', @@ -185,6 +198,7 @@ require('vendor/jquery.scrollTo'); }); expect(this.subject('show')).toBe('/foo/bar/merge_requests/1?view=parallel#L15-35'); }); + it('replaces the current history state', function () { var newState; setLocation({ @@ -197,6 +211,7 @@ require('vendor/jquery.scrollTo'); }, document.title, newState); } }); + it('treats "show" like "notes"', function () { setLocation({ pathname: '/foo/bar/merge_requests/1/commits' @@ -207,12 +222,17 @@ require('vendor/jquery.scrollTo'); describe('#tabShown', () => { beforeEach(function () { + spyOn($, 'ajax').and.callFake(function () {}); loadFixtures('merge_requests/merge_request_with_task_list.html.raw'); }); describe('with "Side-by-side"/parallel diff view', () => { beforeEach(function () { this.class.diffViewType = () => 'parallel'; + gl.Diff.prototype.diffViewType = () => 'parallel'; + spyOn($, 'ajax').and.callFake(function (options) { + options.success({ html: '' }); + }); }); it('maintains `container-limited` for pipelines tab', function (done) { @@ -224,7 +244,6 @@ require('vendor/jquery.scrollTo'); }); }); }; - asyncClick('.merge-request-tabs .pipelines-tab a') .then(() => asyncClick('.merge-request-tabs .diffs-tab a')) .then(() => asyncClick('.merge-request-tabs .pipelines-tab a')) @@ -237,6 +256,28 @@ require('vendor/jquery.scrollTo'); done.fail(`Something went wrong clicking MR tabs: ${err.message}\n${err.stack}`); }); }); + + it('maintains `container-limited` when switching from "Changes" tab before it loads', function (done) { + const asyncClick = function (selector) { + return new Promise((resolve) => { + setTimeout(() => { + document.querySelector(selector).click(); + resolve(); + }); + }); + }; + + asyncClick('.merge-request-tabs .diffs-tab a') + .then(() => asyncClick('.merge-request-tabs .notes-tab a')) + .then(() => { + const hasContainerLimitedClass = document.querySelector('.content-wrapper .container-fluid').classList.contains('container-limited'); + expect(hasContainerLimitedClass).toBe(true); + }) + .then(done) + .catch((err) => { + done.fail(`Something went wrong clicking MR tabs: ${err.message}\n${err.stack}`); + }); + }); }); }); diff --git a/spec/javascripts/monitoring/prometheus_graph_spec.js b/spec/javascripts/monitoring/prometheus_graph_spec.js index c2bcd9c0f7c..4b904fc2960 100644 --- a/spec/javascripts/monitoring/prometheus_graph_spec.js +++ b/spec/javascripts/monitoring/prometheus_graph_spec.js @@ -1,5 +1,4 @@ import 'jquery'; -import '~/lib/utils/common_utils'; import PrometheusGraph from '~/monitoring/prometheus_graph'; import { prometheusMockData } from './prometheus_mock_data'; @@ -12,6 +11,7 @@ describe('PrometheusGraph', () => { beforeEach(() => { loadFixtures(fixtureName); + $('.prometheus-container').data('has-metrics', 'true'); this.prometheusGraph = new PrometheusGraph(); const self = this; const fakeInit = (metricsResponse) => { @@ -75,3 +75,24 @@ describe('PrometheusGraph', () => { }); }); }); + +describe('PrometheusGraphs UX states', () => { + const fixtureName = 'static/environments/metrics.html.raw'; + preloadFixtures(fixtureName); + + beforeEach(() => { + loadFixtures(fixtureName); + this.prometheusGraph = new PrometheusGraph(); + }); + + it('shows a specified state', () => { + this.prometheusGraph.state = '.js-getting-started'; + this.prometheusGraph.updateState(); + const $state = $('.js-getting-started'); + expect($state).toBeDefined(); + expect($('.state-title', $state)).toBeDefined(); + expect($('.state-svg', $state)).toBeDefined(); + expect($('.state-description', $state)).toBeDefined(); + expect($('.state-button', $state)).toBeDefined(); + }); +}); diff --git a/spec/javascripts/u2f/register_spec.js b/spec/javascripts/u2f/register_spec.js index 0f390c8b980..3960759f7cb 100644 --- a/spec/javascripts/u2f/register_spec.js +++ b/spec/javascripts/u2f/register_spec.js @@ -22,7 +22,7 @@ require('./mock_u2f_device'); it('allows registering a U2F device', function() { var deviceResponse, inProgressMessage, registeredMessage, setupButton; setupButton = this.container.find("#js-setup-u2f-device"); - expect(setupButton.text()).toBe('Setup New U2F Device'); + expect(setupButton.text()).toBe('Setup new U2F device'); setupButton.trigger('click'); inProgressMessage = this.container.children("p"); expect(inProgressMessage.text()).toContain("Trying to communicate with your device"); diff --git a/spec/lib/ci/ansi2html_spec.rb b/spec/lib/ci/ansi2html_spec.rb index 0762fd7e56a..a5dfb49478a 100644 --- a/spec/lib/ci/ansi2html_spec.rb +++ b/spec/lib/ci/ansi2html_spec.rb @@ -1,159 +1,160 @@ require 'spec_helper' describe Ci::Ansi2html, lib: true do - subject { Ci::Ansi2html } + subject { described_class } it "prints non-ansi as-is" do - expect(subject.convert("Hello")[:html]).to eq('Hello') + expect(convert_html("Hello")).to eq('Hello') end it "strips non-color-changing controll sequences" do - expect(subject.convert("Hello \e[2Kworld")[:html]).to eq('Hello world') + expect(convert_html("Hello \e[2Kworld")).to eq('Hello world') end it "prints simply red" do - expect(subject.convert("\e[31mHello\e[0m")[:html]).to eq('<span class="term-fg-red">Hello</span>') + expect(convert_html("\e[31mHello\e[0m")).to eq('<span class="term-fg-red">Hello</span>') end it "prints simply red without trailing reset" do - expect(subject.convert("\e[31mHello")[:html]).to eq('<span class="term-fg-red">Hello</span>') + expect(convert_html("\e[31mHello")).to eq('<span class="term-fg-red">Hello</span>') end it "prints simply yellow" do - expect(subject.convert("\e[33mHello\e[0m")[:html]).to eq('<span class="term-fg-yellow">Hello</span>') + expect(convert_html("\e[33mHello\e[0m")).to eq('<span class="term-fg-yellow">Hello</span>') end it "prints default on blue" do - expect(subject.convert("\e[39;44mHello")[:html]).to eq('<span class="term-bg-blue">Hello</span>') + expect(convert_html("\e[39;44mHello")).to eq('<span class="term-bg-blue">Hello</span>') end it "prints red on blue" do - expect(subject.convert("\e[31;44mHello")[:html]).to eq('<span class="term-fg-red term-bg-blue">Hello</span>') + expect(convert_html("\e[31;44mHello")).to eq('<span class="term-fg-red term-bg-blue">Hello</span>') end it "resets colors after red on blue" do - expect(subject.convert("\e[31;44mHello\e[0m world")[:html]).to eq('<span class="term-fg-red term-bg-blue">Hello</span> world') + expect(convert_html("\e[31;44mHello\e[0m world")).to eq('<span class="term-fg-red term-bg-blue">Hello</span> world') end it "performs color change from red/blue to yellow/blue" do - expect(subject.convert("\e[31;44mHello \e[33mworld")[:html]).to eq('<span class="term-fg-red term-bg-blue">Hello </span><span class="term-fg-yellow term-bg-blue">world</span>') + expect(convert_html("\e[31;44mHello \e[33mworld")).to eq('<span class="term-fg-red term-bg-blue">Hello </span><span class="term-fg-yellow term-bg-blue">world</span>') end it "performs color change from red/blue to yellow/green" do - expect(subject.convert("\e[31;44mHello \e[33;42mworld")[:html]).to eq('<span class="term-fg-red term-bg-blue">Hello </span><span class="term-fg-yellow term-bg-green">world</span>') + expect(convert_html("\e[31;44mHello \e[33;42mworld")).to eq('<span class="term-fg-red term-bg-blue">Hello </span><span class="term-fg-yellow term-bg-green">world</span>') end it "performs color change from red/blue to reset to yellow/green" do - expect(subject.convert("\e[31;44mHello\e[0m \e[33;42mworld")[:html]).to eq('<span class="term-fg-red term-bg-blue">Hello</span> <span class="term-fg-yellow term-bg-green">world</span>') + expect(convert_html("\e[31;44mHello\e[0m \e[33;42mworld")).to eq('<span class="term-fg-red term-bg-blue">Hello</span> <span class="term-fg-yellow term-bg-green">world</span>') end it "ignores unsupported codes" do - expect(subject.convert("\e[51mHello\e[0m")[:html]).to eq('Hello') + expect(convert_html("\e[51mHello\e[0m")).to eq('Hello') end it "prints light red" do - expect(subject.convert("\e[91mHello\e[0m")[:html]).to eq('<span class="term-fg-l-red">Hello</span>') + expect(convert_html("\e[91mHello\e[0m")).to eq('<span class="term-fg-l-red">Hello</span>') end it "prints default on light red" do - expect(subject.convert("\e[101mHello\e[0m")[:html]).to eq('<span class="term-bg-l-red">Hello</span>') + expect(convert_html("\e[101mHello\e[0m")).to eq('<span class="term-bg-l-red">Hello</span>') end it "performs color change from red/blue to default/blue" do - expect(subject.convert("\e[31;44mHello \e[39mworld")[:html]).to eq('<span class="term-fg-red term-bg-blue">Hello </span><span class="term-bg-blue">world</span>') + expect(convert_html("\e[31;44mHello \e[39mworld")).to eq('<span class="term-fg-red term-bg-blue">Hello </span><span class="term-bg-blue">world</span>') end it "performs color change from light red/blue to default/blue" do - expect(subject.convert("\e[91;44mHello \e[39mworld")[:html]).to eq('<span class="term-fg-l-red term-bg-blue">Hello </span><span class="term-bg-blue">world</span>') + expect(convert_html("\e[91;44mHello \e[39mworld")).to eq('<span class="term-fg-l-red term-bg-blue">Hello </span><span class="term-bg-blue">world</span>') end it "prints bold text" do - expect(subject.convert("\e[1mHello")[:html]).to eq('<span class="term-bold">Hello</span>') + expect(convert_html("\e[1mHello")).to eq('<span class="term-bold">Hello</span>') end it "resets bold text" do - expect(subject.convert("\e[1mHello\e[21m world")[:html]).to eq('<span class="term-bold">Hello</span> world') - expect(subject.convert("\e[1mHello\e[22m world")[:html]).to eq('<span class="term-bold">Hello</span> world') + expect(convert_html("\e[1mHello\e[21m world")).to eq('<span class="term-bold">Hello</span> world') + expect(convert_html("\e[1mHello\e[22m world")).to eq('<span class="term-bold">Hello</span> world') end it "prints italic text" do - expect(subject.convert("\e[3mHello")[:html]).to eq('<span class="term-italic">Hello</span>') + expect(convert_html("\e[3mHello")).to eq('<span class="term-italic">Hello</span>') end it "resets italic text" do - expect(subject.convert("\e[3mHello\e[23m world")[:html]).to eq('<span class="term-italic">Hello</span> world') + expect(convert_html("\e[3mHello\e[23m world")).to eq('<span class="term-italic">Hello</span> world') end it "prints underlined text" do - expect(subject.convert("\e[4mHello")[:html]).to eq('<span class="term-underline">Hello</span>') + expect(convert_html("\e[4mHello")).to eq('<span class="term-underline">Hello</span>') end it "resets underlined text" do - expect(subject.convert("\e[4mHello\e[24m world")[:html]).to eq('<span class="term-underline">Hello</span> world') + expect(convert_html("\e[4mHello\e[24m world")).to eq('<span class="term-underline">Hello</span> world') end it "prints concealed text" do - expect(subject.convert("\e[8mHello")[:html]).to eq('<span class="term-conceal">Hello</span>') + expect(convert_html("\e[8mHello")).to eq('<span class="term-conceal">Hello</span>') end it "resets concealed text" do - expect(subject.convert("\e[8mHello\e[28m world")[:html]).to eq('<span class="term-conceal">Hello</span> world') + expect(convert_html("\e[8mHello\e[28m world")).to eq('<span class="term-conceal">Hello</span> world') end it "prints crossed-out text" do - expect(subject.convert("\e[9mHello")[:html]).to eq('<span class="term-cross">Hello</span>') + expect(convert_html("\e[9mHello")).to eq('<span class="term-cross">Hello</span>') end it "resets crossed-out text" do - expect(subject.convert("\e[9mHello\e[29m world")[:html]).to eq('<span class="term-cross">Hello</span> world') + expect(convert_html("\e[9mHello\e[29m world")).to eq('<span class="term-cross">Hello</span> world') end it "can print 256 xterm fg colors" do - expect(subject.convert("\e[38;5;16mHello")[:html]).to eq('<span class="xterm-fg-16">Hello</span>') + expect(convert_html("\e[38;5;16mHello")).to eq('<span class="xterm-fg-16">Hello</span>') end it "can print 256 xterm fg colors on normal magenta background" do - expect(subject.convert("\e[38;5;16;45mHello")[:html]).to eq('<span class="xterm-fg-16 term-bg-magenta">Hello</span>') + expect(convert_html("\e[38;5;16;45mHello")).to eq('<span class="xterm-fg-16 term-bg-magenta">Hello</span>') end it "can print 256 xterm bg colors" do - expect(subject.convert("\e[48;5;240mHello")[:html]).to eq('<span class="xterm-bg-240">Hello</span>') + expect(convert_html("\e[48;5;240mHello")).to eq('<span class="xterm-bg-240">Hello</span>') end it "can print 256 xterm bg colors on normal magenta foreground" do - expect(subject.convert("\e[48;5;16;35mHello")[:html]).to eq('<span class="term-fg-magenta xterm-bg-16">Hello</span>') + expect(convert_html("\e[48;5;16;35mHello")).to eq('<span class="term-fg-magenta xterm-bg-16">Hello</span>') end it "prints bold colored text vividly" do - expect(subject.convert("\e[1;31mHello\e[0m")[:html]).to eq('<span class="term-fg-l-red term-bold">Hello</span>') + expect(convert_html("\e[1;31mHello\e[0m")).to eq('<span class="term-fg-l-red term-bold">Hello</span>') end it "prints bold light colored text correctly" do - expect(subject.convert("\e[1;91mHello\e[0m")[:html]).to eq('<span class="term-fg-l-red term-bold">Hello</span>') + expect(convert_html("\e[1;91mHello\e[0m")).to eq('<span class="term-fg-l-red term-bold">Hello</span>') end it "prints <" do - expect(subject.convert("<")[:html]).to eq('<') + expect(convert_html("<")).to eq('<') end it "replaces newlines with line break tags" do - expect(subject.convert("\n")[:html]).to eq('<br>') + expect(convert_html("\n")).to eq('<br>') end it "groups carriage returns with newlines" do - expect(subject.convert("\r\n")[:html]).to eq('<br>') + expect(convert_html("\r\n")).to eq('<br>') end describe "incremental update" do shared_examples 'stateable converter' do - let(:pass1) { subject.convert(pre_text) } - let(:pass2) { subject.convert(pre_text + text, pass1[:state]) } + let(:pass1_stream) { StringIO.new(pre_text) } + let(:pass2_stream) { StringIO.new(pre_text + text) } + let(:pass1) { subject.convert(pass1_stream) } + let(:pass2) { subject.convert(pass2_stream, pass1.state) } it "to returns html to append" do - expect(pass2[:append]).to be_truthy - expect(pass2[:html]).to eq(html) - expect(pass1[:text] + pass2[:text]).to eq(pre_text + text) - expect(pass1[:html] + pass2[:html]).to eq(pre_html + html) + expect(pass2.append).to be_truthy + expect(pass2.html).to eq(html) + expect(pass1.html + pass2.html).to eq(pre_html + html) end end @@ -193,4 +194,27 @@ describe Ci::Ansi2html, lib: true do it_behaves_like 'stateable converter' end end + + describe "truncates" do + let(:text) { "Hello World" } + let(:stream) { StringIO.new(text) } + let(:subject) { described_class.convert(stream) } + + before do + stream.seek(3, IO::SEEK_SET) + end + + it "returns truncated output" do + expect(subject.truncated).to be_truthy + end + + it "does not append output" do + expect(subject.append).to be_falsey + end + end + + def convert_html(data) + stream = StringIO.new(data) + subject.convert(stream).html + end end diff --git a/spec/lib/container_registry/blob_spec.rb b/spec/lib/container_registry/blob_spec.rb index bbacdc67ebd..f06e5fd54a2 100644 --- a/spec/lib/container_registry/blob_spec.rb +++ b/spec/lib/container_registry/blob_spec.rb @@ -1,110 +1,121 @@ require 'spec_helper' describe ContainerRegistry::Blob do - let(:digest) { 'sha256:0123456789012345' } + let(:group) { create(:group, name: 'group') } + let(:project) { create(:empty_project, path: 'test', group: group) } + + let(:repository) do + create(:container_repository, name: 'image', + tags: %w[latest rc1], + project: project) + end + let(:config) do - { - 'digest' => digest, + { 'digest' => 'sha256:0123456789012345', 'mediaType' => 'binary', - 'size' => 1000 - } + 'size' => 1000 } + end + + let(:blob) { described_class.new(repository, config) } + + before do + stub_container_registry_config(enabled: true, + api_url: 'http://registry.gitlab', + host_port: 'registry.gitlab') end - let(:token) { 'authorization-token' } - - let(:registry) { ContainerRegistry::Registry.new('http://example.com', token: token) } - let(:repository) { registry.repository('group/test') } - let(:blob) { repository.blob(config) } it { expect(blob).to respond_to(:repository) } it { expect(blob).to delegate_method(:registry).to(:repository) } it { expect(blob).to delegate_method(:client).to(:repository) } - context '#path' do - subject { blob.path } - - it { is_expected.to eq('example.com/group/test@sha256:0123456789012345') } + describe '#path' do + it 'returns a valid path to the blob' do + expect(blob.path).to eq('group/test/image@sha256:0123456789012345') + end end - context '#digest' do - subject { blob.digest } - - it { is_expected.to eq(digest) } + describe '#digest' do + it 'return correct digest value' do + expect(blob.digest).to eq 'sha256:0123456789012345' + end end - context '#type' do - subject { blob.type } - - it { is_expected.to eq('binary') } + describe '#type' do + it 'returns a correct type' do + expect(blob.type).to eq 'binary' + end end - context '#revision' do - subject { blob.revision } - - it { is_expected.to eq('0123456789012345') } + describe '#revision' do + it 'returns a correct blob SHA' do + expect(blob.revision).to eq '0123456789012345' + end end - context '#short_revision' do - subject { blob.short_revision } - - it { is_expected.to eq('012345678') } + describe '#short_revision' do + it 'return a short SHA' do + expect(blob.short_revision).to eq '012345678' + end end - context '#delete' do + describe '#delete' do before do - stub_request(:delete, 'http://example.com/v2/group/test/blobs/sha256:0123456789012345'). - to_return(status: 200) + stub_request(:delete, 'http://registry.gitlab/v2/group/test/image/blobs/sha256:0123456789012345') + .to_return(status: 200) end - subject { blob.delete } - - it { is_expected.to be_truthy } + it 'returns true when blob has been successfuly deleted' do + expect(blob.delete).to be_truthy + end end - context '#data' do - let(:data) { '{"key":"value"}' } - - subject { blob.data } - + describe '#data' do context 'when locally stored' do before do - stub_request(:get, 'http://example.com/v2/group/test/blobs/sha256:0123456789012345'). + stub_request(:get, 'http://registry.gitlab/v2/group/test/image/blobs/sha256:0123456789012345'). to_return( status: 200, headers: { 'Content-Type' => 'application/json' }, - body: data) + body: '{"key":"value"}') end - it { is_expected.to eq(data) } + it 'returns a correct blob data' do + expect(blob.data).to eq '{"key":"value"}' + end end context 'when externally stored' do + let(:location) { 'http://external.com/blob/file' } + before do - stub_request(:get, 'http://example.com/v2/group/test/blobs/sha256:0123456789012345'). - with(headers: { 'Authorization' => "bearer #{token}" }). - to_return( + stub_request(:get, 'http://registry.gitlab/v2/group/test/image/blobs/sha256:0123456789012345') + .with(headers: { 'Authorization' => 'bearer token' }) + .to_return( status: 307, headers: { 'Location' => location }) end context 'for a valid address' do - let(:location) { 'http://external.com/blob/file' } - before do stub_request(:get, location). with(headers: { 'Authorization' => nil }). to_return( status: 200, headers: { 'Content-Type' => 'application/json' }, - body: data) + body: '{"key":"value"}') end - it { is_expected.to eq(data) } + it 'returns correct data' do + expect(blob.data).to eq '{"key":"value"}' + end end context 'for invalid file' do let(:location) { 'file:///etc/passwd' } - it { expect{ subject }.to raise_error(ArgumentError, 'invalid address') } + it 'raises an error' do + expect { blob.data }.to raise_error(ArgumentError, 'invalid address') + end end end end diff --git a/spec/lib/container_registry/path_spec.rb b/spec/lib/container_registry/path_spec.rb new file mode 100644 index 00000000000..b9c4572c269 --- /dev/null +++ b/spec/lib/container_registry/path_spec.rb @@ -0,0 +1,212 @@ +require 'spec_helper' + +describe ContainerRegistry::Path do + subject { described_class.new(path) } + + describe '#components' do + let(:path) { 'path/to/some/project' } + + it 'splits components by a forward slash' do + expect(subject.components).to eq %w[path to some project] + end + end + + describe '#nodes' do + context 'when repository path is valid' do + let(:path) { 'path/to/some/project' } + + it 'return all project path like node in reverse order' do + expect(subject.nodes).to eq %w[path/to/some/project + path/to/some + path/to] + end + end + + context 'when repository path is invalid' do + let(:path) { '' } + + it 'rasises en error' do + expect { subject.nodes } + .to raise_error described_class::InvalidRegistryPathError + end + end + end + + describe '#to_s' do + let(:path) { 'some/image' } + + it 'return a string with a repository path' do + expect(subject.to_s).to eq path + end + end + + describe '#valid?' do + context 'when path has less than two components' do + let(:path) { 'something/' } + + it { is_expected.not_to be_valid } + end + + context 'when path has more than allowed number of components' do + let(:path) { 'a/b/c/d/e/f/g/h/i/j/k/l/m/n/o/p/r/s/t/u/w/y/z' } + + it { is_expected.not_to be_valid } + end + + context 'when path has invalid characters' do + let(:path) { 'some\path' } + + it { is_expected.not_to be_valid } + end + + context 'when path has two or more components' do + let(:path) { 'some/path' } + + it { is_expected.to be_valid } + end + + context 'when path is related to multi-level image' do + let(:path) { 'some/path/my/image' } + + it { is_expected.to be_valid } + end + end + + describe '#has_repository?' do + context 'when project exists' do + let(:project) { create(:empty_project) } + let(:path) { "#{project.full_path}/my/image" } + + context 'when path already has matching repository' do + before do + create(:container_repository, project: project, name: 'my/image') + end + + it { is_expected.to have_repository } + it { is_expected.to have_project } + end + + context 'when path does not have matching repository' do + it { is_expected.not_to have_repository } + it { is_expected.to have_project } + end + end + + context 'when project does not exist' do + let(:path) { 'some/project/my/image' } + + it { is_expected.not_to have_repository } + it { is_expected.not_to have_project } + end + end + + describe '#repository_project' do + let(:group) { create(:group, path: 'some_group') } + + context 'when project for given path exists' do + let(:path) { 'some_group/some_project' } + + before do + create(:empty_project, group: group, name: 'some_project') + create(:empty_project, name: 'some_project') + end + + it 'returns a correct project' do + expect(subject.repository_project.group).to eq group + end + end + + context 'when project for given path does not exist' do + let(:path) { 'not/matching' } + + it 'returns nil' do + expect(subject.repository_project).to be_nil + end + end + + context 'when matching multi-level path' do + let(:project) do + create(:empty_project, group: group, name: 'some_project') + end + + context 'when using the zero-level path' do + let(:path) { project.full_path } + + it 'supports zero-level path' do + expect(subject.repository_project).to eq project + end + end + + context 'when using first-level path' do + let(:path) { "#{project.full_path}/repository" } + + it 'supports first-level path' do + expect(subject.repository_project).to eq project + end + end + + context 'when using second-level path' do + let(:path) { "#{project.full_path}/repository/name" } + + it 'supports second-level path' do + expect(subject.repository_project).to eq project + end + end + + context 'when using too deep nesting in the path' do + let(:path) { "#{project.full_path}/repository/name/invalid" } + + it 'does not support three-levels of nesting' do + expect(subject.repository_project).to be_nil + end + end + end + end + + describe '#repository_name' do + context 'when project does not exist' do + let(:path) { 'some/name' } + + it 'returns nil' do + expect(subject.repository_name).to be_nil + end + end + + context 'when project exists' do + let(:group) { create(:group, path: 'some_group') } + + let(:project) do + create(:empty_project, group: group, name: 'some_project') + end + + before do + allow(path).to receive(:repository_project) + .and_return(project) + end + + context 'when project path equal repository path' do + let(:path) { 'some_group/some_project' } + + it 'returns an empty string' do + expect(subject.repository_name).to eq '' + end + end + + context 'when repository path has one additional level' do + let(:path) { 'some_group/some_project/repository' } + + it 'returns a correct repository name' do + expect(subject.repository_name).to eq 'repository' + end + end + + context 'when repository path has two additional levels' do + let(:path) { 'some_group/some_project/repository/image' } + + it 'returns a correct repository name' do + expect(subject.repository_name).to eq 'repository/image' + end + end + end + end +end diff --git a/spec/lib/container_registry/registry_spec.rb b/spec/lib/container_registry/registry_spec.rb index 4f3f8b24fc4..4d6eea94bf0 100644 --- a/spec/lib/container_registry/registry_spec.rb +++ b/spec/lib/container_registry/registry_spec.rb @@ -10,7 +10,7 @@ describe ContainerRegistry::Registry do it { is_expected.to respond_to(:uri) } it { is_expected.to respond_to(:path) } - it { expect(subject.repository('test')).not_to be_nil } + it { expect(subject).not_to be_nil } context '#path' do subject { registry.path } diff --git a/spec/lib/container_registry/repository_spec.rb b/spec/lib/container_registry/repository_spec.rb deleted file mode 100644 index c364e759108..00000000000 --- a/spec/lib/container_registry/repository_spec.rb +++ /dev/null @@ -1,65 +0,0 @@ -require 'spec_helper' - -describe ContainerRegistry::Repository do - let(:registry) { ContainerRegistry::Registry.new('http://example.com') } - let(:repository) { registry.repository('group/test') } - - it { expect(repository).to respond_to(:registry) } - it { expect(repository).to delegate_method(:client).to(:registry) } - it { expect(repository.tag('test')).not_to be_nil } - - context '#path' do - subject { repository.path } - - it { is_expected.to eq('example.com/group/test') } - end - - context 'manifest processing' do - before do - stub_request(:get, 'http://example.com/v2/group/test/tags/list'). - with(headers: { 'Accept' => 'application/vnd.docker.distribution.manifest.v2+json' }). - to_return( - status: 200, - body: JSON.dump(tags: ['test']), - headers: { 'Content-Type' => 'application/json' }) - end - - context '#manifest' do - subject { repository.manifest } - - it { is_expected.not_to be_nil } - end - - context '#valid?' do - subject { repository.valid? } - - it { is_expected.to be_truthy } - end - - context '#tags' do - subject { repository.tags } - - it { is_expected.not_to be_empty } - end - end - - context '#delete_tags' do - let(:tag) { ContainerRegistry::Tag.new(repository, 'tag') } - - before { expect(repository).to receive(:tags).twice.and_return([tag]) } - - subject { repository.delete_tags } - - context 'succeeds' do - before { expect(tag).to receive(:delete).and_return(true) } - - it { is_expected.to be_truthy } - end - - context 'any fails' do - before { expect(tag).to receive(:delete).and_return(false) } - - it { is_expected.to be_falsey } - end - end -end diff --git a/spec/lib/container_registry/tag_spec.rb b/spec/lib/container_registry/tag_spec.rb index c5e31ae82b6..bc1912d8e6c 100644 --- a/spec/lib/container_registry/tag_spec.rb +++ b/spec/lib/container_registry/tag_spec.rb @@ -1,25 +1,59 @@ require 'spec_helper' describe ContainerRegistry::Tag do - let(:registry) { ContainerRegistry::Registry.new('http://example.com') } - let(:repository) { registry.repository('group/test') } - let(:tag) { repository.tag('tag') } - let(:headers) { { 'Accept' => 'application/vnd.docker.distribution.manifest.v2+json' } } + let(:group) { create(:group, name: 'group') } + let(:project) { create(:project, path: 'test', group: group) } + + let(:repository) do + create(:container_repository, name: '', project: project) + end + + let(:headers) do + { 'Accept' => 'application/vnd.docker.distribution.manifest.v2+json' } + end + + let(:tag) { described_class.new(repository, 'tag') } + + before do + stub_container_registry_config(enabled: true, + api_url: 'http://registry.gitlab', + host_port: 'registry.gitlab') + end it { expect(tag).to respond_to(:repository) } it { expect(tag).to delegate_method(:registry).to(:repository) } it { expect(tag).to delegate_method(:client).to(:repository) } - context '#path' do - subject { tag.path } + describe '#path' do + context 'when tag belongs to zero-level repository' do + let(:repository) do + create(:container_repository, name: '', + tags: %w[rc1], + project: project) + end + + it 'returns path to the image' do + expect(tag.path).to eq('group/test:tag') + end + end + + context 'when tag belongs to first-level repository' do + let(:repository) do + create(:container_repository, name: 'my_image', + tags: %w[tag], + project: project) + end - it { is_expected.to eq('example.com/group/test:tag') } + it 'returns path to the image' do + expect(tag.path).to eq('group/test/my_image:tag') + end + end end context 'manifest processing' do context 'schema v1' do before do - stub_request(:get, 'http://example.com/v2/group/test/manifests/tag'). + stub_request(:get, 'http://registry.gitlab/v2/group/test/manifests/tag'). with(headers: headers). to_return( status: 200, @@ -56,7 +90,7 @@ describe ContainerRegistry::Tag do context 'schema v2' do before do - stub_request(:get, 'http://example.com/v2/group/test/manifests/tag'). + stub_request(:get, 'http://registry.gitlab/v2/group/test/manifests/tag'). with(headers: headers). to_return( status: 200, @@ -93,7 +127,7 @@ describe ContainerRegistry::Tag do context 'when locally stored' do before do - stub_request(:get, 'http://example.com/v2/group/test/blobs/sha256:d7a513a663c1a6dcdba9ed832ca53c02ac2af0c333322cd6ca92936d1d9917ac'). + stub_request(:get, 'http://registry.gitlab/v2/group/test/blobs/sha256:d7a513a663c1a6dcdba9ed832ca53c02ac2af0c333322cd6ca92936d1d9917ac'). with(headers: { 'Accept' => 'application/octet-stream' }). to_return( status: 200, @@ -105,7 +139,7 @@ describe ContainerRegistry::Tag do context 'when externally stored' do before do - stub_request(:get, 'http://example.com/v2/group/test/blobs/sha256:d7a513a663c1a6dcdba9ed832ca53c02ac2af0c333322cd6ca92936d1d9917ac'). + stub_request(:get, 'http://registry.gitlab/v2/group/test/blobs/sha256:d7a513a663c1a6dcdba9ed832ca53c02ac2af0c333322cd6ca92936d1d9917ac'). with(headers: { 'Accept' => 'application/octet-stream' }). to_return( status: 307, @@ -123,29 +157,29 @@ describe ContainerRegistry::Tag do end end - context 'manifest digest' do + context 'with stubbed digest' do before do - stub_request(:head, 'http://example.com/v2/group/test/manifests/tag'). - with(headers: headers). - to_return(status: 200, headers: { 'Docker-Content-Digest' => 'sha256:digest' }) + stub_request(:head, 'http://registry.gitlab/v2/group/test/manifests/tag') + .with(headers: headers) + .to_return(status: 200, headers: { 'Docker-Content-Digest' => 'sha256:digest' }) end - context '#digest' do - subject { tag.digest } - - it { is_expected.to eq('sha256:digest') } + describe '#digest' do + it 'returns a correct tag digest' do + expect(tag.digest).to eq 'sha256:digest' + end end - context '#delete' do + describe '#delete' do before do - stub_request(:delete, 'http://example.com/v2/group/test/manifests/sha256:digest'). - with(headers: headers). - to_return(status: 200) + stub_request(:delete, 'http://registry.gitlab/v2/group/test/manifests/sha256:digest') + .with(headers: headers) + .to_return(status: 200) end - subject { tag.delete } - - it { is_expected.to be_truthy } + it 'correctly deletes the tag' do + expect(tag.delete).to be_truthy + end end end end diff --git a/spec/lib/gitlab/ci/cron_parser_spec.rb b/spec/lib/gitlab/ci/cron_parser_spec.rb new file mode 100644 index 00000000000..0864bc7258d --- /dev/null +++ b/spec/lib/gitlab/ci/cron_parser_spec.rb @@ -0,0 +1,116 @@ +require 'spec_helper' + +describe Gitlab::Ci::CronParser do + shared_examples_for "returns time in the future" do + it { is_expected.to be > Time.now } + end + + describe '#next_time_from' do + subject { described_class.new(cron, cron_timezone).next_time_from(Time.now) } + + context 'when cron and cron_timezone are valid' do + context 'when specific time' do + let(:cron) { '3 4 5 6 *' } + let(:cron_timezone) { 'UTC' } + + it_behaves_like "returns time in the future" + + it 'returns exact time' do + expect(subject.min).to eq(3) + expect(subject.hour).to eq(4) + expect(subject.day).to eq(5) + expect(subject.month).to eq(6) + end + end + + context 'when specific day of week' do + let(:cron) { '* * * * 0' } + let(:cron_timezone) { 'UTC' } + + it_behaves_like "returns time in the future" + + it 'returns exact day of week' do + expect(subject.wday).to eq(0) + end + end + + context 'when slash used' do + let(:cron) { '*/10 */6 */10 */10 *' } + let(:cron_timezone) { 'UTC' } + + it_behaves_like "returns time in the future" + + it 'returns specific time' do + expect(subject.min).to be_in([0, 10, 20, 30, 40, 50]) + expect(subject.hour).to be_in([0, 6, 12, 18]) + expect(subject.day).to be_in([1, 11, 21, 31]) + expect(subject.month).to be_in([1, 11]) + end + end + + context 'when range used' do + let(:cron) { '0,20,40 * 1-5 * *' } + let(:cron_timezone) { 'UTC' } + + it_behaves_like "returns time in the future" + + it 'returns specific time' do + expect(subject.min).to be_in([0, 20, 40]) + expect(subject.day).to be_in((1..5).to_a) + end + end + + context 'when cron_timezone is US/Pacific' do + let(:cron) { '0 0 * * *' } + let(:cron_timezone) { 'US/Pacific' } + + it_behaves_like "returns time in the future" + + it 'converts time in server time zone' do + expect(subject.hour).to eq((Time.zone.now.in_time_zone(cron_timezone).utc_offset / 60 / 60).abs) + end + end + end + + context 'when cron and cron_timezone are invalid' do + let(:cron) { 'invalid_cron' } + let(:cron_timezone) { 'invalid_cron_timezone' } + + it 'returns nil' do + is_expected.to be_nil + end + end + end + + describe '#cron_valid?' do + subject { described_class.new(cron, Gitlab::Ci::CronParser::VALID_SYNTAX_SAMPLE_TIME_ZONE).cron_valid? } + + context 'when cron is valid' do + let(:cron) { '* * * * *' } + + it { is_expected.to eq(true) } + end + + context 'when cron is invalid' do + let(:cron) { '*********' } + + it { is_expected.to eq(false) } + end + end + + describe '#cron_timezone_valid?' do + subject { described_class.new(Gitlab::Ci::CronParser::VALID_SYNTAX_SAMPLE_CRON, cron_timezone).cron_timezone_valid? } + + context 'when cron is valid' do + let(:cron_timezone) { 'Europe/Istanbul' } + + it { is_expected.to eq(true) } + end + + context 'when cron is invalid' do + let(:cron_timezone) { 'Invalid-zone' } + + it { is_expected.to eq(false) } + end + end +end diff --git a/spec/lib/gitlab/ci/trace/stream_spec.rb b/spec/lib/gitlab/ci/trace/stream_spec.rb new file mode 100644 index 00000000000..f1a1a71c528 --- /dev/null +++ b/spec/lib/gitlab/ci/trace/stream_spec.rb @@ -0,0 +1,201 @@ +require 'spec_helper' + +describe Gitlab::Ci::Trace::Stream do + describe 'delegates' do + subject { described_class.new { nil } } + + it { is_expected.to delegate_method(:close).to(:stream) } + it { is_expected.to delegate_method(:tell).to(:stream) } + it { is_expected.to delegate_method(:seek).to(:stream) } + it { is_expected.to delegate_method(:size).to(:stream) } + it { is_expected.to delegate_method(:path).to(:stream) } + it { is_expected.to delegate_method(:truncate).to(:stream) } + it { is_expected.to delegate_method(:valid?).to(:stream).as(:present?) } + it { is_expected.to delegate_method(:file?).to(:path).as(:present?) } + end + + describe '#limit' do + let(:stream) do + described_class.new do + StringIO.new("12345678") + end + end + + it 'if size is larger we start from beggining' do + stream.limit(10) + + expect(stream.tell).to eq(0) + end + + it 'if size is smaller we start from the end' do + stream.limit(2) + + expect(stream.tell).to eq(6) + end + end + + describe '#append' do + let(:stream) do + described_class.new do + StringIO.new("12345678") + end + end + + it "truncates and append content" do + stream.append("89", 4) + stream.seek(0) + + expect(stream.size).to eq(6) + expect(stream.raw).to eq("123489") + end + end + + describe '#set' do + let(:stream) do + described_class.new do + StringIO.new("12345678") + end + end + + before do + stream.set("8901") + end + + it "overwrite content" do + stream.seek(0) + + expect(stream.size).to eq(4) + expect(stream.raw).to eq("8901") + end + end + + describe '#raw' do + let(:path) { __FILE__ } + let(:lines) { File.readlines(path) } + let(:stream) do + described_class.new do + File.open(path) + end + end + + it 'returns all contents if last_lines is not specified' do + result = stream.raw + + expect(result).to eq(lines.join) + expect(result.encoding).to eq(Encoding.default_external) + end + + context 'limit max lines' do + before do + # specifying BUFFER_SIZE forces to seek backwards + allow(described_class).to receive(:BUFFER_SIZE) + .and_return(2) + end + + it 'returns last few lines' do + result = stream.raw(last_lines: 2) + + expect(result).to eq(lines.last(2).join) + expect(result.encoding).to eq(Encoding.default_external) + end + + it 'returns everything if trying to get too many lines' do + result = stream.raw(last_lines: lines.size * 2) + + expect(result).to eq(lines.join) + expect(result.encoding).to eq(Encoding.default_external) + end + end + end + + describe '#html_with_state' do + let(:stream) do + described_class.new do + StringIO.new("1234") + end + end + + it 'returns html content with state' do + result = stream.html_with_state + + expect(result.html).to eq("1234") + end + + context 'follow-up state' do + let!(:last_result) { stream.html_with_state } + + before do + stream.append("5678", 4) + stream.seek(0) + end + + it "returns appended trace" do + result = stream.html_with_state(last_result.state) + + expect(result.append).to be_truthy + expect(result.html).to eq("5678") + end + end + end + + describe '#html' do + let(:stream) do + described_class.new do + StringIO.new("12\n34\n56") + end + end + + it "returns html" do + expect(stream.html).to eq("12<br>34<br>56") + end + + it "returns html for last line only" do + expect(stream.html(last_lines: 1)).to eq("56") + end + end + + describe '#extract_coverage' do + let(:stream) do + described_class.new do + StringIO.new(data) + end + end + + subject { stream.extract_coverage(regex) } + + context 'valid content & regex' do + let(:data) { 'Coverage 1033 / 1051 LOC (98.29%) covered' } + let(:regex) { '\(\d+.\d+\%\) covered' } + + it { is_expected.to eq(98.29) } + end + + context 'valid content & bad regex' do + let(:data) { 'Coverage 1033 / 1051 LOC (98.29%) covered\n' } + let(:regex) { 'very covered' } + + it { is_expected.to be_nil } + end + + context 'no coverage content & regex' do + let(:data) { 'No coverage for today :sad:' } + let(:regex) { '\(\d+.\d+\%\) covered' } + + it { is_expected.to be_nil } + end + + context 'multiple results in content & regex' do + let(:data) { ' (98.39%) covered. (98.29%) covered' } + let(:regex) { '\(\d+.\d+\%\) covered' } + + it { is_expected.to eq(98.29) } + end + + context 'using a regex capture' do + let(:data) { 'TOTAL 9926 3489 65%' } + let(:regex) { 'TOTAL\s+\d+\s+\d+\s+(\d{1,3}\%)' } + + it { is_expected.to eq(65) } + end + end +end diff --git a/spec/lib/gitlab/ci/trace_reader_spec.rb b/spec/lib/gitlab/ci/trace_reader_spec.rb deleted file mode 100644 index ff5551bf703..00000000000 --- a/spec/lib/gitlab/ci/trace_reader_spec.rb +++ /dev/null @@ -1,52 +0,0 @@ -require 'spec_helper' - -describe Gitlab::Ci::TraceReader do - let(:path) { __FILE__ } - let(:lines) { File.readlines(path) } - let(:bytesize) { lines.sum(&:bytesize) } - - it 'returns last few lines' do - 10.times do - subject = build_subject - last_lines = random_lines - - expected = lines.last(last_lines).join - result = subject.read(last_lines: last_lines) - - expect(result).to eq(expected) - expect(result.encoding).to eq(Encoding.default_external) - end - end - - it 'returns everything if trying to get too many lines' do - result = build_subject.read(last_lines: lines.size * 2) - - expect(result).to eq(lines.join) - expect(result.encoding).to eq(Encoding.default_external) - end - - it 'returns all contents if last_lines is not specified' do - result = build_subject.read - - expect(result).to eq(lines.join) - expect(result.encoding).to eq(Encoding.default_external) - end - - it 'raises an error if not passing an integer for last_lines' do - expect do - build_subject.read(last_lines: lines) - end.to raise_error(ArgumentError) - end - - def random_lines - Random.rand(lines.size) + 1 - end - - def random_buffer - Random.rand(bytesize) + 1 - end - - def build_subject - described_class.new(__FILE__, buffer_size: random_buffer) - end -end diff --git a/spec/lib/gitlab/ci/trace_spec.rb b/spec/lib/gitlab/ci/trace_spec.rb new file mode 100644 index 00000000000..69e8dc9220d --- /dev/null +++ b/spec/lib/gitlab/ci/trace_spec.rb @@ -0,0 +1,216 @@ +require 'spec_helper' + +describe Gitlab::Ci::Trace do + let(:build) { create(:ci_build) } + let(:trace) { described_class.new(build) } + + describe "associations" do + it { expect(trace).to respond_to(:job) } + it { expect(trace).to delegate_method(:old_trace).to(:job) } + end + + describe '#html' do + before do + trace.set("12\n34") + end + + it "returns formatted html" do + expect(trace.html).to eq("12<br>34") + end + + it "returns last line of formatted html" do + expect(trace.html(last_lines: 1)).to eq("34") + end + end + + describe '#raw' do + before do + trace.set("12\n34") + end + + it "returns raw output" do + expect(trace.raw).to eq("12\n34") + end + + it "returns last line of raw output" do + expect(trace.raw(last_lines: 1)).to eq("34") + end + end + + describe '#extract_coverage' do + let(:regex) { '\(\d+.\d+\%\) covered' } + + before do + trace.set('Coverage 1033 / 1051 LOC (98.29%) covered') + end + + it "returns valid coverage" do + expect(trace.extract_coverage(regex)).to eq(98.29) + end + end + + describe '#set' do + before do + trace.set("12") + end + + it "returns trace" do + expect(trace.raw).to eq("12") + end + + context 'overwrite trace' do + before do + trace.set("34") + end + + it "returns new trace" do + expect(trace.raw).to eq("34") + end + end + + context 'runners token' do + let(:token) { 'my_secret_token' } + + before do + build.project.update(runners_token: token) + trace.set(token) + end + + it "hides token" do + expect(trace.raw).not_to include(token) + end + end + + context 'hides build token' do + let(:token) { 'my_secret_token' } + + before do + build.update(token: token) + trace.set(token) + end + + it "hides token" do + expect(trace.raw).not_to include(token) + end + end + end + + describe '#append' do + before do + trace.set("1234") + end + + it "returns correct trace" do + expect(trace.append("56", 4)).to eq(6) + expect(trace.raw).to eq("123456") + end + + context 'tries to append trace at different offset' do + it "fails with append" do + expect(trace.append("56", 2)).to eq(-4) + expect(trace.raw).to eq("1234") + end + end + + context 'runners token' do + let(:token) { 'my_secret_token' } + + before do + build.project.update(runners_token: token) + trace.append(token, 0) + end + + it "hides token" do + expect(trace.raw).not_to include(token) + end + end + + context 'build token' do + let(:token) { 'my_secret_token' } + + before do + build.update(token: token) + trace.append(token, 0) + end + + it "hides token" do + expect(trace.raw).not_to include(token) + end + end + end + + describe 'trace handling' do + context 'trace does not exist' do + it { expect(trace.exist?).to be(false) } + end + + context 'new trace path is used' do + before do + trace.send(:ensure_directory) + + File.open(trace.send(:default_path), "w") do |file| + file.write("data") + end + end + + it "trace exist" do + expect(trace.exist?).to be(true) + end + + it "can be erased" do + trace.erase! + expect(trace.exist?).to be(false) + end + end + + context 'deprecated path' do + let(:path) { trace.send(:deprecated_path) } + + context 'with valid ci_id' do + before do + build.project.update(ci_id: 1000) + + FileUtils.mkdir_p(File.dirname(path)) + + File.open(path, "w") do |file| + file.write("data") + end + end + + it "trace exist" do + expect(trace.exist?).to be(true) + end + + it "can be erased" do + trace.erase! + expect(trace.exist?).to be(false) + end + end + + context 'without valid ci_id' do + it "does not return deprecated path" do + expect(path).to be_nil + end + end + end + + context 'stored in database' do + before do + build.send(:write_attribute, :trace, "data") + end + + it "trace exist" do + expect(trace.exist?).to be(true) + end + + it "can be erased" do + trace.erase! + expect(trace.exist?).to be(false) + end + + it "returns database data" do + expect(trace.raw).to eq("data") + end + end + end +end diff --git a/spec/lib/gitlab/etag_caching/middleware_spec.rb b/spec/lib/gitlab/etag_caching/middleware_spec.rb index 6ec4360adc2..24df04e985a 100644 --- a/spec/lib/gitlab/etag_caching/middleware_spec.rb +++ b/spec/lib/gitlab/etag_caching/middleware_spec.rb @@ -47,9 +47,9 @@ describe Gitlab::EtagCaching::Middleware do it 'tracks "etag_caching_key_not_found" event' do expect(Gitlab::Metrics).to receive(:add_event) - .with(:etag_caching_middleware_used) + .with(:etag_caching_middleware_used, endpoint: 'issue_notes') expect(Gitlab::Metrics).to receive(:add_event) - .with(:etag_caching_key_not_found) + .with(:etag_caching_key_not_found, endpoint: 'issue_notes') middleware.call(build_env(path, if_none_match)) end @@ -91,11 +91,17 @@ describe Gitlab::EtagCaching::Middleware do expect(status).to eq 304 end + it 'returns empty body' do + _, _, body = middleware.call(build_env(path, if_none_match)) + + expect(body).to be_empty + end + it 'tracks "etag_caching_cache_hit" event' do expect(Gitlab::Metrics).to receive(:add_event) - .with(:etag_caching_middleware_used) + .with(:etag_caching_middleware_used, endpoint: 'issue_notes') expect(Gitlab::Metrics).to receive(:add_event) - .with(:etag_caching_cache_hit) + .with(:etag_caching_cache_hit, endpoint: 'issue_notes') middleware.call(build_env(path, if_none_match)) end @@ -132,9 +138,9 @@ describe Gitlab::EtagCaching::Middleware do mock_app_response expect(Gitlab::Metrics).to receive(:add_event) - .with(:etag_caching_middleware_used) + .with(:etag_caching_middleware_used, endpoint: 'issue_notes') expect(Gitlab::Metrics).to receive(:add_event) - .with(:etag_caching_resource_changed) + .with(:etag_caching_resource_changed, endpoint: 'issue_notes') middleware.call(build_env(path, if_none_match)) end @@ -150,9 +156,9 @@ describe Gitlab::EtagCaching::Middleware do it 'tracks "etag_caching_header_missing" event' do expect(Gitlab::Metrics).to receive(:add_event) - .with(:etag_caching_middleware_used) + .with(:etag_caching_middleware_used, endpoint: 'issue_notes') expect(Gitlab::Metrics).to receive(:add_event) - .with(:etag_caching_header_missing) + .with(:etag_caching_header_missing, endpoint: 'issue_notes') middleware.call(build_env(path, if_none_match)) end diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 24654bf6afd..9770d2e3b13 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -89,16 +89,28 @@ pipelines: - statuses - builds - trigger_requests +- auto_canceled_by +- auto_canceled_pipelines +- auto_canceled_jobs +- pending_builds +- retryable_builds +- cancelable_statuses +- manual_actions +- artifacts statuses: - project - pipeline - user +- auto_canceled_by variables: - project triggers: - project - trigger_requests - owner +- trigger_schedule +trigger_schedule: +- trigger deploy_keys: - user - deploy_keys_projects @@ -116,6 +128,9 @@ merge_access_levels: - protected_branch push_access_levels: - protected_branch +container_repositories: +- project +- name project: - taggings - base_tags @@ -143,6 +158,7 @@ project: - asana_service - gemnasium_service - slack_service +- microsoft_teams_service - mattermost_service - buildkite_service - bamboo_service @@ -192,8 +208,10 @@ project: - builds - runner_projects - runners +- active_runners - variables - triggers +- trigger_schedules - environments - deployments - project_feature @@ -202,6 +220,7 @@ project: - project_authorizations - route - statistics +- container_repositories - uploads award_emoji: - awardable diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index 1ad16a9b57d..04b5c80f22d 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -183,6 +183,7 @@ Ci::Pipeline: - duration - user_id - lock_version +- auto_canceled_by_id CommitStatus: - id - project_id @@ -223,6 +224,7 @@ CommitStatus: - token - lock_version - coverage_regex +- auto_canceled_by_id Ci::Variable: - id - project_id @@ -240,6 +242,17 @@ Ci::Trigger: - updated_at - owner_id - description +- ref +Ci::TriggerSchedule: +- id +- project_id +- trigger_id +- deleted_at +- created_at +- updated_at +- cron +- cron_timezone +- next_run_at DeployKey: - id - user_id diff --git a/spec/lib/microsoft_teams/activity_spec.rb b/spec/lib/microsoft_teams/activity_spec.rb new file mode 100644 index 00000000000..7890ae2e7b0 --- /dev/null +++ b/spec/lib/microsoft_teams/activity_spec.rb @@ -0,0 +1,16 @@ +require 'spec_helper' + +describe MicrosoftTeams::Activity do + subject { described_class.new(title: 'title', subtitle: 'subtitle', text: 'text', image: 'image') } + + describe '#prepare' do + it 'returns the correct JSON object' do + expect(subject.prepare).to eq({ + 'activityTitle' => 'title', + 'activitySubtitle' => 'subtitle', + 'activityText' => 'text', + 'activityImage' => 'image' + }) + end + end +end diff --git a/spec/lib/microsoft_teams/notifier_spec.rb b/spec/lib/microsoft_teams/notifier_spec.rb new file mode 100644 index 00000000000..3035693812f --- /dev/null +++ b/spec/lib/microsoft_teams/notifier_spec.rb @@ -0,0 +1,55 @@ +require 'spec_helper' + +describe MicrosoftTeams::Notifier do + subject { described_class.new(webhook_url) } + + let(:webhook_url) { 'https://example.gitlab.com/'} + let(:header) { { 'Content-Type' => 'application/json' } } + let(:options) do + { + title: 'JohnDoe4/project2', + pretext: '[[JohnDoe4/project2](http://localhost/namespace2/gitlabhq)] Issue [#1 Awesome issue](http://localhost/namespace2/gitlabhq/issues/1) opened by user6', + activity: { + title: 'Issue opened by user6', + subtitle: 'in [JohnDoe4/project2](http://localhost/namespace2/gitlabhq)', + text: '[#1 Awesome issue](http://localhost/namespace2/gitlabhq/issues/1)', + image: 'http://someimage.com' + }, + attachments: 'please fix' + } + end + + let(:body) do + { + 'sections' => [ + { + 'activityTitle' => 'Issue opened by user6', + 'activitySubtitle' => 'in [JohnDoe4/project2](http://localhost/namespace2/gitlabhq)', + 'activityText' => '[#1 Awesome issue](http://localhost/namespace2/gitlabhq/issues/1)', + 'activityImage' => 'http://someimage.com' + }, + { + 'title' => 'Details', + 'facts' => [ + { + 'name' => 'Attachments', + 'value' => 'please fix' + } + ] + } + ], + 'title' => 'JohnDoe4/project2', + 'summary' => '[[JohnDoe4/project2](http://localhost/namespace2/gitlabhq)] Issue [#1 Awesome issue](http://localhost/namespace2/gitlabhq/issues/1) opened by user6' + } + end + + describe '#ping' do + before do + stub_request(:post, webhook_url).with(body: JSON(body), headers: { 'Content-Type' => 'application/json' }).to_return(status: 200, body: "", headers: {}) + end + + it 'expects to receive successfull answer' do + expect(subject.ping(options)).to be true + end + end +end diff --git a/spec/models/award_emoji_spec.rb b/spec/models/award_emoji_spec.rb index cb3c592f8cd..2a9a27752c1 100644 --- a/spec/models/award_emoji_spec.rb +++ b/spec/models/award_emoji_spec.rb @@ -25,6 +25,20 @@ describe AwardEmoji, models: true do expect(new_award).not_to be_valid end + + # Assume User A and User B both created award emoji of the same name + # on the same awardable. When User A is deleted, User A's award emoji + # is moved to the ghost user. When User B is deleted, User B's award emoji + # also needs to be moved to the ghost user - this cannot happen unless + # the uniqueness validation is disabled for ghost users. + it "allows duplicate award emoji for ghost users" do + user = create(:user, :ghost) + issue = create(:issue) + create(:award_emoji, user: user, awardable: issue) + new_award = build(:award_emoji, user: user, awardable: issue) + + expect(new_award).to be_valid + end end end end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 8dbcf50ee0c..6e8845cdcf4 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -17,8 +17,9 @@ describe Ci::Build, :models do it { is_expected.to belong_to(:trigger_request) } it { is_expected.to belong_to(:erased_by) } it { is_expected.to have_many(:deployments) } - it { is_expected.to validate_presence_of :ref } - it { is_expected.to respond_to :trace_html } + it { is_expected.to validate_presence_of(:ref) } + it { is_expected.to respond_to(:has_trace?) } + it { is_expected.to respond_to(:trace) } describe '#actionize' do context 'when build is a created' do @@ -78,32 +79,6 @@ describe Ci::Build, :models do end end - describe '#append_trace' do - subject { build.trace_html } - - context 'when build.trace hides runners token' do - let(:token) { 'my_secret_token' } - - before do - build.project.update(runners_token: token) - build.append_trace(token, 0) - end - - it { is_expected.not_to include(token) } - end - - context 'when build.trace hides build token' do - let(:token) { 'my_secret_token' } - - before do - build.update(token: token) - build.append_trace(token, 0) - end - - it { is_expected.not_to include(token) } - end - end - describe '#artifacts?' do subject { build.artifacts? } @@ -272,12 +247,98 @@ describe Ci::Build, :models do describe '#update_coverage' do context "regarding coverage_regex's value," do - it "saves the correct extracted coverage value" do + before do build.coverage_regex = '\(\d+.\d+\%\) covered' - allow(build).to receive(:trace) { 'Coverage 1033 / 1051 LOC (98.29%) covered' } - expect(build).to receive(:update_attributes).with(coverage: 98.29) { true } - expect(build.update_coverage).to be true + build.trace.set('Coverage 1033 / 1051 LOC (98.29%) covered') end + + it "saves the correct extracted coverage value" do + expect(build.update_coverage).to be(true) + expect(build.coverage).to eq(98.29) + end + end + end + + describe '#trace' do + subject { build.trace } + + it { is_expected.to be_a(Gitlab::Ci::Trace) } + end + + describe '#has_trace?' do + subject { build.has_trace? } + + it "expect to call exist? method" do + expect_any_instance_of(Gitlab::Ci::Trace).to receive(:exist?) + .and_return(true) + + is_expected.to be(true) + end + end + + describe '#trace=' do + it "expect to fail trace=" do + expect { build.trace = "new" }.to raise_error(NotImplementedError) + end + end + + describe '#old_trace' do + subject { build.old_trace } + + before do + build.update_column(:trace, 'old trace') + end + + it "expect to receive data from database" do + is_expected.to eq('old trace') + end + end + + describe '#erase_old_trace!' do + subject { build.send(:read_attribute, :trace) } + + before do + build.send(:write_attribute, :trace, 'old trace') + end + + it "expect to receive data from database" do + build.erase_old_trace! + + is_expected.to be_nil + end + end + + describe '#hide_secrets' do + let(:subject) { build.hide_secrets(data) } + + context 'hide runners token' do + let(:data) { 'new token data'} + + before do + build.project.update(runners_token: 'token') + end + + it { is_expected.to eq('new xxxxx data') } + end + + context 'hide build token' do + let(:data) { 'new token data'} + + before do + build.update(token: 'token') + end + + it { is_expected.to eq('new xxxxx data') } + end + + context 'hide build token' do + let(:data) { 'new token data'} + + before do + build.update(token: 'token') + end + + it { is_expected.to eq('new xxxxx data') } end end @@ -438,7 +499,7 @@ describe Ci::Build, :models do end it 'erases build trace in trace file' do - expect(build.trace).to be_empty + expect(build).not_to have_trace end it 'sets erased to true' do @@ -532,38 +593,6 @@ describe Ci::Build, :models do end end - describe '#extract_coverage' do - context 'valid content & regex' do - subject { build.extract_coverage('Coverage 1033 / 1051 LOC (98.29%) covered', '\(\d+.\d+\%\) covered') } - - it { is_expected.to eq(98.29) } - end - - context 'valid content & bad regex' do - subject { build.extract_coverage('Coverage 1033 / 1051 LOC (98.29%) covered', 'very covered') } - - it { is_expected.to be_nil } - end - - context 'no coverage content & regex' do - subject { build.extract_coverage('No coverage for today :sad:', '\(\d+.\d+\%\) covered') } - - it { is_expected.to be_nil } - end - - context 'multiple results in content & regex' do - subject { build.extract_coverage(' (98.39%) covered. (98.29%) covered', '\(\d+.\d+\%\) covered') } - - it { is_expected.to eq(98.29) } - end - - context 'using a regex capture' do - subject { build.extract_coverage('TOTAL 9926 3489 65%', 'TOTAL\s+\d+\s+\d+\s+(\d{1,3}\%)') } - - it { is_expected.to eq(65) } - end - end - describe '#first_pending' do let!(:first) { create(:ci_build, pipeline: pipeline, status: 'pending', created_at: Date.yesterday) } let!(:second) { create(:ci_build, pipeline: pipeline, status: 'pending') } @@ -735,40 +764,6 @@ describe Ci::Build, :models do end end - describe '#has_commands?' do - context 'when build has commands' do - let(:build) do - create(:ci_build, commands: 'rspec') - end - - it 'has commands' do - expect(build).to have_commands - end - end - - context 'when does not have commands' do - context 'when commands are an empty string' do - let(:build) do - create(:ci_build, commands: '') - end - - it 'has no commands' do - expect(build).not_to have_commands - end - end - - context 'when commands are not set at all' do - let(:build) do - create(:ci_build, commands: nil) - end - - it 'has no commands' do - expect(build).not_to have_commands - end - end - end - end - describe '#has_tags?' do context 'when build has tags' do subject { create(:ci_build, tag_list: ['tag']) } @@ -983,32 +978,6 @@ describe Ci::Build, :models do it { is_expected.to eq(project.name) } end - describe '#raw_trace' do - subject { build.raw_trace } - - context 'when build.trace hides runners token' do - let(:token) { 'my_secret_token' } - - before do - build.project.update(runners_token: token) - build.update(trace: token) - end - - it { is_expected.not_to include(token) } - end - - context 'when build.trace hides build token' do - let(:token) { 'my_secret_token' } - - before do - build.update(token: token) - build.update(trace: token) - end - - it { is_expected.not_to include(token) } - end - end - describe '#ref_slug' do { 'master' => 'master', @@ -1074,61 +1043,6 @@ describe Ci::Build, :models do end end - describe '#trace' do - it 'obfuscates project runners token' do - allow(build).to receive(:raw_trace).and_return("Test: #{build.project.runners_token}") - - expect(build.trace).to eq("Test: xxxxxxxxxxxxxxxxxxxx") - end - - it 'empty project runners token' do - allow(build).to receive(:raw_trace).and_return(test_trace) - # runners_token can't normally be set to nil - allow(build.project).to receive(:runners_token).and_return(nil) - - expect(build.trace).to eq(test_trace) - end - - context 'when build does not have trace' do - it 'is is empty' do - expect(build.trace).to be_nil - end - end - - context 'when trace contains text' do - let(:text) { 'example output' } - before do - build.trace = text - end - - it { expect(build.trace).to eq(text) } - end - - context 'when trace hides runners token' do - let(:token) { 'my_secret_token' } - - before do - build.update(trace: token) - build.project.update(runners_token: token) - end - - it { expect(build.trace).not_to include(token) } - it { expect(build.raw_trace).to include(token) } - end - - context 'when build.trace hides build token' do - let(:token) { 'my_secret_token' } - - before do - build.update(trace: token) - build.update(token: token) - end - - it { expect(build.trace).not_to include(token) } - it { expect(build.raw_trace).to include(token) } - end - end - describe '#has_expiring_artifacts?' do context 'when artifacts have expiration date set' do before { build.update(artifacts_expire_at: 1.day.from_now) } @@ -1147,66 +1061,6 @@ describe Ci::Build, :models do end end - describe '#has_trace_file?' do - context 'when there is no trace' do - it { expect(build.has_trace_file?).to be_falsey } - it { expect(build.trace).to be_nil } - end - - context 'when there is a trace' do - context 'when trace is stored in file' do - let(:build_with_trace) { create(:ci_build, :trace) } - - it { expect(build_with_trace.has_trace_file?).to be_truthy } - it { expect(build_with_trace.trace).to eq('BUILD TRACE') } - end - - context 'when trace is stored in old file' do - before do - allow(build.project).to receive(:ci_id).and_return(999) - allow(File).to receive(:exist?).with(build.path_to_trace).and_return(false) - allow(File).to receive(:exist?).with(build.old_path_to_trace).and_return(true) - allow(File).to receive(:read).with(build.old_path_to_trace).and_return(test_trace) - end - - it { expect(build.has_trace_file?).to be_truthy } - it { expect(build.trace).to eq(test_trace) } - end - - context 'when trace is stored in DB' do - before do - allow(build.project).to receive(:ci_id).and_return(nil) - allow(build).to receive(:read_attribute).with(:trace).and_return(test_trace) - allow(File).to receive(:exist?).with(build.path_to_trace).and_return(false) - allow(File).to receive(:exist?).with(build.old_path_to_trace).and_return(false) - end - - it { expect(build.has_trace_file?).to be_falsey } - it { expect(build.trace).to eq(test_trace) } - end - end - end - - describe '#trace_file_path' do - context 'when trace is stored in file' do - before do - allow(build).to receive(:has_trace_file?).and_return(true) - allow(build).to receive(:has_old_trace_file?).and_return(false) - end - - it { expect(build.trace_file_path).to eq(build.path_to_trace) } - end - - context 'when trace is stored in old file' do - before do - allow(build).to receive(:has_trace_file?).and_return(true) - allow(build).to receive(:has_old_trace_file?).and_return(true) - end - - it { expect(build.trace_file_path).to eq(build.old_path_to_trace) } - end - end - describe '#update_project_statistics' do let!(:build) { create(:ci_build, artifacts_size: 23) } @@ -1460,7 +1314,7 @@ describe Ci::Build, :models do { key: 'CI_REGISTRY', value: 'registry.example.com', public: true } end let(:ci_registry_image) do - { key: 'CI_REGISTRY_IMAGE', value: project.container_registry_repository_url, public: true } + { key: 'CI_REGISTRY_IMAGE', value: project.container_registry_url, public: true } end context 'and is disabled for project' do diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index e4a24fd63c2..32aa2f4b336 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -12,10 +12,13 @@ describe Ci::Pipeline, models: true do it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:user) } + it { is_expected.to belong_to(:auto_canceled_by) } it { is_expected.to have_many(:statuses) } it { is_expected.to have_many(:trigger_requests) } it { is_expected.to have_many(:builds) } + it { is_expected.to have_many(:auto_canceled_pipelines) } + it { is_expected.to have_many(:auto_canceled_jobs) } it { is_expected.to validate_presence_of :sha } it { is_expected.to validate_presence_of :status } @@ -134,6 +137,43 @@ describe Ci::Pipeline, models: true do end end + describe '#auto_canceled?' do + subject { pipeline.auto_canceled? } + + context 'when it is canceled' do + before do + pipeline.cancel + end + + context 'when there is auto_canceled_by' do + before do + pipeline.update(auto_canceled_by: create(:ci_empty_pipeline)) + end + + it 'is auto canceled' do + is_expected.to be_truthy + end + end + + context 'when there is no auto_canceled_by' do + it 'is not auto canceled' do + is_expected.to be_falsey + end + end + + context 'when it is retried and canceled manually' do + before do + pipeline.enqueue + pipeline.cancel + end + + it 'is not auto canceled' do + is_expected.to be_falsey + end + end + end + end + describe 'pipeline stages' do before do create(:commit_status, pipeline: pipeline, @@ -335,6 +375,14 @@ describe Ci::Pipeline, models: true do end end + describe 'pipeline ETag caching' do + it 'executes ExpirePipelinesCacheService' do + expect_any_instance_of(Ci::ExpirePipelineCacheService).to receive(:execute).with(pipeline) + + pipeline.cancel + end + end + def create_build(name, queued_at = current, started_from = 0) create(:ci_build, name: name, diff --git a/spec/models/ci/trigger_schedule_spec.rb b/spec/models/ci/trigger_schedule_spec.rb new file mode 100644 index 00000000000..75d21541cee --- /dev/null +++ b/spec/models/ci/trigger_schedule_spec.rb @@ -0,0 +1,76 @@ +require 'spec_helper' + +describe Ci::TriggerSchedule, models: true do + it { is_expected.to belong_to(:project) } + it { is_expected.to belong_to(:trigger) } + it { is_expected.to respond_to(:ref) } + + describe '#set_next_run_at' do + context 'when creates new TriggerSchedule' do + before do + trigger_schedule = create(:ci_trigger_schedule, :nightly) + @expected_next_run_at = Gitlab::Ci::CronParser.new(trigger_schedule.cron, trigger_schedule.cron_timezone) + .next_time_from(Time.now) + end + + it 'updates next_run_at automatically' do + expect(Ci::TriggerSchedule.last.next_run_at).to eq(@expected_next_run_at) + end + end + + context 'when updates cron of exsisted TriggerSchedule' do + before do + trigger_schedule = create(:ci_trigger_schedule, :nightly) + new_cron = '0 0 1 1 *' + trigger_schedule.update!(cron: new_cron) # Subject + @expected_next_run_at = Gitlab::Ci::CronParser.new(new_cron, trigger_schedule.cron_timezone) + .next_time_from(Time.now) + end + + it 'updates next_run_at automatically' do + expect(Ci::TriggerSchedule.last.next_run_at).to eq(@expected_next_run_at) + end + end + end + + describe '#schedule_next_run!' do + context 'when reschedules after 10 days from now' do + before do + trigger_schedule = create(:ci_trigger_schedule, :nightly) + time_future = Time.now + 10.days + allow(Time).to receive(:now).and_return(time_future) + trigger_schedule.schedule_next_run! # Subject + @expected_next_run_at = Gitlab::Ci::CronParser.new(trigger_schedule.cron, trigger_schedule.cron_timezone) + .next_time_from(time_future) + end + + it 'points to proper next_run_at' do + expect(Ci::TriggerSchedule.last.next_run_at).to eq(@expected_next_run_at) + end + end + + context 'when cron is invalid' do + before do + trigger_schedule = create(:ci_trigger_schedule, :nightly) + trigger_schedule.cron = 'Invalid-cron' + trigger_schedule.schedule_next_run! # Subject + end + + it 'sets nil to next_run_at' do + expect(Ci::TriggerSchedule.last.next_run_at).to be_nil + end + end + + context 'when cron_timezone is invalid' do + before do + trigger_schedule = create(:ci_trigger_schedule, :nightly) + trigger_schedule.cron_timezone = 'Invalid-cron_timezone' + trigger_schedule.schedule_next_run! # Subject + end + + it 'sets nil to next_run_at' do + expect(Ci::TriggerSchedule.last.next_run_at).to be_nil + end + end + end +end diff --git a/spec/models/ci/trigger_spec.rb b/spec/models/ci/trigger_spec.rb index 1bcb673cb16..d26121018ce 100644 --- a/spec/models/ci/trigger_spec.rb +++ b/spec/models/ci/trigger_spec.rb @@ -7,6 +7,7 @@ describe Ci::Trigger, models: true do it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:owner) } it { is_expected.to have_many(:trigger_requests) } + it { is_expected.to have_one(:trigger_schedule) } end describe 'before_validation' do @@ -16,8 +17,8 @@ describe Ci::Trigger, models: true do expect(trigger.token).not_to be_nil end - it 'does not set an random token if one provided' do - trigger = create(:ci_trigger, project: project) + it 'does not set a random token if one provided' do + trigger = create(:ci_trigger, project: project, token: 'token') expect(trigger.token).to eq('token') end diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index 7343b735a74..0ee85489574 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -16,6 +16,7 @@ describe CommitStatus, :models do it { is_expected.to belong_to(:pipeline) } it { is_expected.to belong_to(:user) } it { is_expected.to belong_to(:project) } + it { is_expected.to belong_to(:auto_canceled_by) } it { is_expected.to validate_presence_of(:name) } it { is_expected.to validate_inclusion_of(:status).in_array(%w(pending running failed success canceled)) } @@ -101,6 +102,32 @@ describe CommitStatus, :models do end end + describe '#auto_canceled?' do + subject { commit_status.auto_canceled? } + + context 'when it is canceled' do + before do + commit_status.update(status: 'canceled') + end + + context 'when there is auto_canceled_by' do + before do + commit_status.update(auto_canceled_by: create(:ci_empty_pipeline)) + end + + it 'is auto canceled' do + is_expected.to be_truthy + end + end + + context 'when there is no auto_canceled_by' do + it 'is not auto canceled' do + is_expected.to be_falsey + end + end + end + end + describe '#duration' do subject { commit_status.duration } diff --git a/spec/models/concerns/has_status_spec.rb b/spec/models/concerns/has_status_spec.rb index 82abad0e2f6..67dae7cf4c0 100644 --- a/spec/models/concerns/has_status_spec.rb +++ b/spec/models/concerns/has_status_spec.rb @@ -231,6 +231,18 @@ describe HasStatus do end end + describe '.created_or_pending' do + subject { CommitStatus.created_or_pending } + + %i[created pending].each do |status| + it_behaves_like 'containing the job', status + end + + %i[running failed success].each do |status| + it_behaves_like 'not containing the job', status + end + end + describe '.finished' do subject { CommitStatus.finished } diff --git a/spec/models/concerns/routable_spec.rb b/spec/models/concerns/routable_spec.rb index 677e60e1282..f191605dbdb 100644 --- a/spec/models/concerns/routable_spec.rb +++ b/spec/models/concerns/routable_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Group, 'Routable' do - let!(:group) { create(:group) } + let!(:group) { create(:group, name: 'foo') } describe 'Validations' do it { is_expected.to validate_presence_of(:route) } @@ -81,6 +81,113 @@ describe Group, 'Routable' do it { is_expected.to eq([nested_group]) } end + describe '.member_self_and_descendants' do + let!(:user) { create(:user) } + let!(:nested_group) { create(:group, parent: group) } + + before { group.add_owner(user) } + subject { described_class.member_self_and_descendants(user.id) } + + it { is_expected.to match_array [group, nested_group] } + end + + describe '.member_hierarchy' do + # foo/bar would also match foo/barbaz instead of just foo/bar and foo/bar/baz + let!(:user) { create(:user) } + + # group + # _______ (foo) _______ + # | | + # | | + # nested_group_1 nested_group_2 + # (bar) (barbaz) + # | | + # | | + # nested_group_1_1 nested_group_2_1 + # (baz) (baz) + # + let!(:nested_group_1) { create :group, parent: group, name: 'bar' } + let!(:nested_group_1_1) { create :group, parent: nested_group_1, name: 'baz' } + let!(:nested_group_2) { create :group, parent: group, name: 'barbaz' } + let!(:nested_group_2_1) { create :group, parent: nested_group_2, name: 'baz' } + + context 'user is not a member of any group' do + subject { described_class.member_hierarchy(user.id) } + + it 'returns an empty array' do + is_expected.to eq [] + end + end + + context 'user is member of all groups' do + before do + group.add_owner(user) + nested_group_1.add_owner(user) + nested_group_1_1.add_owner(user) + nested_group_2.add_owner(user) + nested_group_2_1.add_owner(user) + end + subject { described_class.member_hierarchy(user.id) } + + it 'returns all groups' do + is_expected.to match_array [ + group, + nested_group_1, nested_group_1_1, + nested_group_2, nested_group_2_1 + ] + end + end + + context 'user is member of the top group' do + before { group.add_owner(user) } + subject { described_class.member_hierarchy(user.id) } + + it 'returns all groups' do + is_expected.to match_array [ + group, + nested_group_1, nested_group_1_1, + nested_group_2, nested_group_2_1 + ] + end + end + + context 'user is member of the first child (internal node), branch 1' do + before { nested_group_1.add_owner(user) } + subject { described_class.member_hierarchy(user.id) } + + it 'returns the groups in the hierarchy' do + is_expected.to match_array [ + group, + nested_group_1, nested_group_1_1 + ] + end + end + + context 'user is member of the first child (internal node), branch 2' do + before { nested_group_2.add_owner(user) } + subject { described_class.member_hierarchy(user.id) } + + it 'returns the groups in the hierarchy' do + is_expected.to match_array [ + group, + nested_group_2, nested_group_2_1 + ] + end + end + + context 'user is member of the last child (leaf node)' do + before { nested_group_1_1.add_owner(user) } + subject { described_class.member_hierarchy(user.id) } + + it 'returns the groups in the hierarchy' do + is_expected.to match_array [ + group, + nested_group_1, nested_group_1_1 + ] + end + end + end + describe '#full_path' do let(:group) { create(:group) } let(:nested_group) { create(:group, parent: group) } diff --git a/spec/models/container_repository_spec.rb b/spec/models/container_repository_spec.rb new file mode 100644 index 00000000000..f7ee0b57072 --- /dev/null +++ b/spec/models/container_repository_spec.rb @@ -0,0 +1,209 @@ +require 'spec_helper' + +describe ContainerRepository do + let(:group) { create(:group, name: 'group') } + let(:project) { create(:project, path: 'test', group: group) } + + let(:container_repository) do + create(:container_repository, name: 'my_image', project: project) + end + + before do + stub_container_registry_config(enabled: true, + api_url: 'http://registry.gitlab', + host_port: 'registry.gitlab') + + stub_request(:get, 'http://registry.gitlab/v2/group/test/my_image/tags/list') + .with(headers: { 'Accept' => 'application/vnd.docker.distribution.manifest.v2+json' }) + .to_return( + status: 200, + body: JSON.dump(tags: ['test_tag']), + headers: { 'Content-Type' => 'application/json' }) + end + + describe 'associations' do + it 'belongs to the project' do + expect(container_repository).to belong_to(:project) + end + end + + describe '#tag' do + it 'has a test tag' do + expect(container_repository.tag('test')).not_to be_nil + end + end + + describe '#path' do + it 'returns a full path to the repository' do + expect(container_repository.path).to eq('group/test/my_image') + end + end + + describe '#manifest' do + subject { container_repository.manifest } + + it { is_expected.not_to be_nil } + end + + describe '#valid?' do + subject { container_repository.valid? } + + it { is_expected.to be_truthy } + end + + describe '#tags' do + subject { container_repository.tags } + + it { is_expected.not_to be_empty } + end + + describe '#has_tags?' do + it 'has tags' do + expect(container_repository).to have_tags + end + end + + describe '#delete_tags!' do + let(:container_repository) do + create(:container_repository, name: 'my_image', + tags: %w[latest rc1], + project: project) + end + + context 'when action succeeds' do + it 'returns status that indicates success' do + expect(container_repository.client) + .to receive(:delete_repository_tag) + .and_return(true) + + expect(container_repository.delete_tags!).to be_truthy + end + end + + context 'when action fails' do + it 'returns status that indicates failure' do + expect(container_repository.client) + .to receive(:delete_repository_tag) + .and_return(false) + + expect(container_repository.delete_tags!).to be_falsey + end + end + end + + describe '#root_repository?' do + context 'when repository is a root repository' do + let(:repository) { create(:container_repository, :root) } + + it 'returns true' do + expect(repository).to be_root_repository + end + end + + context 'when repository is not a root repository' do + it 'returns false' do + expect(container_repository).not_to be_root_repository + end + end + end + + describe '.build_from_path' do + let(:registry_path) do + ContainerRegistry::Path.new(project.full_path + '/some/image') + end + + let(:repository) do + described_class.build_from_path(registry_path) + end + + it 'fabricates repository assigned to a correct project' do + expect(repository.project).to eq project + end + + it 'fabricates repository with a correct name' do + expect(repository.name).to eq 'some/image' + end + + it 'is not persisted' do + expect(repository).not_to be_persisted + end + end + + describe '.create_from_path!' do + let(:repository) do + described_class.create_from_path!(ContainerRegistry::Path.new(path)) + end + + let(:repository_path) { ContainerRegistry::Path.new(path) } + + context 'when received multi-level repository path' do + let(:path) { project.full_path + '/some/image' } + + it 'fabricates repository assigned to a correct project' do + expect(repository.project).to eq project + end + + it 'fabricates repository with a correct name' do + expect(repository.name).to eq 'some/image' + end + end + + context 'when path is too long' do + let(:path) do + project.full_path + '/a/b/c/d/e/f/g/h/i/j/k/l/n/o/p/s/t/u/x/y/z' + end + + it 'does not create repository and raises error' do + expect { repository }.to raise_error( + ContainerRegistry::Path::InvalidRegistryPathError) + end + end + + context 'when received multi-level repository with nested groups' do + let(:group) { create(:group, :nested, name: 'nested') } + let(:path) { project.full_path + '/some/image' } + + it 'fabricates repository assigned to a correct project' do + expect(repository.project).to eq project + end + + it 'fabricates repository with a correct name' do + expect(repository.name).to eq 'some/image' + end + + it 'has path including a nested group' do + expect(repository.path).to include 'nested/test/some/image' + end + end + + context 'when received root repository path' do + let(:path) { project.full_path } + + it 'fabricates repository assigned to a correct project' do + expect(repository.project).to eq project + end + + it 'fabricates repository with an empty name' do + expect(repository.name).to be_empty + end + end + end + + describe '.build_root_repository' do + let(:repository) do + described_class.build_root_repository(project) + end + + it 'fabricates a root repository object' do + expect(repository).to be_root_repository + end + + it 'assignes it to the correct project' do + expect(repository.project).to eq project + end + + it 'does not persist it' do + expect(repository).not_to be_persisted + end + end +end diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 9f0e7fbbe26..af7753caba6 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -100,12 +100,26 @@ describe Environment, models: true do let(:head_commit) { project.commit } let(:commit) { project.commit.parent } - it 'returns deployment id for the environment' do - expect(environment.first_deployment_for(commit)).to eq deployment1 + context 'Gitaly find_ref_name feature disabled' do + it 'returns deployment id for the environment' do + expect(environment.first_deployment_for(commit)).to eq deployment1 + end + + it 'return nil when no deployment is found' do + expect(environment.first_deployment_for(head_commit)).to eq nil + end end - it 'return nil when no deployment is found' do - expect(environment.first_deployment_for(head_commit)).to eq nil + context 'Gitaly find_ref_name feature enabled' do + before do + allow(Gitlab::GitalyClient).to receive(:feature_enabled?).with(:find_ref_name).and_return(true) + end + + it 'calls GitalyClient' do + expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:find_ref_name) + + environment.first_deployment_for(commit) + end end end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 5d87938235a..8ffde6f7fbb 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -55,6 +55,8 @@ describe Group, models: true do it { is_expected.to validate_uniqueness_of(:name).scoped_to(:parent_id) } it { is_expected.to validate_presence_of :path } it { is_expected.not_to validate_presence_of :owner } + it { is_expected.to validate_presence_of :two_factor_grace_period } + it { is_expected.to validate_numericality_of(:two_factor_grace_period).is_greater_than_or_equal_to(0) } end describe '.visible_to_user' do @@ -315,4 +317,44 @@ describe Group, models: true do to include(master.id, developer.id) end end + + describe '#update_two_factor_requirement' do + let(:user) { create(:user) } + + before do + group.add_user(user, GroupMember::OWNER) + end + + it 'is called when require_two_factor_authentication is changed' do + expect_any_instance_of(User).to receive(:update_two_factor_requirement) + + group.update!(require_two_factor_authentication: true) + end + + it 'is called when two_factor_grace_period is changed' do + expect_any_instance_of(User).to receive(:update_two_factor_requirement) + + group.update!(two_factor_grace_period: 23) + end + + it 'is not called when other attributes are changed' do + expect_any_instance_of(User).not_to receive(:update_two_factor_requirement) + + group.update!(description: 'foobar') + end + + it 'calls #update_two_factor_requirement on each group member' do + other_user = create(:user) + group.add_user(other_user, GroupMember::OWNER) + + calls = 0 + allow_any_instance_of(User).to receive(:update_two_factor_requirement) do + calls += 1 + end + + group.update!(require_two_factor_authentication: true, two_factor_grace_period: 23) + + expect(calls).to eq 2 + end + end end diff --git a/spec/models/members/group_member_spec.rb b/spec/models/members/group_member_spec.rb index 370aeb9e0a9..024380b7ebb 100644 --- a/spec/models/members/group_member_spec.rb +++ b/spec/models/members/group_member_spec.rb @@ -61,7 +61,7 @@ describe GroupMember, models: true do describe '#after_accept_request' do it 'calls NotificationService.accept_group_access_request' do - member = create(:group_member, user: build_stubbed(:user), requested_at: Time.now) + member = create(:group_member, user: build(:user), requested_at: Time.now) expect_any_instance_of(NotificationService).to receive(:new_group_member) @@ -75,4 +75,19 @@ describe GroupMember, models: true do it { is_expected.to eq 'Group' } end end + + describe '#update_two_factor_requirement' do + let(:user) { build :user } + let(:group_member) { build :group_member, user: user } + + it 'is called after creation and deletion' do + expect(user).to receive(:update_two_factor_requirement) + + group_member.save + + expect(user).to receive(:update_two_factor_requirement) + + group_member.destroy + end + end end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index d9216112259..e406d0a16bd 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -148,18 +148,22 @@ describe Namespace, models: true do expect(@namespace.move_dir).to be_truthy end - context "when any project has container tags" do + context "when any project has container images" do + let(:container_repository) { create(:container_repository) } + before do stub_container_registry_config(enabled: true) - stub_container_registry_tags('tag') + stub_container_registry_tags(repository: :any, tags: ['tag']) - create(:empty_project, namespace: @namespace) + create(:empty_project, namespace: @namespace, container_repositories: [container_repository]) allow(@namespace).to receive(:path_was).and_return(@namespace.path) allow(@namespace).to receive(:path).and_return('new_path') end - it { expect { @namespace.move_dir }.to raise_error('Namespace cannot be moved, because at least one project has tags in container registry') } + it 'raises an error about not movable project' do + expect { @namespace.move_dir }.to raise_error(/Namespace cannot be moved/) + end end context 'with subgroups' do diff --git a/spec/models/project_services/chat_message/issue_message_spec.rb b/spec/models/project_services/chat_message/issue_message_spec.rb index 190ff4c535d..34e2d94b1ed 100644 --- a/spec/models/project_services/chat_message/issue_message_spec.rb +++ b/spec/models/project_services/chat_message/issue_message_spec.rb @@ -7,7 +7,8 @@ describe ChatMessage::IssueMessage, models: true do { user: { name: 'Test User', - username: 'test.user' + username: 'test.user', + avatar_url: 'http://someavatar.com' }, project_name: 'project_name', project_url: 'http://somewhere.com', @@ -25,43 +26,84 @@ describe ChatMessage::IssueMessage, models: true do } end - let(:color) { '#C95823' } + context 'without markdown' do + let(:color) { '#C95823' } - context '#initialize' do - before do - args[:object_attributes][:description] = nil + context '#initialize' do + before do + args[:object_attributes][:description] = nil + end + + it 'returns a non-null description' do + expect(subject.description).to eq('') + end end - it 'returns a non-null description' do - expect(subject.description).to eq('') + context 'open' do + it 'returns a message regarding opening of issues' do + expect(subject.pretext).to eq( + '[<http://somewhere.com|project_name>] Issue opened by test.user') + expect(subject.attachments).to eq([ + { + title: "#100 Issue title", + title_link: "http://url.com", + text: "issue description", + color: color, + } + ]) + end end - end - context 'open' do - it 'returns a message regarding opening of issues' do - expect(subject.pretext).to eq( - '[<http://somewhere.com|project_name>] Issue opened by test.user') - expect(subject.attachments).to eq([ - { - title: "#100 Issue title", - title_link: "http://url.com", - text: "issue description", - color: color, - } - ]) + context 'close' do + before do + args[:object_attributes][:action] = 'close' + args[:object_attributes][:state] = 'closed' + end + + it 'returns a message regarding closing of issues' do + expect(subject.pretext). to eq( + '[<http://somewhere.com|project_name>] Issue <http://url.com|#100 Issue title> closed by test.user') + expect(subject.attachments).to be_empty + end end end - context 'close' do + context 'with markdown' do before do - args[:object_attributes][:action] = 'close' - args[:object_attributes][:state] = 'closed' + args[:markdown] = true end - it 'returns a message regarding closing of issues' do - expect(subject.pretext). to eq( - '[<http://somewhere.com|project_name>] Issue <http://url.com|#100 Issue title> closed by test.user') - expect(subject.attachments).to be_empty + context 'open' do + it 'returns a message regarding opening of issues' do + expect(subject.pretext).to eq( + '[[project_name](http://somewhere.com)] Issue opened by test.user') + expect(subject.attachments).to eq('issue description') + expect(subject.activity).to eq({ + title: 'Issue opened by test.user', + subtitle: 'in [project_name](http://somewhere.com)', + text: '[#100 Issue title](http://url.com)', + image: 'http://someavatar.com' + }) + end + end + + context 'close' do + before do + args[:object_attributes][:action] = 'close' + args[:object_attributes][:state] = 'closed' + end + + it 'returns a message regarding closing of issues' do + expect(subject.pretext). to eq( + '[[project_name](http://somewhere.com)] Issue [#100 Issue title](http://url.com) closed by test.user') + expect(subject.attachments).to be_empty + expect(subject.activity).to eq({ + title: 'Issue closed by test.user', + subtitle: 'in [project_name](http://somewhere.com)', + text: '[#100 Issue title](http://url.com)', + image: 'http://someavatar.com' + }) + end end end end diff --git a/spec/models/project_services/chat_message/merge_message_spec.rb b/spec/models/project_services/chat_message/merge_message_spec.rb index cc154112e90..fa0a1f4a5b7 100644 --- a/spec/models/project_services/chat_message/merge_message_spec.rb +++ b/spec/models/project_services/chat_message/merge_message_spec.rb @@ -7,45 +7,84 @@ describe ChatMessage::MergeMessage, models: true do { user: { name: 'Test User', - username: 'test.user' + username: 'test.user', + avatar_url: 'http://someavatar.com' }, project_name: 'project_name', project_url: 'http://somewhere.com', object_attributes: { - title: "Issue title\nSecond line", + title: "Merge Request title\nSecond line", id: 10, iid: 100, assignee_id: 1, url: 'http://url.com', state: 'opened', - description: 'issue description', + description: 'merge request description', source_branch: 'source_branch', target_branch: 'target_branch', } } end - let(:color) { '#345' } + context 'without markdown' do + let(:color) { '#345' } - context 'open' do - it 'returns a message regarding opening of merge requests' do - expect(subject.pretext).to eq( - 'test.user opened <http://somewhere.com/merge_requests/100|merge request !100> '\ - 'in <http://somewhere.com|project_name>: *Issue title*') - expect(subject.attachments).to be_empty + context 'open' do + it 'returns a message regarding opening of merge requests' do + expect(subject.pretext).to eq( + 'test.user opened <http://somewhere.com/merge_requests/100|!100 *Merge Request title*> in <http://somewhere.com|project_name>: *Merge Request title*') + expect(subject.attachments).to be_empty + end + end + + context 'close' do + before do + args[:object_attributes][:state] = 'closed' + end + it 'returns a message regarding closing of merge requests' do + expect(subject.pretext).to eq( + 'test.user closed <http://somewhere.com/merge_requests/100|!100 *Merge Request title*> in <http://somewhere.com|project_name>: *Merge Request title*') + expect(subject.attachments).to be_empty + end end end - context 'close' do + context 'with markdown' do before do - args[:object_attributes][:state] = 'closed' + args[:markdown] = true + end + + context 'open' do + it 'returns a message regarding opening of merge requests' do + expect(subject.pretext).to eq( + 'test.user opened [!100 *Merge Request title*](http://somewhere.com/merge_requests/100) in [project_name](http://somewhere.com): *Merge Request title*') + expect(subject.attachments).to be_empty + expect(subject.activity).to eq({ + title: 'Merge Request opened by test.user', + subtitle: 'in [project_name](http://somewhere.com)', + text: '[!100 *Merge Request title*](http://somewhere.com/merge_requests/100)', + image: 'http://someavatar.com' + }) + end end - it 'returns a message regarding closing of merge requests' do - expect(subject.pretext).to eq( - 'test.user closed <http://somewhere.com/merge_requests/100|merge request !100> '\ - 'in <http://somewhere.com|project_name>: *Issue title*') - expect(subject.attachments).to be_empty + + context 'close' do + before do + args[:object_attributes][:state] = 'closed' + end + + it 'returns a message regarding closing of merge requests' do + expect(subject.pretext).to eq( + 'test.user closed [!100 *Merge Request title*](http://somewhere.com/merge_requests/100) in [project_name](http://somewhere.com): *Merge Request title*') + expect(subject.attachments).to be_empty + expect(subject.activity).to eq({ + title: 'Merge Request closed by test.user', + subtitle: 'in [project_name](http://somewhere.com)', + text: '[!100 *Merge Request title*](http://somewhere.com/merge_requests/100)', + image: 'http://someavatar.com' + }) + end end end end diff --git a/spec/models/project_services/chat_message/note_message_spec.rb b/spec/models/project_services/chat_message/note_message_spec.rb index da700a08e57..7cd9c61ee2b 100644 --- a/spec/models/project_services/chat_message/note_message_spec.rb +++ b/spec/models/project_services/chat_message/note_message_spec.rb @@ -1,130 +1,190 @@ require 'spec_helper' describe ChatMessage::NoteMessage, models: true do - let(:color) { '#345' } + subject { described_class.new(args) } - before do - @args = { - user: { - name: 'Test User', - username: 'test.user', - avatar_url: 'http://fakeavatar' - }, - project_name: 'project_name', - project_url: 'http://somewhere.com', - repository: { - name: 'project_name', - url: 'http://somewhere.com', - }, - object_attributes: { - id: 10, - note: 'comment on a commit', - url: 'http://url.com', - noteable_type: 'Commit' - } + let(:color) { '#345' } + let(:args) do + { + user: { + name: 'Test User', + username: 'test.user', + avatar_url: 'http://fakeavatar' + }, + project_name: 'project_name', + project_url: 'http://somewhere.com', + repository: { + name: 'project_name', + url: 'http://somewhere.com', + }, + object_attributes: { + id: 10, + note: 'comment on a commit', + url: 'http://url.com', + noteable_type: 'Commit' + } } end context 'commit notes' do before do - @args[:object_attributes][:note] = 'comment on a commit' - @args[:object_attributes][:noteable_type] = 'Commit' - @args[:commit] = { - id: '5f163b2b95e6f53cbd428f5f0b103702a52b9a23', - message: "Added a commit message\ndetails\n123\n" + args[:object_attributes][:note] = 'comment on a commit' + args[:object_attributes][:noteable_type] = 'Commit' + args[:commit] = { + id: '5f163b2b95e6f53cbd428f5f0b103702a52b9a23', + message: "Added a commit message\ndetails\n123\n" } end - it 'returns a message regarding notes on commits' do - message = described_class.new(@args) - expect(message.pretext).to eq("test.user <http://url.com|commented on " \ - "commit 5f163b2b> in <http://somewhere.com|project_name>: " \ - "*Added a commit message*") - expected_attachments = [ - { - text: "comment on a commit", - color: color, - } - ] - expect(message.attachments).to eq(expected_attachments) + context 'without markdown' do + it 'returns a message regarding notes on commits' do + expect(subject.pretext).to eq("test.user <http://url.com|commented on " \ + "commit 5f163b2b> in <http://somewhere.com|project_name>: " \ + "*Added a commit message*") + expect(subject.attachments).to eq([{ + text: 'comment on a commit', + color: color + }]) + end + end + + context 'with markdown' do + before do + args[:markdown] = true + end + + it 'returns a message regarding notes on commits' do + expect(subject.pretext).to eq( + 'test.user [commented on commit 5f163b2b](http://url.com) in [project_name](http://somewhere.com): *Added a commit message*' + ) + expect(subject.attachments).to eq('comment on a commit') + expect(subject.activity).to eq({ + title: 'test.user [commented on commit 5f163b2b](http://url.com)', + subtitle: 'in [project_name](http://somewhere.com)', + text: 'Added a commit message', + image: 'http://fakeavatar' + }) + end end end context 'merge request notes' do before do - @args[:object_attributes][:note] = 'comment on a merge request' - @args[:object_attributes][:noteable_type] = 'MergeRequest' - @args[:merge_request] = { - id: 1, - iid: 30, - title: "merge request title\ndetails\n" + args[:object_attributes][:note] = 'comment on a merge request' + args[:object_attributes][:noteable_type] = 'MergeRequest' + args[:merge_request] = { + id: 1, + iid: 30, + title: "merge request title\ndetails\n" } end - it 'returns a message regarding notes on a merge request' do - message = described_class.new(@args) - expect(message.pretext).to eq("test.user <http://url.com|commented on " \ - "merge request !30> in <http://somewhere.com|project_name>: " \ - "*merge request title*") - expected_attachments = [ - { - text: "comment on a merge request", - color: color, - } - ] - expect(message.attachments).to eq(expected_attachments) + context 'without markdown' do + it 'returns a message regarding notes on a merge request' do + expect(subject.pretext).to eq("test.user <http://url.com|commented on " \ + "merge request !30> in <http://somewhere.com|project_name>: " \ + "*merge request title*") + expect(subject.attachments).to eq([{ + text: 'comment on a merge request', + color: color + }]) + end + end + + context 'with markdown' do + before do + args[:markdown] = true + end + + it 'returns a message regarding notes on a merge request' do + expect(subject.pretext).to eq( + 'test.user [commented on merge request !30](http://url.com) in [project_name](http://somewhere.com): *merge request title*') + expect(subject.attachments).to eq('comment on a merge request') + expect(subject.activity).to eq({ + title: 'test.user [commented on merge request !30](http://url.com)', + subtitle: 'in [project_name](http://somewhere.com)', + text: 'merge request title', + image: 'http://fakeavatar' + }) + end end end context 'issue notes' do before do - @args[:object_attributes][:note] = 'comment on an issue' - @args[:object_attributes][:noteable_type] = 'Issue' - @args[:issue] = { - id: 1, - iid: 20, - title: "issue title\ndetails\n" + args[:object_attributes][:note] = 'comment on an issue' + args[:object_attributes][:noteable_type] = 'Issue' + args[:issue] = { + id: 1, + iid: 20, + title: "issue title\ndetails\n" } end - it 'returns a message regarding notes on an issue' do - message = described_class.new(@args) - expect(message.pretext).to eq( - "test.user <http://url.com|commented on " \ - "issue #20> in <http://somewhere.com|project_name>: " \ - "*issue title*") - expected_attachments = [ - { - text: "comment on an issue", - color: color, - } - ] - expect(message.attachments).to eq(expected_attachments) + context 'without markdown' do + it 'returns a message regarding notes on an issue' do + expect(subject.pretext).to eq( + "test.user <http://url.com|commented on " \ + "issue #20> in <http://somewhere.com|project_name>: " \ + "*issue title*") + expect(subject.attachments).to eq([{ + text: 'comment on an issue', + color: color + }]) + end + end + + context 'with markdown' do + before do + args[:markdown] = true + end + + it 'returns a message regarding notes on an issue' do + expect(subject.pretext).to eq( + 'test.user [commented on issue #20](http://url.com) in [project_name](http://somewhere.com): *issue title*') + expect(subject.attachments).to eq('comment on an issue') + expect(subject.activity).to eq({ + title: 'test.user [commented on issue #20](http://url.com)', + subtitle: 'in [project_name](http://somewhere.com)', + text: 'issue title', + image: 'http://fakeavatar' + }) + end end end context 'project snippet notes' do before do - @args[:object_attributes][:note] = 'comment on a snippet' - @args[:object_attributes][:noteable_type] = 'Snippet' - @args[:snippet] = { - id: 5, - title: "snippet title\ndetails\n" + args[:object_attributes][:note] = 'comment on a snippet' + args[:object_attributes][:noteable_type] = 'Snippet' + args[:snippet] = { + id: 5, + title: "snippet title\ndetails\n" } end - it 'returns a message regarding notes on a project snippet' do - message = described_class.new(@args) - expect(message.pretext).to eq("test.user <http://url.com|commented on " \ - "snippet #5> in <http://somewhere.com|project_name>: " \ - "*snippet title*") - expected_attachments = [ - { - text: "comment on a snippet", - color: color, - } - ] - expect(message.attachments).to eq(expected_attachments) + context 'without markdown' do + it 'returns a message regarding notes on a project snippet' do + expect(subject.pretext).to eq("test.user <http://url.com|commented on " \ + "snippet $5> in <http://somewhere.com|project_name>: " \ + "*snippet title*") + expect(subject.attachments).to eq([{ + text: 'comment on a snippet', + color: color + }]) + end + end + + context 'with markdown' do + before do + args[:markdown] = true + end + + it 'returns a message regarding notes on a project snippet' do + expect(subject.pretext).to eq( + 'test.user [commented on snippet $5](http://url.com) in [project_name](http://somewhere.com): *snippet title*') + expect(subject.attachments).to eq('comment on a snippet') + end end end end diff --git a/spec/models/project_services/chat_message/pipeline_message_spec.rb b/spec/models/project_services/chat_message/pipeline_message_spec.rb index bf2a9616455..ec5c6c5e0ed 100644 --- a/spec/models/project_services/chat_message/pipeline_message_spec.rb +++ b/spec/models/project_services/chat_message/pipeline_message_spec.rb @@ -2,8 +2,8 @@ require 'spec_helper' describe ChatMessage::PipelineMessage do subject { described_class.new(args) } - let(:user) { { name: 'hacker' } } + let(:user) { { name: 'hacker' } } let(:args) do { object_attributes: { @@ -14,54 +14,122 @@ describe ChatMessage::PipelineMessage do status: status, duration: duration }, - project: { path_with_namespace: 'project_name', - web_url: 'http://example.gitlab.com' }, + project: { + path_with_namespace: 'project_name', + web_url: 'http://example.gitlab.com' + }, user: user } end - let(:message) { build_message } + context 'without markdown' do + context 'pipeline succeeded' do + let(:status) { 'success' } + let(:color) { 'good' } + let(:duration) { 10 } + let(:message) { build_message('passed') } + + it 'returns a message with information about succeeded build' do + expect(subject.pretext).to be_empty + expect(subject.fallback).to eq(message) + expect(subject.attachments).to eq([text: message, color: color]) + end + end - context 'pipeline succeeded' do - let(:status) { 'success' } - let(:color) { 'good' } - let(:duration) { 10 } - let(:message) { build_message('passed') } + context 'pipeline failed' do + let(:status) { 'failed' } + let(:color) { 'danger' } + let(:duration) { 10 } + let(:message) { build_message } - it 'returns a message with information about succeeded build' do - verify_message + it 'returns a message with information about failed build' do + expect(subject.pretext).to be_empty + expect(subject.fallback).to eq(message) + expect(subject.attachments).to eq([text: message, color: color]) + end + + context 'when triggered by API therefore lacking user' do + let(:user) { nil } + let(:message) { build_message(status, 'API') } + + it 'returns a message stating it is by API' do + expect(subject.pretext).to be_empty + expect(subject.fallback).to eq(message) + expect(subject.attachments).to eq([text: message, color: color]) + end + end end - end - context 'pipeline failed' do - let(:status) { 'failed' } - let(:color) { 'danger' } - let(:duration) { 10 } + def build_message(status_text = status, name = user[:name]) + "<http://example.gitlab.com|project_name>:" \ + " Pipeline <http://example.gitlab.com/pipelines/123|#123>" \ + " of <http://example.gitlab.com/commits/develop|develop> branch" \ + " by #{name} #{status_text} in #{duration} #{'second'.pluralize(duration)}" + end + end - it 'returns a message with information about failed build' do - verify_message + context 'with markdown' do + before do + args[:markdown] = true end - context 'when triggered by API therefore lacking user' do - let(:user) { nil } - let(:message) { build_message(status, 'API') } + context 'pipeline succeeded' do + let(:status) { 'success' } + let(:color) { 'good' } + let(:duration) { 10 } + let(:message) { build_markdown_message('passed') } - it 'returns a message stating it is by API' do - verify_message + it 'returns a message with information about succeeded build' do + expect(subject.pretext).to be_empty + expect(subject.attachments).to eq(message) + expect(subject.activity).to eq({ + title: 'Pipeline [#123](http://example.gitlab.com/pipelines/123) of [develop](http://example.gitlab.com/commits/develop) branch by hacker passed', + subtitle: 'in [project_name](http://example.gitlab.com)', + text: 'in 10 seconds', + image: '' + }) end end - end - def verify_message - expect(subject.pretext).to be_empty - expect(subject.fallback).to eq(message) - expect(subject.attachments).to eq([text: message, color: color]) - end + context 'pipeline failed' do + let(:status) { 'failed' } + let(:color) { 'danger' } + let(:duration) { 10 } + let(:message) { build_markdown_message } + + it 'returns a message with information about failed build' do + expect(subject.pretext).to be_empty + expect(subject.attachments).to eq(message) + expect(subject.activity).to eq({ + title: 'Pipeline [#123](http://example.gitlab.com/pipelines/123) of [develop](http://example.gitlab.com/commits/develop) branch by hacker failed', + subtitle: 'in [project_name](http://example.gitlab.com)', + text: 'in 10 seconds', + image: '' + }) + end - def build_message(status_text = status, name = user[:name]) - "<http://example.gitlab.com|project_name>:" \ - " Pipeline <http://example.gitlab.com/pipelines/123|#123>" \ - " of <http://example.gitlab.com/commits/develop|develop> branch" \ - " by #{name} #{status_text} in #{duration} #{'second'.pluralize(duration)}" + context 'when triggered by API therefore lacking user' do + let(:user) { nil } + let(:message) { build_markdown_message(status, 'API') } + + it 'returns a message stating it is by API' do + expect(subject.pretext).to be_empty + expect(subject.attachments).to eq(message) + expect(subject.activity).to eq({ + title: 'Pipeline [#123](http://example.gitlab.com/pipelines/123) of [develop](http://example.gitlab.com/commits/develop) branch by API failed', + subtitle: 'in [project_name](http://example.gitlab.com)', + text: 'in 10 seconds', + image: '' + }) + end + end + end + + def build_markdown_message(status_text = status, name = user[:name]) + "[project_name](http://example.gitlab.com):" \ + " Pipeline [#123](http://example.gitlab.com/pipelines/123)" \ + " of [develop](http://example.gitlab.com/commits/develop)" \ + " branch by #{name} #{status_text} in #{duration} #{'second'.pluralize(duration)}" + end end end diff --git a/spec/models/project_services/chat_message/push_message_spec.rb b/spec/models/project_services/chat_message/push_message_spec.rb index 24928873bad..63eb078c44e 100644 --- a/spec/models/project_services/chat_message/push_message_spec.rb +++ b/spec/models/project_services/chat_message/push_message_spec.rb @@ -10,6 +10,7 @@ describe ChatMessage::PushMessage, models: true do project_name: 'project_name', ref: 'refs/heads/master', user_name: 'test.user', + user_avatar: 'http://someavatar.com', project_url: 'http://url.com' } end @@ -24,18 +25,36 @@ describe ChatMessage::PushMessage, models: true do ] end - it 'returns a message regarding pushes' do - expect(subject.pretext).to eq( - 'test.user pushed to branch <http://url.com/commits/master|master> of '\ - '<http://url.com|project_name> (<http://url.com/compare/before...after|Compare changes>)' - ) - expect(subject.attachments).to eq([ - { - text: "<http://url1.com|abcdefgh>: message1 - author1\n"\ - "<http://url2.com|12345678>: message2 - author2", + context 'without markdown' do + it 'returns a message regarding pushes' do + expect(subject.pretext).to eq( + 'test.user pushed to branch <http://url.com/commits/master|master> of '\ + '<http://url.com|project_name> (<http://url.com/compare/before...after|Compare changes>)') + expect(subject.attachments).to eq([{ + text: "<http://url1.com|abcdefgh>: message1 - author1\n\n"\ + "<http://url2.com|12345678>: message2 - author2", color: color, - } - ]) + }]) + end + end + + context 'with markdown' do + before do + args[:markdown] = true + end + + it 'returns a message regarding pushes' do + expect(subject.pretext).to eq( + 'test.user pushed to branch [master](http://url.com/commits/master) of [project_name](http://url.com) ([Compare changes](http://url.com/compare/before...after))') + expect(subject.attachments).to eq( + "[abcdefgh](http://url1.com): message1 - author1\n\n[12345678](http://url2.com): message2 - author2") + expect(subject.activity).to eq({ + title: 'test.user pushed to branch', + subtitle: 'in [project_name](http://url.com)', + text: '[Compare changes](http://url.com/compare/before...after)', + image: 'http://someavatar.com' + }) + end end end @@ -47,15 +66,36 @@ describe ChatMessage::PushMessage, models: true do project_name: 'project_name', ref: 'refs/tags/new_tag', user_name: 'test.user', + user_avatar: 'http://someavatar.com', project_url: 'http://url.com' } end - it 'returns a message regarding pushes' do - expect(subject.pretext).to eq('test.user pushed new tag ' \ - '<http://url.com/commits/new_tag|new_tag> to ' \ - '<http://url.com|project_name>') - expect(subject.attachments).to be_empty + context 'without markdown' do + it 'returns a message regarding pushes' do + expect(subject.pretext).to eq('test.user pushed new tag ' \ + '<http://url.com/commits/new_tag|new_tag> to ' \ + '<http://url.com|project_name>') + expect(subject.attachments).to be_empty + end + end + + context 'with markdown' do + before do + args[:markdown] = true + end + + it 'returns a message regarding pushes' do + expect(subject.pretext).to eq( + 'test.user pushed new tag [new_tag](http://url.com/commits/new_tag) to [project_name](http://url.com)') + expect(subject.attachments).to be_empty + expect(subject.activity).to eq({ + title: 'test.user created tag', + subtitle: 'in [project_name](http://url.com)', + text: '[Compare changes](http://url.com/compare/0000000000000000000000000000000000000000...after)', + image: 'http://someavatar.com' + }) + end end end @@ -64,12 +104,31 @@ describe ChatMessage::PushMessage, models: true do args[:before] = Gitlab::Git::BLANK_SHA end - it 'returns a message regarding a new branch' do - expect(subject.pretext).to eq( - 'test.user pushed new branch <http://url.com/commits/master|master> to '\ - '<http://url.com|project_name>' - ) - expect(subject.attachments).to be_empty + context 'without markdown' do + it 'returns a message regarding a new branch' do + expect(subject.pretext).to eq( + 'test.user pushed new branch <http://url.com/commits/master|master> to '\ + '<http://url.com|project_name>') + expect(subject.attachments).to be_empty + end + end + + context 'with markdown' do + before do + args[:markdown] = true + end + + it 'returns a message regarding a new branch' do + expect(subject.pretext).to eq( + 'test.user pushed new branch [master](http://url.com/commits/master) to [project_name](http://url.com)') + expect(subject.attachments).to be_empty + expect(subject.activity).to eq({ + title: 'test.user created branch', + subtitle: 'in [project_name](http://url.com)', + text: '[Compare changes](http://url.com/compare/0000000000000000000000000000000000000000...after)', + image: 'http://someavatar.com' + }) + end end end @@ -78,11 +137,30 @@ describe ChatMessage::PushMessage, models: true do args[:after] = Gitlab::Git::BLANK_SHA end - it 'returns a message regarding a removed branch' do - expect(subject.pretext).to eq( - 'test.user removed branch master from <http://url.com|project_name>' - ) - expect(subject.attachments).to be_empty + context 'without markdown' do + it 'returns a message regarding a removed branch' do + expect(subject.pretext).to eq( + 'test.user removed branch master from <http://url.com|project_name>') + expect(subject.attachments).to be_empty + end + end + + context 'with markdown' do + before do + args[:markdown] = true + end + + it 'returns a message regarding a removed branch' do + expect(subject.pretext).to eq( + 'test.user removed branch master from [project_name](http://url.com)') + expect(subject.attachments).to be_empty + expect(subject.activity).to eq({ + title: 'test.user removed branch', + subtitle: 'in [project_name](http://url.com)', + text: '[Compare changes](http://url.com/compare/before...0000000000000000000000000000000000000000)', + image: 'http://someavatar.com' + }) + end end end end diff --git a/spec/models/project_services/chat_message/wiki_page_message_spec.rb b/spec/models/project_services/chat_message/wiki_page_message_spec.rb index a2ad61e38e7..0df7db2abc2 100644 --- a/spec/models/project_services/chat_message/wiki_page_message_spec.rb +++ b/spec/models/project_services/chat_message/wiki_page_message_spec.rb @@ -7,7 +7,8 @@ describe ChatMessage::WikiPageMessage, models: true do { user: { name: 'Test User', - username: 'test.user' + username: 'test.user', + avatar_url: 'http://someavatar.com' }, project_name: 'project_name', project_url: 'http://somewhere.com', @@ -19,54 +20,128 @@ describe ChatMessage::WikiPageMessage, models: true do } end - describe '#pretext' do - context 'when :action == "create"' do - before { args[:object_attributes][:action] = 'create' } + context 'without markdown' do + describe '#pretext' do + context 'when :action == "create"' do + before { args[:object_attributes][:action] = 'create' } - it 'returns a message that a new wiki page was created' do - expect(subject.pretext).to eq( - 'test.user created <http://url.com|wiki page> in <http://somewhere.com|project_name>: '\ - '*Wiki page title*') + it 'returns a message that a new wiki page was created' do + expect(subject.pretext).to eq( + 'test.user created <http://url.com|wiki page> in <http://somewhere.com|project_name>: '\ + '*Wiki page title*') + end + end + + context 'when :action == "update"' do + before { args[:object_attributes][:action] = 'update' } + + it 'returns a message that a wiki page was updated' do + expect(subject.pretext).to eq( + 'test.user edited <http://url.com|wiki page> in <http://somewhere.com|project_name>: '\ + '*Wiki page title*') + end end end - context 'when :action == "update"' do - before { args[:object_attributes][:action] = 'update' } + describe '#attachments' do + let(:color) { '#345' } - it 'returns a message that a wiki page was updated' do - expect(subject.pretext).to eq( - 'test.user edited <http://url.com|wiki page> in <http://somewhere.com|project_name>: '\ - '*Wiki page title*') + context 'when :action == "create"' do + before { args[:object_attributes][:action] = 'create' } + + it 'returns the attachment for a new wiki page' do + expect(subject.attachments).to eq([ + { + text: "Wiki page description", + color: color, + } + ]) + end + end + + context 'when :action == "update"' do + before { args[:object_attributes][:action] = 'update' } + + it 'returns the attachment for an updated wiki page' do + expect(subject.attachments).to eq([ + { + text: "Wiki page description", + color: color, + } + ]) + end end end end - describe '#attachments' do - let(:color) { '#345' } + context 'with markdown' do + before do + args[:markdown] = true + end + + describe '#pretext' do + context 'when :action == "create"' do + before { args[:object_attributes][:action] = 'create' } + + it 'returns a message that a new wiki page was created' do + expect(subject.pretext).to eq( + 'test.user created [wiki page](http://url.com) in [project_name](http://somewhere.com): *Wiki page title*') + end + end - context 'when :action == "create"' do - before { args[:object_attributes][:action] = 'create' } + context 'when :action == "update"' do + before { args[:object_attributes][:action] = 'update' } - it 'returns the attachment for a new wiki page' do - expect(subject.attachments).to eq([ - { - text: "Wiki page description", - color: color, - } - ]) + it 'returns a message that a wiki page was updated' do + expect(subject.pretext).to eq( + 'test.user edited [wiki page](http://url.com) in [project_name](http://somewhere.com): *Wiki page title*') + end end end - context 'when :action == "update"' do - before { args[:object_attributes][:action] = 'update' } + describe '#attachments' do + context 'when :action == "create"' do + before { args[:object_attributes][:action] = 'create' } + + it 'returns the attachment for a new wiki page' do + expect(subject.attachments).to eq('Wiki page description') + end + end + + context 'when :action == "update"' do + before { args[:object_attributes][:action] = 'update' } + + it 'returns the attachment for an updated wiki page' do + expect(subject.attachments).to eq('Wiki page description') + end + end + end + + describe '#activity' do + context 'when :action == "create"' do + before { args[:object_attributes][:action] = 'create' } + + it 'returns the attachment for a new wiki page' do + expect(subject.activity).to eq({ + title: 'test.user created [wiki page](http://url.com)', + subtitle: 'in [project_name](http://somewhere.com)', + text: 'Wiki page title', + image: 'http://someavatar.com' + }) + end + end + + context 'when :action == "update"' do + before { args[:object_attributes][:action] = 'update' } - it 'returns the attachment for an updated wiki page' do - expect(subject.attachments).to eq([ - { - text: "Wiki page description", - color: color, - } - ]) + it 'returns the attachment for an updated wiki page' do + expect(subject.activity).to eq({ + title: 'test.user edited [wiki page](http://url.com)', + subtitle: 'in [project_name](http://somewhere.com)', + text: 'Wiki page title', + image: 'http://someavatar.com' + }) + end end end end diff --git a/spec/models/project_services/microsoft_teams_service_spec.rb b/spec/models/project_services/microsoft_teams_service_spec.rb new file mode 100644 index 00000000000..facc034f69c --- /dev/null +++ b/spec/models/project_services/microsoft_teams_service_spec.rb @@ -0,0 +1,277 @@ +require 'spec_helper' + +describe MicrosoftTeamsService, models: true do + let(:chat_service) { described_class.new } + let(:webhook_url) { 'https://example.gitlab.com/' } + + describe "Associations" do + it { is_expected.to belong_to :project } + it { is_expected.to have_one :service_hook } + end + + describe 'Validations' do + context 'when service is active' do + before { subject.active = true } + + it { is_expected.to validate_presence_of(:webhook) } + it_behaves_like 'issue tracker service URL attribute', :webhook + end + + context 'when service is inactive' do + before { subject.active = false } + + it { is_expected.not_to validate_presence_of(:webhook) } + end + end + + describe "#execute" do + let(:user) { create(:user) } + let(:project) { create(:project, :repository) } + + before do + allow(chat_service).to receive_messages( + project: project, + project_id: project.id, + service_hook: true, + webhook: webhook_url + ) + + WebMock.stub_request(:post, webhook_url) + end + + context 'with push events' do + let(:push_sample_data) do + Gitlab::DataBuilder::Push.build_sample(project, user) + end + + it "calls Microsoft Teams API for push events" do + chat_service.execute(push_sample_data) + + expect(WebMock).to have_requested(:post, webhook_url).once + end + + it 'specifies the webhook when it is configured' do + expect(MicrosoftTeams::Notifier).to receive(:new).with(webhook_url).and_return(double(:microsoft_teams_service).as_null_object) + + chat_service.execute(push_sample_data) + end + end + + context 'with issue events' do + let(:opts) { { title: 'Awesome issue', description: 'please fix' } } + let(:issues_sample_data) do + service = Issues::CreateService.new(project, user, opts) + issue = service.execute + service.hook_data(issue, 'open') + end + + it "calls Microsoft Teams API" do + chat_service.execute(issues_sample_data) + + expect(WebMock).to have_requested(:post, webhook_url).once + end + end + + context 'with merge events' do + let(:opts) do + { + title: 'Awesome merge_request', + description: 'please fix', + source_branch: 'feature', + target_branch: 'master' + } + end + + let(:merge_sample_data) do + service = MergeRequests::CreateService.new(project, user, opts) + merge_request = service.execute + service.hook_data(merge_request, 'open') + end + + it "calls Microsoft Teams API" do + chat_service.execute(merge_sample_data) + + expect(WebMock).to have_requested(:post, webhook_url).once + end + end + + context 'with wiki page events' do + let(:opts) do + { + title: "Awesome wiki_page", + content: "Some text describing some thing or another", + format: "md", + message: "user created page: Awesome wiki_page" + } + end + + let(:wiki_page_sample_data) do + service = WikiPages::CreateService.new(project, user, opts) + wiki_page = service.execute + service.hook_data(wiki_page, 'create') + end + + it "calls Microsoft Teams API" do + chat_service.execute(wiki_page_sample_data) + + expect(WebMock).to have_requested(:post, webhook_url).once + end + end + end + + describe "Note events" do + let(:user) { create(:user) } + let(:project) { create(:project, :repository, creator: user) } + + before do + allow(chat_service).to receive_messages( + project: project, + project_id: project.id, + service_hook: true, + webhook: webhook_url + ) + + WebMock.stub_request(:post, webhook_url) + end + + context 'when commit comment event executed' do + let(:commit_note) do + create(:note_on_commit, author: user, + project: project, + commit_id: project.repository.commit.id, + note: 'a comment on a commit') + end + + it "calls Microsoft Teams API for commit comment events" do + data = Gitlab::DataBuilder::Note.build(commit_note, user) + + chat_service.execute(data) + + expect(WebMock).to have_requested(:post, webhook_url).once + end + end + + context 'when merge request comment event executed' do + let(:merge_request_note) do + create(:note_on_merge_request, project: project, + note: "merge request note") + end + + it "calls Microsoft Teams API for merge request comment events" do + data = Gitlab::DataBuilder::Note.build(merge_request_note, user) + + chat_service.execute(data) + + expect(WebMock).to have_requested(:post, webhook_url).once + end + end + + context 'when issue comment event executed' do + let(:issue_note) do + create(:note_on_issue, project: project, note: "issue note") + end + + it "calls Microsoft Teams API for issue comment events" do + data = Gitlab::DataBuilder::Note.build(issue_note, user) + + chat_service.execute(data) + + expect(WebMock).to have_requested(:post, webhook_url).once + end + end + + context 'when snippet comment event executed' do + let(:snippet_note) do + create(:note_on_project_snippet, project: project, + note: "snippet note") + end + + it "calls Microsoft Teams API for snippet comment events" do + data = Gitlab::DataBuilder::Note.build(snippet_note, user) + + chat_service.execute(data) + + expect(WebMock).to have_requested(:post, webhook_url).once + end + end + end + + describe 'Pipeline events' do + let(:user) { create(:user) } + let(:project) { create(:project, :repository) } + + let(:pipeline) do + create(:ci_pipeline, + project: project, status: status, + sha: project.commit.sha, ref: project.default_branch) + end + + before do + allow(chat_service).to receive_messages( + project: project, + service_hook: true, + webhook: webhook_url + ) + end + + shared_examples 'call Microsoft Teams API' do + before do + WebMock.stub_request(:post, webhook_url) + end + + it 'calls Microsoft Teams API for pipeline events' do + data = Gitlab::DataBuilder::Pipeline.build(pipeline) + + chat_service.execute(data) + + expect(WebMock).to have_requested(:post, webhook_url).once + end + end + + context 'with failed pipeline' do + let(:status) { 'failed' } + + it_behaves_like 'call Microsoft Teams API' + end + + context 'with succeeded pipeline' do + let(:status) { 'success' } + + context 'with default to notify_only_broken_pipelines' do + it 'does not call Microsoft Teams API for pipeline events' do + data = Gitlab::DataBuilder::Pipeline.build(pipeline) + result = chat_service.execute(data) + + expect(result).to be_falsy + end + end + + context 'with setting notify_only_broken_pipelines to false' do + before do + chat_service.notify_only_broken_pipelines = false + end + + it_behaves_like 'call Microsoft Teams API' + end + end + + context 'only notify for the default branch' do + context 'when enabled' do + let(:pipeline) do + create(:ci_pipeline, project: project, status: 'failed', ref: 'not-the-default-branch') + end + + before do + chat_service.notify_only_default_branch = true + end + + it 'does not call the Microsoft Teams API for pipeline events' do + data = Gitlab::DataBuilder::Pipeline.build(pipeline) + result = chat_service.execute(data) + + expect(result).to be_falsy + end + end + end + end +end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 59a2560ca06..6d4ef78f8ec 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -22,6 +22,7 @@ describe Project, models: true do it { is_expected.to have_many(:protected_branches).dependent(:destroy) } it { is_expected.to have_one(:forked_project_link).dependent(:destroy) } it { is_expected.to have_one(:slack_service).dependent(:destroy) } + it { is_expected.to have_one(:microsoft_teams_service).dependent(:destroy) } it { is_expected.to have_one(:mattermost_service).dependent(:destroy) } it { is_expected.to have_one(:pushover_service).dependent(:destroy) } it { is_expected.to have_one(:asana_service).dependent(:destroy) } @@ -57,6 +58,7 @@ describe Project, models: true do it { is_expected.to have_many(:builds) } it { is_expected.to have_many(:runner_projects) } it { is_expected.to have_many(:runners) } + it { is_expected.to have_many(:active_runners) } it { is_expected.to have_many(:variables) } it { is_expected.to have_many(:triggers) } it { is_expected.to have_many(:pages_domains) } @@ -1157,11 +1159,12 @@ describe Project, models: true do # Project#gitlab_shell returns a new instance of Gitlab::Shell on every # call. This makes testing a bit easier. allow(project).to receive(:gitlab_shell).and_return(gitlab_shell) - allow(project).to receive(:previous_changes).and_return('path' => ['foo']) end it 'renames a repository' do + stub_container_registry_config(enabled: false) + expect(gitlab_shell).to receive(:mv_repository). ordered. with(project.repository_storage_path, "#{project.namespace.full_path}/foo", "#{project.full_path}"). @@ -1185,10 +1188,13 @@ describe Project, models: true do project.rename_repo end - context 'container registry with tags' do + context 'container registry with images' do + let(:container_repository) { create(:container_repository) } + before do stub_container_registry_config(enabled: true) - stub_container_registry_tags('tag') + stub_container_registry_tags(repository: :any, tags: ['tag']) + project.container_repositories << container_repository end subject { project.rename_repo } @@ -1386,38 +1392,17 @@ describe Project, models: true do end end - describe '#container_registry_path_with_namespace' do - let(:project) { create(:empty_project, path: 'PROJECT') } - - subject { project.container_registry_path_with_namespace } - - it { is_expected.not_to eq(project.path_with_namespace) } - it { is_expected.to eq(project.path_with_namespace.downcase) } - end - - describe '#container_registry_repository' do - let(:project) { create(:empty_project) } - - before { stub_container_registry_config(enabled: true) } - - subject { project.container_registry_repository } - - it { is_expected.not_to be_nil } - end - - describe '#container_registry_repository_url' do + describe '#container_registry_url' do let(:project) { create(:empty_project) } - subject { project.container_registry_repository_url } + subject { project.container_registry_url } before { stub_container_registry_config(**registry_settings) } context 'for enabled registry' do let(:registry_settings) do - { - enabled: true, - host_port: 'example.com', - } + { enabled: true, + host_port: 'example.com' } end it { is_expected.not_to be_nil } @@ -1425,9 +1410,7 @@ describe Project, models: true do context 'for disabled registry' do let(:registry_settings) do - { - enabled: false - } + { enabled: false } end it { is_expected.to be_nil } @@ -1437,28 +1420,60 @@ describe Project, models: true do describe '#has_container_registry_tags?' do let(:project) { create(:empty_project) } - subject { project.has_container_registry_tags? } - - context 'for enabled registry' do + context 'when container registry is enabled' do before { stub_container_registry_config(enabled: true) } - context 'with tags' do - before { stub_container_registry_tags('test', 'test2') } + context 'when tags are present for multi-level registries' do + before do + create(:container_repository, project: project, name: 'image') + + stub_container_registry_tags(repository: /image/, + tags: %w[latest rc1]) + end - it { is_expected.to be_truthy } + it 'should have image tags' do + expect(project).to have_container_registry_tags + end end - context 'when no tags' do - before { stub_container_registry_tags } + context 'when tags are present for root repository' do + before do + stub_container_registry_tags(repository: project.full_path, + tags: %w[latest rc1 pre1]) + end - it { is_expected.to be_falsey } + it 'should have image tags' do + expect(project).to have_container_registry_tags + end + end + + context 'when there are no tags at all' do + before do + stub_container_registry_tags(repository: :any, tags: []) + end + + it 'should not have image tags' do + expect(project).not_to have_container_registry_tags + end end end - context 'for disabled registry' do + context 'when container registry is disabled' do before { stub_container_registry_config(enabled: false) } - it { is_expected.to be_falsey } + it 'should not have image tags' do + expect(project).not_to have_container_registry_tags + end + + it 'should not check root repository tags' do + expect(project).not_to receive(:full_path) + expect(project).not_to have_container_registry_tags + end + + it 'should iterate through container repositories' do + expect(project).to receive(:container_repositories) + expect(project).not_to have_container_registry_tags + end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index a9e37be1157..6f7b9c2388a 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -28,7 +28,6 @@ describe User, models: true do it { is_expected.to have_many(:merge_requests).dependent(:destroy) } it { is_expected.to have_many(:assigned_merge_requests).dependent(:nullify) } it { is_expected.to have_many(:identities).dependent(:destroy) } - it { is_expected.to have_one(:abuse_report) } it { is_expected.to have_many(:spam_logs).dependent(:destroy) } it { is_expected.to have_many(:todos).dependent(:destroy) } it { is_expected.to have_many(:award_emoji).dependent(:destroy) } @@ -37,6 +36,34 @@ describe User, models: true do it { is_expected.to have_many(:pipelines).dependent(:nullify) } it { is_expected.to have_many(:chat_names).dependent(:destroy) } it { is_expected.to have_many(:uploads).dependent(:destroy) } + it { is_expected.to have_many(:reported_abuse_reports).dependent(:destroy).class_name('AbuseReport') } + + describe "#abuse_report" do + let(:current_user) { create(:user) } + let(:other_user) { create(:user) } + + it { is_expected.to have_one(:abuse_report) } + + it "refers to the abuse report whose user_id is the current user" do + abuse_report = create(:abuse_report, reporter: other_user, user: current_user) + + expect(current_user.abuse_report).to eq(abuse_report) + end + + it "does not refer to the abuse report whose reporter_id is the current user" do + create(:abuse_report, reporter: current_user, user: other_user) + + expect(current_user.abuse_report).to be_nil + end + + it "does not update the user_id of an abuse report when the user is updated" do + abuse_report = create(:abuse_report, reporter: current_user, user: other_user) + + current_user.block + + expect(abuse_report.reload.user).to eq(other_user) + end + end describe '#group_members' do it 'does not include group memberships for which user is a requester' do @@ -1407,6 +1434,17 @@ describe User, models: true do it { expect(user.nested_groups).to eq([nested_group]) } end + describe '#all_expanded_groups' do + let!(:user) { create(:user) } + let!(:group) { create(:group) } + let!(:nested_group_1) { create(:group, parent: group) } + let!(:nested_group_2) { create(:group, parent: group) } + + before { nested_group_1.add_owner(user) } + + it { expect(user.all_expanded_groups).to match_array [group, nested_group_1] } + end + describe '#nested_groups_projects' do let!(:user) { create(:user) } let!(:group) { create(:group) } @@ -1521,4 +1559,76 @@ describe User, models: true do end end end + + describe '#update_two_factor_requirement' do + let(:user) { create :user } + + context 'with 2FA requirement on groups' do + let(:group1) { create :group, require_two_factor_authentication: true, two_factor_grace_period: 23 } + let(:group2) { create :group, require_two_factor_authentication: true, two_factor_grace_period: 32 } + + before do + group1.add_user(user, GroupMember::OWNER) + group2.add_user(user, GroupMember::OWNER) + + user.update_two_factor_requirement + end + + it 'requires 2FA' do + expect(user.require_two_factor_authentication_from_group).to be true + end + + it 'uses the shortest grace period' do + expect(user.two_factor_grace_period).to be 23 + end + end + + context 'with 2FA requirement on nested parent group' do + let!(:group1) { create :group, require_two_factor_authentication: true } + let!(:group1a) { create :group, require_two_factor_authentication: false, parent: group1 } + + before do + group1a.add_user(user, GroupMember::OWNER) + + user.update_two_factor_requirement + end + + it 'requires 2FA' do + expect(user.require_two_factor_authentication_from_group).to be true + end + end + + context 'with 2FA requirement on nested child group' do + let!(:group1) { create :group, require_two_factor_authentication: false } + let!(:group1a) { create :group, require_two_factor_authentication: true, parent: group1 } + + before do + group1.add_user(user, GroupMember::OWNER) + + user.update_two_factor_requirement + end + + it 'requires 2FA' do + expect(user.require_two_factor_authentication_from_group).to be true + end + end + + context 'without 2FA requirement on groups' do + let(:group) { create :group } + + before do + group.add_user(user, GroupMember::OWNER) + + user.update_two_factor_requirement + end + + it 'does not require 2FA' do + expect(user.require_two_factor_authentication_from_group).to be false + end + + it 'falls back to the default grace period' do + expect(user.two_factor_grace_period).to be 48 + end + end + end end diff --git a/spec/presenters/ci/build_presenter_spec.rb b/spec/presenters/ci/build_presenter_spec.rb index 7a35da38b2b..2190ab0e82e 100644 --- a/spec/presenters/ci/build_presenter_spec.rb +++ b/spec/presenters/ci/build_presenter_spec.rb @@ -57,6 +57,32 @@ describe Ci::BuildPresenter do end end + describe '#status_title' do + context 'when build is auto-canceled' do + before do + expect(build).to receive(:auto_canceled?).and_return(true) + expect(build).to receive(:auto_canceled_by_id).and_return(1) + end + + it 'shows that the build is auto-canceled' do + status_title = presenter.status_title + + expect(status_title).to include('auto-canceled') + expect(status_title).to include('Pipeline #1') + end + end + + context 'when build is not auto-canceled' do + before do + expect(build).to receive(:auto_canceled?).and_return(false) + end + + it 'does not have a status title' do + expect(presenter.status_title).to be_nil + end + end + end + describe 'quack like a Ci::Build permission-wise' do context 'user is not allowed' do let(:project) { build_stubbed(:empty_project, public_builds: false) } diff --git a/spec/presenters/ci/pipeline_presenter_spec.rb b/spec/presenters/ci/pipeline_presenter_spec.rb new file mode 100644 index 00000000000..9134d1cc31c --- /dev/null +++ b/spec/presenters/ci/pipeline_presenter_spec.rb @@ -0,0 +1,54 @@ +require 'spec_helper' + +describe Ci::PipelinePresenter do + let(:project) { create(:empty_project) } + let(:pipeline) { create(:ci_pipeline, project: project) } + + subject(:presenter) do + described_class.new(pipeline) + end + + it 'inherits from Gitlab::View::Presenter::Delegated' do + expect(described_class.superclass).to eq(Gitlab::View::Presenter::Delegated) + end + + describe '#initialize' do + it 'takes a pipeline and optional params' do + expect { presenter }.not_to raise_error + end + + it 'exposes pipeline' do + expect(presenter.pipeline).to eq(pipeline) + end + + it 'forwards missing methods to pipeline' do + expect(presenter.ref).to eq(pipeline.ref) + end + end + + describe '#status_title' do + context 'when pipeline is auto-canceled' do + before do + expect(pipeline).to receive(:auto_canceled?).and_return(true) + expect(pipeline).to receive(:auto_canceled_by_id).and_return(1) + end + + it 'shows that the pipeline is auto-canceled' do + status_title = presenter.status_title + + expect(status_title).to include('auto-canceled') + expect(status_title).to include('Pipeline #1') + end + end + + context 'when pipeline is not auto-canceled' do + before do + expect(pipeline).to receive(:auto_canceled?).and_return(false) + end + + it 'does not have a status title' do + expect(presenter.status_title).to be_nil + end + end + end +end diff --git a/spec/requests/api/jobs_spec.rb b/spec/requests/api/jobs_spec.rb index 9450701064b..d8a56c02a63 100644 --- a/spec/requests/api/jobs_spec.rb +++ b/spec/requests/api/jobs_spec.rb @@ -320,7 +320,7 @@ describe API::Jobs, api: true do context 'authorized user' do it 'returns specific job trace' do expect(response).to have_http_status(200) - expect(response.body).to eq(build.trace) + expect(response.body).to eq(build.trace.raw) end end @@ -408,7 +408,7 @@ describe API::Jobs, api: true do it 'erases job content' do expect(response).to have_http_status(201) - expect(build.trace).to be_empty + expect(build).not_to have_trace expect(build.artifacts_file.exists?).to be_falsy expect(build.artifacts_metadata.exists?).to be_falsy end diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb index 1cfac7353d4..409a59d6c23 100644 --- a/spec/requests/api/runner_spec.rb +++ b/spec/requests/api/runner_spec.rb @@ -592,7 +592,7 @@ describe API::Runner do update_job(trace: 'BUILD TRACE UPDATED') expect(response).to have_http_status(200) - expect(job.reload.trace).to eq 'BUILD TRACE UPDATED' + expect(job.reload.trace.raw).to eq 'BUILD TRACE UPDATED' end end @@ -600,7 +600,7 @@ describe API::Runner do it 'does not override trace information' do update_job - expect(job.reload.trace).to eq 'BUILD TRACE' + expect(job.reload.trace.raw).to eq 'BUILD TRACE' end end @@ -631,7 +631,7 @@ describe API::Runner do context 'when request is valid' do it 'gets correct response' do expect(response.status).to eq 202 - expect(job.reload.trace).to eq 'BUILD TRACE appended' + expect(job.reload.trace.raw).to eq 'BUILD TRACE appended' expect(response.header).to have_key 'Range' expect(response.header).to have_key 'Job-Status' end @@ -642,7 +642,7 @@ describe API::Runner do it "changes the job's trace" do patch_the_trace - expect(job.reload.trace).to eq 'BUILD TRACE appended appended' + expect(job.reload.trace.raw).to eq 'BUILD TRACE appended appended' end context 'when Runner makes a force-patch' do @@ -651,7 +651,7 @@ describe API::Runner do it "doesn't change the build.trace" do force_patch_the_trace - expect(job.reload.trace).to eq 'BUILD TRACE appended' + expect(job.reload.trace.raw).to eq 'BUILD TRACE appended' end end end @@ -664,7 +664,7 @@ describe API::Runner do it 'changes the job.trace' do patch_the_trace - expect(job.reload.trace).to eq 'BUILD TRACE appended appended' + expect(job.reload.trace.raw).to eq 'BUILD TRACE appended appended' end context 'when Runner makes a force-patch' do @@ -673,7 +673,7 @@ describe API::Runner do it "doesn't change the job.trace" do force_patch_the_trace - expect(job.reload.trace).to eq 'BUILD TRACE appended' + expect(job.reload.trace.raw).to eq 'BUILD TRACE appended' end end end @@ -698,7 +698,7 @@ describe API::Runner do it 'gets correct response' do expect(response.status).to eq 202 - expect(job.reload.trace).to eq 'BUILD TRACE appended' + expect(job.reload.trace.raw).to eq 'BUILD TRACE appended' expect(response.header).to have_key 'Range' expect(response.header).to have_key 'Job-Status' end @@ -738,9 +738,11 @@ describe API::Runner do def patch_the_trace(content = ' appended', request_headers = nil) unless request_headers - offset = job.trace_length - limit = offset + content.length - 1 - request_headers = headers.merge({ 'Content-Range' => "#{offset}-#{limit}" }) + job.trace.read do |stream| + offset = stream.size + limit = offset + content.length - 1 + request_headers = headers.merge({ 'Content-Range' => "#{offset}-#{limit}" }) + end end Timecop.travel(job.updated_at + update_interval) do diff --git a/spec/requests/api/v3/builds_spec.rb b/spec/requests/api/v3/builds_spec.rb index a50c22a6dd1..e97d2b0cee0 100644 --- a/spec/requests/api/v3/builds_spec.rb +++ b/spec/requests/api/v3/builds_spec.rb @@ -330,7 +330,7 @@ describe API::V3::Builds, api: true do context 'authorized user' do it 'returns specific job trace' do expect(response).to have_http_status(200) - expect(response.body).to eq(build.trace) + expect(response.body).to eq(build.trace.raw) end end @@ -418,7 +418,7 @@ describe API::V3::Builds, api: true do it 'erases job content' do expect(response.status).to eq 201 - expect(build.trace).to be_empty + expect(build).not_to have_trace expect(build.artifacts_file.exists?).to be_falsy expect(build.artifacts_metadata.exists?).to be_falsy end diff --git a/spec/requests/ci/api/builds_spec.rb b/spec/requests/ci/api/builds_spec.rb index c879f37f50d..ef30d8638dd 100644 --- a/spec/requests/ci/api/builds_spec.rb +++ b/spec/requests/ci/api/builds_spec.rb @@ -285,7 +285,7 @@ describe Ci::API::Builds do end it 'does not override trace information when no trace is given' do - expect(build.reload.trace).to eq 'BUILD TRACE' + expect(build.reload.trace.raw).to eq 'BUILD TRACE' end context 'job has been erased' do @@ -309,9 +309,11 @@ describe Ci::API::Builds do def patch_the_trace(content = ' appended', request_headers = nil) unless request_headers - offset = build.trace_length - limit = offset + content.length - 1 - request_headers = headers.merge({ 'Content-Range' => "#{offset}-#{limit}" }) + build.trace.read do |stream| + offset = stream.size + limit = offset + content.length - 1 + request_headers = headers.merge({ 'Content-Range' => "#{offset}-#{limit}" }) + end end Timecop.travel(build.updated_at + update_interval) do @@ -335,7 +337,7 @@ describe Ci::API::Builds do context 'when request is valid' do it 'gets correct response' do expect(response.status).to eq 202 - expect(build.reload.trace).to eq 'BUILD TRACE appended' + expect(build.reload.trace.raw).to eq 'BUILD TRACE appended' expect(response.header).to have_key 'Range' expect(response.header).to have_key 'Build-Status' end @@ -346,7 +348,7 @@ describe Ci::API::Builds do it 'changes the build trace' do patch_the_trace - expect(build.reload.trace).to eq 'BUILD TRACE appended appended' + expect(build.reload.trace.raw).to eq 'BUILD TRACE appended appended' end context 'when Runner makes a force-patch' do @@ -355,7 +357,7 @@ describe Ci::API::Builds do it "doesn't change the build.trace" do force_patch_the_trace - expect(build.reload.trace).to eq 'BUILD TRACE appended' + expect(build.reload.trace.raw).to eq 'BUILD TRACE appended' end end end @@ -368,7 +370,7 @@ describe Ci::API::Builds do it 'changes the build.trace' do patch_the_trace - expect(build.reload.trace).to eq 'BUILD TRACE appended appended' + expect(build.reload.trace.raw).to eq 'BUILD TRACE appended appended' end context 'when Runner makes a force-patch' do @@ -377,7 +379,7 @@ describe Ci::API::Builds do it "doesn't change the build.trace" do force_patch_the_trace - expect(build.reload.trace).to eq 'BUILD TRACE appended' + expect(build.reload.trace.raw).to eq 'BUILD TRACE appended' end end end @@ -403,7 +405,7 @@ describe Ci::API::Builds do it 'gets correct response' do expect(response.status).to eq 202 - expect(build.reload.trace).to eq 'BUILD TRACE appended' + expect(build.reload.trace.raw).to eq 'BUILD TRACE appended' expect(response.header).to have_key 'Range' expect(response.header).to have_key 'Build-Status' end diff --git a/spec/serializers/pipeline_serializer_spec.rb b/spec/serializers/pipeline_serializer_spec.rb index 8642b803844..f6249ab4664 100644 --- a/spec/serializers/pipeline_serializer_spec.rb +++ b/spec/serializers/pipeline_serializer_spec.rb @@ -93,6 +93,44 @@ describe PipelineSerializer do end end end + + context 'number of queries' do + let(:resource) { Ci::Pipeline.all } + let(:project) { create(:empty_project) } + + before do + Ci::Pipeline::AVAILABLE_STATUSES.each do |status| + create_pipeline(status) + end + + RequestStore.begin! + end + + after do + RequestStore.end! + RequestStore.clear! + end + + it "verifies number of queries" do + recorded = ActiveRecord::QueryRecorder.new { subject } + expect(recorded.count).to be_within(1).of(50) + expect(recorded.cached_count).to eq(0) + end + + def create_pipeline(status) + create(:ci_empty_pipeline, project: project, status: status).tap do |pipeline| + Ci::Build::AVAILABLE_STATUSES.each do |status| + create_build(pipeline, status, status) + end + end + end + + def create_build(pipeline, stage, status) + create(:ci_build, :tags, :triggered, :artifacts, + pipeline: pipeline, stage: stage, + name: stage, status: status) + end + end end describe '#represent_status' do diff --git a/spec/services/auth/container_registry_authentication_service_spec.rb b/spec/services/auth/container_registry_authentication_service_spec.rb index b91234ddb1e..e273dfe1552 100644 --- a/spec/services/auth/container_registry_authentication_service_spec.rb +++ b/spec/services/auth/container_registry_authentication_service_spec.rb @@ -6,14 +6,15 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do let(:current_params) { {} } let(:rsa_key) { OpenSSL::PKey::RSA.generate(512) } let(:payload) { JWT.decode(subject[:token], rsa_key).first } + let(:authentication_abilities) do - [ - :read_container_image, - :create_container_image - ] + [:read_container_image, :create_container_image] end - subject { described_class.new(current_project, current_user, current_params).execute(authentication_abilities: authentication_abilities) } + subject do + described_class.new(current_project, current_user, current_params) + .execute(authentication_abilities: authentication_abilities) + end before do allow(Gitlab.config.registry).to receive_messages(enabled: true, issuer: 'rspec', key: nil) @@ -40,13 +41,11 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do end end - shared_examples 'a accessible' do + shared_examples 'an accessible' do let(:access) do - [{ - 'type' => 'repository', + [{ 'type' => 'repository', 'name' => project.path_with_namespace, - 'actions' => actions, - }] + 'actions' => actions }] end it_behaves_like 'a valid token' @@ -59,19 +58,19 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do end shared_examples 'a pullable' do - it_behaves_like 'a accessible' do + it_behaves_like 'an accessible' do let(:actions) { ['pull'] } end end shared_examples 'a pushable' do - it_behaves_like 'a accessible' do + it_behaves_like 'an accessible' do let(:actions) { ['push'] } end end shared_examples 'a pullable and pushable' do - it_behaves_like 'a accessible' do + it_behaves_like 'an accessible' do let(:actions) { %w(pull push) } end end @@ -81,15 +80,30 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do it { is_expected.not_to include(:token) } end + shared_examples 'container repository factory' do + it 'creates a new container repository resource' do + expect { subject } + .to change { project.container_repositories.count }.by(1) + end + end + + shared_examples 'not a container repository factory' do + it 'does not create a new container repository resource' do + expect { subject }.not_to change { ContainerRepository.count } + end + end + describe '#full_access_token' do let(:project) { create(:empty_project) } let(:token) { described_class.full_access_token(project.path_with_namespace) } subject { { token: token } } - it_behaves_like 'a accessible' do + it_behaves_like 'an accessible' do let(:actions) { ['*'] } end + + it_behaves_like 'not a container repository factory' end context 'user authorization' do @@ -110,16 +124,20 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do end it_behaves_like 'a pushable' + it_behaves_like 'container repository factory' end context 'allow reporter to pull images' do before { project.team << [current_user, :reporter] } - let(:current_params) do - { scope: "repository:#{project.path_with_namespace}:pull" } - end + context 'when pulling from root level repository' do + let(:current_params) do + { scope: "repository:#{project.path_with_namespace}:pull" } + end - it_behaves_like 'a pullable' + it_behaves_like 'a pullable' + it_behaves_like 'not a container repository factory' + end end context 'return a least of privileges' do @@ -130,6 +148,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do end it_behaves_like 'a pullable' + it_behaves_like 'not a container repository factory' end context 'disallow guest to pull or push images' do @@ -140,6 +159,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do end it_behaves_like 'an inaccessible' + it_behaves_like 'not a container repository factory' end end @@ -152,6 +172,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do end it_behaves_like 'a pullable' + it_behaves_like 'not a container repository factory' end context 'disallow anyone to push images' do @@ -160,6 +181,16 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do end it_behaves_like 'an inaccessible' + it_behaves_like 'not a container repository factory' + end + + context 'when repository name is invalid' do + let(:current_params) do + { scope: 'repository:invalid:push' } + end + + it_behaves_like 'an inaccessible' + it_behaves_like 'not a container repository factory' end end @@ -173,6 +204,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do end it_behaves_like 'a pullable' + it_behaves_like 'not a container repository factory' end context 'disallow anyone to push images' do @@ -181,6 +213,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do end it_behaves_like 'an inaccessible' + it_behaves_like 'not a container repository factory' end end @@ -191,6 +224,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do end it_behaves_like 'an inaccessible' + it_behaves_like 'not a container repository factory' end end end @@ -198,11 +232,9 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do context 'build authorized as user' do let(:current_project) { create(:empty_project) } let(:current_user) { create(:user) } + let(:authentication_abilities) do - [ - :build_read_container_image, - :build_create_container_image - ] + [:build_read_container_image, :build_create_container_image] end before do @@ -219,6 +251,10 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do it_behaves_like 'a pullable and pushable' do let(:project) { current_project } end + + it_behaves_like 'container repository factory' do + let(:project) { current_project } + end end context 'for other projects' do @@ -231,11 +267,13 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do let(:project) { create(:empty_project, :public) } it_behaves_like 'a pullable' + it_behaves_like 'not a container repository factory' end shared_examples 'pullable for being team member' do context 'when you are not member' do it_behaves_like 'an inaccessible' + it_behaves_like 'not a container repository factory' end context 'when you are member' do @@ -244,12 +282,14 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do end it_behaves_like 'a pullable' + it_behaves_like 'not a container repository factory' end context 'when you are owner' do let(:project) { create(:empty_project, namespace: current_user.namespace) } it_behaves_like 'a pullable' + it_behaves_like 'not a container repository factory' end end @@ -263,6 +303,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do context 'when you are not member' do it_behaves_like 'an inaccessible' + it_behaves_like 'not a container repository factory' end context 'when you are member' do @@ -271,12 +312,14 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do end it_behaves_like 'a pullable' + it_behaves_like 'not a container repository factory' end context 'when you are owner' do let(:project) { create(:empty_project, namespace: current_user.namespace) } it_behaves_like 'a pullable' + it_behaves_like 'not a container repository factory' end end end @@ -296,12 +339,14 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do end it_behaves_like 'an inaccessible' + it_behaves_like 'not a container repository factory' end context 'when you are owner' do let(:project) { create(:empty_project, :public, namespace: current_user.namespace) } it_behaves_like 'an inaccessible' + it_behaves_like 'not a container repository factory' end end end @@ -318,6 +363,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do end it_behaves_like 'an inaccessible' + it_behaves_like 'not a container repository factory' end end end @@ -325,6 +371,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do context 'unauthorized' do context 'disallow to use scope-less authentication' do it_behaves_like 'a forbidden' + it_behaves_like 'not a container repository factory' end context 'for invalid scope' do @@ -333,6 +380,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do end it_behaves_like 'a forbidden' + it_behaves_like 'not a container repository factory' end context 'for private project' do @@ -354,6 +402,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do end it_behaves_like 'a pullable' + it_behaves_like 'not a container repository factory' end context 'when pushing' do @@ -362,6 +411,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do end it_behaves_like 'a forbidden' + it_behaves_like 'not a container repository factory' end end end diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index d2f0337c260..fa5014cee07 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -9,72 +9,140 @@ describe Ci::CreatePipelineService, services: true do end describe '#execute' do - def execute(params) + def execute_service(after: project.commit.id, message: 'Message', ref: 'refs/heads/master') + params = { ref: ref, + before: '00000000', + after: after, + commits: [{ message: message }] } + described_class.new(project, user, params).execute end context 'valid params' do - let(:pipeline) do - execute(ref: 'refs/heads/master', - before: '00000000', - after: project.commit.id, - commits: [{ message: "Message" }]) + let(:pipeline) { execute_service } + + let(:pipeline_on_previous_commit) do + execute_service( + after: previous_commit_sha_from_ref('master') + ) end it { expect(pipeline).to be_kind_of(Ci::Pipeline) } it { expect(pipeline).to be_valid } - it { expect(pipeline).to be_persisted } it { expect(pipeline).to eq(project.pipelines.last) } it { expect(pipeline).to have_attributes(user: user) } + it { expect(pipeline).to have_attributes(status: 'pending') } it { expect(pipeline.builds.first).to be_kind_of(Ci::Build) } + + context 'auto-cancel enabled' do + before do + project.update(auto_cancel_pending_pipelines: 'enabled') + end + + it 'does not cancel HEAD pipeline' do + pipeline + pipeline_on_previous_commit + + expect(pipeline.reload).to have_attributes(status: 'pending', auto_canceled_by_id: nil) + end + + it 'auto cancel pending non-HEAD pipelines' do + pipeline_on_previous_commit + pipeline + + expect(pipeline_on_previous_commit.reload).to have_attributes(status: 'canceled', auto_canceled_by_id: pipeline.id) + end + + it 'does not cancel running outdated pipelines' do + pipeline_on_previous_commit.run + execute_service + + expect(pipeline_on_previous_commit.reload).to have_attributes(status: 'running', auto_canceled_by_id: nil) + end + + it 'cancel created outdated pipelines' do + pipeline_on_previous_commit.update(status: 'created') + pipeline + + expect(pipeline_on_previous_commit.reload).to have_attributes(status: 'canceled', auto_canceled_by_id: pipeline.id) + end + + it 'does not cancel pipelines from the other branches' do + pending_pipeline = execute_service( + ref: 'refs/heads/feature', + after: previous_commit_sha_from_ref('feature') + ) + pipeline + + expect(pending_pipeline.reload).to have_attributes(status: 'pending', auto_canceled_by_id: nil) + end + end + + context 'auto-cancel disabled' do + before do + project.update(auto_cancel_pending_pipelines: 'disabled') + end + + it 'does not auto cancel pending non-HEAD pipelines' do + pipeline_on_previous_commit + pipeline + + expect(pipeline_on_previous_commit.reload) + .to have_attributes(status: 'pending', auto_canceled_by_id: nil) + end + end + + def previous_commit_sha_from_ref(ref) + project.commit(ref).parent.sha + end end context "skip tag if there is no build for it" do it "creates commit if there is appropriate job" do - result = execute(ref: 'refs/heads/master', - before: '00000000', - after: project.commit.id, - commits: [{ message: "Message" }]) - expect(result).to be_persisted + expect(execute_service).to be_persisted end it "creates commit if there is no appropriate job but deploy job has right ref setting" do config = YAML.dump({ deploy: { script: "ls", only: ["master"] } }) stub_ci_pipeline_yaml_file(config) - result = execute(ref: 'refs/heads/master', - before: '00000000', - after: project.commit.id, - commits: [{ message: "Message" }]) - expect(result).to be_persisted + expect(execute_service).to be_persisted end end it 'skips creating pipeline for refs without .gitlab-ci.yml' do stub_ci_pipeline_yaml_file(nil) - result = execute(ref: 'refs/heads/master', - before: '00000000', - after: project.commit.id, - commits: [{ message: 'Message' }]) - expect(result).not_to be_persisted + expect(execute_service).not_to be_persisted expect(Ci::Pipeline.count).to eq(0) end - it 'fails commits if yaml is invalid' do - message = 'message' - allow_any_instance_of(Ci::Pipeline).to receive(:git_commit_message) { message } - stub_ci_pipeline_yaml_file('invalid: file: file') - commits = [{ message: message }] - pipeline = execute(ref: 'refs/heads/master', - before: '00000000', - after: project.commit.id, - commits: commits) - - expect(pipeline).to be_persisted - expect(pipeline.builds.any?).to be false - expect(pipeline.status).to eq('failed') - expect(pipeline.yaml_errors).not_to be_nil + shared_examples 'a failed pipeline' do + it 'creates failed pipeline' do + stub_ci_pipeline_yaml_file(ci_yaml) + + pipeline = execute_service(message: message) + + expect(pipeline).to be_persisted + expect(pipeline.builds.any?).to be false + expect(pipeline.status).to eq('failed') + expect(pipeline.yaml_errors).not_to be_nil + end + end + + context 'when yaml is invalid' do + let(:ci_yaml) { 'invalid: file: fiile' } + let(:message) { 'Message' } + + it_behaves_like 'a failed pipeline' + + context 'when receive git commit' do + before do + allow_any_instance_of(Ci::Pipeline).to receive(:git_commit_message) { message } + end + + it_behaves_like 'a failed pipeline' + end end context 'when commit contains a [ci skip] directive' do @@ -97,11 +165,7 @@ describe Ci::CreatePipelineService, services: true do ci_messages.each do |ci_message| it "skips builds creation if the commit message is #{ci_message}" do - commits = [{ message: ci_message }] - pipeline = execute(ref: 'refs/heads/master', - before: '00000000', - after: project.commit.id, - commits: commits) + pipeline = execute_service(message: ci_message) expect(pipeline).to be_persisted expect(pipeline.builds.any?).to be false @@ -109,58 +173,34 @@ describe Ci::CreatePipelineService, services: true do end end - it "does not skips builds creation if there is no [ci skip] or [skip ci] tag in commit message" do - allow_any_instance_of(Ci::Pipeline).to receive(:git_commit_message) { "some message" } + shared_examples 'creating a pipeline' do + it 'does not skip pipeline creation' do + allow_any_instance_of(Ci::Pipeline).to receive(:git_commit_message) { commit_message } - commits = [{ message: "some message" }] - pipeline = execute(ref: 'refs/heads/master', - before: '00000000', - after: project.commit.id, - commits: commits) + pipeline = execute_service(message: commit_message) - expect(pipeline).to be_persisted - expect(pipeline.builds.first.name).to eq("rspec") + expect(pipeline).to be_persisted + expect(pipeline.builds.first.name).to eq("rspec") + end end - it "does not skip builds creation if the commit message is nil" do - allow_any_instance_of(Ci::Pipeline).to receive(:git_commit_message) { nil } - - commits = [{ message: nil }] - pipeline = execute(ref: 'refs/heads/master', - before: '00000000', - after: project.commit.id, - commits: commits) + context 'when commit message does not contain [ci skip] nor [skip ci]' do + let(:commit_message) { 'some message' } - expect(pipeline).to be_persisted - expect(pipeline.builds.first.name).to eq("rspec") + it_behaves_like 'creating a pipeline' end - it "fails builds creation if there is [ci skip] tag in commit message and yaml is invalid" do - stub_ci_pipeline_yaml_file('invalid: file: fiile') - commits = [{ message: message }] - pipeline = execute(ref: 'refs/heads/master', - before: '00000000', - after: project.commit.id, - commits: commits) + context 'when commit message is nil' do + let(:commit_message) { nil } - expect(pipeline).to be_persisted - expect(pipeline.builds.any?).to be false - expect(pipeline.status).to eq("failed") - expect(pipeline.yaml_errors).not_to be_nil + it_behaves_like 'creating a pipeline' end - end - it "creates commit with failed status if yaml is invalid" do - stub_ci_pipeline_yaml_file('invalid: file') - commits = [{ message: "some message" }] - pipeline = execute(ref: 'refs/heads/master', - before: '00000000', - after: project.commit.id, - commits: commits) - - expect(pipeline).to be_persisted - expect(pipeline.status).to eq("failed") - expect(pipeline.builds.any?).to be false + context 'when there is [ci skip] tag in commit message and yaml is invalid' do + let(:ci_yaml) { 'invalid: file: fiile' } + + it_behaves_like 'a failed pipeline' + end end context 'when there are no jobs for this pipeline' do @@ -170,10 +210,7 @@ describe Ci::CreatePipelineService, services: true do end it 'does not create a new pipeline' do - result = execute(ref: 'refs/heads/master', - before: '00000000', - after: project.commit.id, - commits: [{ message: 'some msg' }]) + result = execute_service expect(result).not_to be_persisted expect(Ci::Build.all).to be_empty @@ -188,10 +225,7 @@ describe Ci::CreatePipelineService, services: true do end it 'does not create a new pipeline' do - result = execute(ref: 'refs/heads/master', - before: '00000000', - after: project.commit.id, - commits: [{ message: 'some msg' }]) + result = execute_service expect(result).to be_persisted expect(result.manual_actions).not_to be_empty @@ -205,10 +239,7 @@ describe Ci::CreatePipelineService, services: true do end it 'creates the environment' do - result = execute(ref: 'refs/heads/master', - before: '00000000', - after: project.commit.id, - commits: [{ message: 'some msg' }]) + result = execute_service expect(result).to be_persisted expect(Environment.find_by(name: "review/master")).not_to be_nil diff --git a/spec/services/ci/expire_pipeline_cache_service_spec.rb b/spec/services/ci/expire_pipeline_cache_service_spec.rb new file mode 100644 index 00000000000..3c735872c30 --- /dev/null +++ b/spec/services/ci/expire_pipeline_cache_service_spec.rb @@ -0,0 +1,20 @@ +require 'spec_helper' + +describe Ci::ExpirePipelineCacheService, services: true do + let(:user) { create(:user) } + let(:project) { create(:empty_project) } + let(:pipeline) { create(:ci_pipeline, project: project) } + subject { described_class.new(project, user) } + + describe '#execute' do + it 'invalidate Etag caching for project pipelines path' do + pipelines_path = "/#{project.full_path}/pipelines.json" + new_mr_pipelines_path = "/#{project.full_path}/merge_requests/new.json" + + expect_any_instance_of(Gitlab::EtagCaching::Store).to receive(:touch).with(pipelines_path) + expect_any_instance_of(Gitlab::EtagCaching::Store).to receive(:touch).with(new_mr_pipelines_path) + + subject.execute(pipeline) + end + end +end diff --git a/spec/services/ci/process_pipeline_service_spec.rb b/spec/services/ci/process_pipeline_service_spec.rb index bb98fb37a90..245e19822f3 100644 --- a/spec/services/ci/process_pipeline_service_spec.rb +++ b/spec/services/ci/process_pipeline_service_spec.rb @@ -462,7 +462,9 @@ describe Ci::ProcessPipelineService, '#execute', :services do builds.find_by(name: name).play(user) end - delegate :manual_actions, to: :pipeline + def manual_actions + pipeline.manual_actions(true) + end def create_build(name, **opts) create(:ci_build, :created, pipeline: pipeline, name: name, **opts) diff --git a/spec/services/ci/retry_build_service_spec.rb b/spec/services/ci/retry_build_service_spec.rb index 8567817147b..b2d37657770 100644 --- a/spec/services/ci/retry_build_service_spec.rb +++ b/spec/services/ci/retry_build_service_spec.rb @@ -16,20 +16,21 @@ describe Ci::RetryBuildService, :services do %i[id status user token coverage trace runner artifacts_expire_at artifacts_file artifacts_metadata artifacts_size created_at updated_at started_at finished_at queued_at erased_by - erased_at].freeze + erased_at auto_canceled_by].freeze IGNORE_ACCESSORS = %i[type lock_version target_url base_tags commit_id deployments erased_by_id last_deployment project_id runner_id tag_taggings taggings tags trigger_request_id - user_id].freeze + user_id auto_canceled_by_id].freeze shared_examples 'build duplication' do let(:build) do create(:ci_build, :failed, :artifacts_expired, :erased, :queued, :coverage, :tags, :allowed_to_fail, :on_tag, :teardown_environment, :triggered, :trace, - description: 'some build', pipeline: pipeline) + description: 'some build', pipeline: pipeline, + auto_canceled_by: create(:ci_empty_pipeline)) end describe 'clone accessors' do diff --git a/spec/services/projects/destroy_service_spec.rb b/spec/services/projects/destroy_service_spec.rb index b1e10f4562e..4b8589b2736 100644 --- a/spec/services/projects/destroy_service_spec.rb +++ b/spec/services/projects/destroy_service_spec.rb @@ -7,6 +7,11 @@ describe Projects::DestroyService, services: true do let!(:remove_path) { path.sub(/\.git\Z/, "+#{project.id}+deleted.git") } let!(:async) { false } # execute or async_execute + before do + stub_container_registry_config(enabled: true) + stub_container_registry_tags(repository: :any, tags: []) + end + shared_examples 'deleting the project' do it 'deletes the project' do expect(Project.unscoped.all).not_to include(project) @@ -89,30 +94,64 @@ describe Projects::DestroyService, services: true do it_behaves_like 'deleting the project with pipeline and build' end - context 'container registry' do - before do - stub_container_registry_config(enabled: true) - stub_container_registry_tags('tag') - end + describe 'container registry' do + context 'when there are regular container repositories' do + let(:container_repository) { create(:container_repository) } + + before do + stub_container_registry_tags(repository: project.full_path + '/image', + tags: ['tag']) + project.container_repositories << container_repository + end + + context 'when image repository deletion succeeds' do + it 'removes tags' do + expect_any_instance_of(ContainerRepository) + .to receive(:delete_tags!).and_return(true) + + destroy_project(project, user) + end + end - context 'tags deletion succeeds' do - it do - expect_any_instance_of(ContainerRegistry::Tag).to receive(:delete).and_return(true) + context 'when image repository deletion fails' do + it 'raises an exception' do + expect_any_instance_of(ContainerRepository) + .to receive(:delete_tags!).and_return(false) - destroy_project(project, user, {}) + expect{ destroy_project(project, user) } + .to raise_error(ActiveRecord::RecordNotDestroyed) + end end end - context 'tags deletion fails' do - before { expect_any_instance_of(ContainerRegistry::Tag).to receive(:delete).and_return(false) } + context 'when there are tags for legacy root repository' do + before do + stub_container_registry_tags(repository: project.full_path, + tags: ['tag']) + end + + context 'when image repository tags deletion succeeds' do + it 'removes tags' do + expect_any_instance_of(ContainerRepository) + .to receive(:delete_tags!).and_return(true) - subject { destroy_project(project, user, {}) } + destroy_project(project, user) + end + end + + context 'when image repository tags deletion fails' do + it 'raises an exception' do + expect_any_instance_of(ContainerRepository) + .to receive(:delete_tags!).and_return(false) - it { expect{subject}.to raise_error(Projects::DestroyService::DestroyError) } + expect { destroy_project(project, user) } + .to raise_error(Projects::DestroyService::DestroyError) + end + end end end - def destroy_project(project, user, params) + def destroy_project(project, user, params = {}) if async Projects::DestroyService.new(project, user, params).async_execute else diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb index f8187fefc14..29ccce59c53 100644 --- a/spec/services/projects/transfer_service_spec.rb +++ b/spec/services/projects/transfer_service_spec.rb @@ -29,9 +29,12 @@ describe Projects::TransferService, services: true do end context 'disallow transfering of project with tags' do + let(:container_repository) { create(:container_repository) } + before do stub_container_registry_config(enabled: true) - stub_container_registry_tags('tag') + stub_container_registry_tags(repository: :any, tags: ['tag']) + project.container_repositories << container_repository end subject { transfer_project(project, user, group) } diff --git a/spec/services/users/destroy_spec.rb b/spec/services/users/destroy_service_spec.rb index 66c61b7f8ff..43c18992d1a 100644 --- a/spec/services/users/destroy_spec.rb +++ b/spec/services/users/destroy_service_spec.rb @@ -46,43 +46,47 @@ describe Users::DestroyService, services: true do project.add_developer(user) end - context "for an issue the user has created" do - let!(:issue) { create(:issue, project: project, author: user) } + context "for an issue the user was assigned to" do + let!(:issue) { create(:issue, project: project, assignee: user) } before do service.execute(user) end - it 'does not delete the issue' do + it 'does not delete issues the user is assigned to' do expect(Issue.find_by_id(issue.id)).to be_present end - it 'migrates the issue so that the "Ghost User" is the issue owner' do + it 'migrates the issue so that it is "Unassigned"' do migrated_issue = Issue.find_by_id(issue.id) - expect(migrated_issue.author).to eq(User.ghost) + expect(migrated_issue.assignee).to be_nil end + end + end - it 'blocks the user before migrating issues to the "Ghost User' do - expect(user).to be_blocked - end + context "a deleted user's merge_requests" do + let(:project) { create(:project) } + + before do + project.add_developer(user) end - context "for an issue the user was assigned to" do - let!(:issue) { create(:issue, project: project, assignee: user) } + context "for an merge request the user was assigned to" do + let!(:merge_request) { create(:merge_request, source_project: project, assignee: user) } before do service.execute(user) end - it 'does not delete issues the user is assigned to' do - expect(Issue.find_by_id(issue.id)).to be_present + it 'does not delete merge requests the user is assigned to' do + expect(MergeRequest.find_by_id(merge_request.id)).to be_present end - it 'migrates the issue so that it is "Unassigned"' do - migrated_issue = Issue.find_by_id(issue.id) + it 'migrates the merge request so that it is "Unassigned"' do + migrated_merge_request = MergeRequest.find_by_id(merge_request.id) - expect(migrated_issue.assignee).to be_nil + expect(migrated_merge_request.assignee).to be_nil end end end @@ -141,5 +145,13 @@ describe Users::DestroyService, services: true do expect(User.exists?(user.id)).to be(false) end end + + context "migrating associated records" do + it 'delegates to the `MigrateToGhostUser` service to move associated records to the ghost user' do + expect_any_instance_of(Users::MigrateToGhostUserService).to receive(:execute).once + + service.execute(user) + end + end end end diff --git a/spec/services/users/migrate_to_ghost_user_service_spec.rb b/spec/services/users/migrate_to_ghost_user_service_spec.rb new file mode 100644 index 00000000000..8c5b7e41c15 --- /dev/null +++ b/spec/services/users/migrate_to_ghost_user_service_spec.rb @@ -0,0 +1,64 @@ +require 'spec_helper' + +describe Users::MigrateToGhostUserService, services: true do + let!(:user) { create(:user) } + let!(:project) { create(:project) } + let(:service) { described_class.new(user) } + + context "migrating a user's associated records to the ghost user" do + context 'issues' do + include_examples "migrating a deleted user's associated records to the ghost user", Issue do + let(:created_record) { create(:issue, project: project, author: user) } + let(:assigned_record) { create(:issue, project: project, assignee: user) } + end + end + + context 'merge requests' do + include_examples "migrating a deleted user's associated records to the ghost user", MergeRequest do + let(:created_record) { create(:merge_request, source_project: project, author: user, target_branch: "first") } + let(:assigned_record) { create(:merge_request, source_project: project, assignee: user, target_branch: 'second') } + end + end + + context 'notes' do + include_examples "migrating a deleted user's associated records to the ghost user", Note do + let(:created_record) { create(:note, project: project, author: user) } + end + end + + context 'abuse reports' do + include_examples "migrating a deleted user's associated records to the ghost user", AbuseReport do + let(:created_record) { create(:abuse_report, reporter: user, user: create(:user)) } + end + end + + context 'award emoji' do + include_examples "migrating a deleted user's associated records to the ghost user", AwardEmoji do + let(:created_record) { create(:award_emoji, user: user) } + let(:author_alias) { :user } + + context "when the awardable already has an award emoji of the same name assigned to the ghost user" do + let(:awardable) { create(:issue) } + let!(:existing_award_emoji) { create(:award_emoji, user: User.ghost, name: "thumbsup", awardable: awardable) } + let!(:award_emoji) { create(:award_emoji, user: user, name: "thumbsup", awardable: awardable) } + + it "migrates the award emoji regardless" do + service.execute + + migrated_record = AwardEmoji.find_by_id(award_emoji.id) + + expect(migrated_record.user).to eq(User.ghost) + end + + it "does not leave the migrated award emoji in an invalid state" do + service.execute + + migrated_record = AwardEmoji.find_by_id(award_emoji.id) + + expect(migrated_record).to be_valid + end + end + end + end + end +end diff --git a/spec/support/filtered_search_helpers.rb b/spec/support/filtered_search_helpers.rb index 6b009b132b6..36be0bb6bf8 100644 --- a/spec/support/filtered_search_helpers.rb +++ b/spec/support/filtered_search_helpers.rb @@ -30,7 +30,7 @@ module FilteredSearchHelpers end def clear_search_field - find('.filtered-search-input-container .clear-search').click + find('.filtered-search-box .clear-search').click end def reset_filters @@ -51,7 +51,7 @@ module FilteredSearchHelpers # Iterates through each visual token inside # .tokens-container to make sure the correct names and values are rendered def expect_tokens(tokens) - page.find '.filtered-search-input-container .tokens-container' do + page.find '.filtered-search-box .tokens-container' do page.all(:css, '.tokens-container li').each_with_index do |el, index| token_name = tokens[index][:name] token_value = tokens[index][:value] @@ -71,4 +71,18 @@ module FilteredSearchHelpers def get_filtered_search_placeholder find('.filtered-search')['placeholder'] end + + def remove_recent_searches + execute_script('window.localStorage.removeItem(\'issue-recent-searches\');') + end + + def set_recent_searches(input) + execute_script("window.localStorage.setItem('issue-recent-searches', '#{input}');") + end + + def wait_for_filtered_search(text) + Timeout.timeout(Capybara.default_max_wait_time) do + loop until find('.filtered-search').value.strip == text + end + end end diff --git a/spec/support/query_recorder.rb b/spec/support/query_recorder.rb index e40d5ebd9a8..55b531b4cf7 100644 --- a/spec/support/query_recorder.rb +++ b/spec/support/query_recorder.rb @@ -1,21 +1,29 @@ module ActiveRecord class QueryRecorder - attr_reader :log + attr_reader :log, :cached def initialize(&block) @log = [] + @cached = [] ActiveSupport::Notifications.subscribed(method(:callback), 'sql.active_record', &block) end def callback(name, start, finish, message_id, values) - return if %w(CACHE SCHEMA).include?(values[:name]) - @log << values[:sql] + if values[:name]&.include?("CACHE") + @cached << values[:sql] + elsif !values[:name]&.include?("SCHEMA") + @log << values[:sql] + end end def count @log.count end + def cached_count + @cached.count + end + def log_message @log.join("\n\n") end diff --git a/spec/support/services/migrate_to_ghost_user_service_shared_examples.rb b/spec/support/services/migrate_to_ghost_user_service_shared_examples.rb new file mode 100644 index 00000000000..0eac587e973 --- /dev/null +++ b/spec/support/services/migrate_to_ghost_user_service_shared_examples.rb @@ -0,0 +1,39 @@ +require "spec_helper" + +shared_examples "migrating a deleted user's associated records to the ghost user" do |record_class| + record_class_name = record_class.to_s.titleize.downcase + + let(:project) { create(:project) } + + before do + project.add_developer(user) + end + + context "for a #{record_class_name} the user has created" do + let!(:record) { created_record } + + it "does not delete the #{record_class_name}" do + service.execute + + expect(record_class.find_by_id(record.id)).to be_present + end + + it "migrates the #{record_class_name} so that the 'Ghost User' is the #{record_class_name} owner" do + service.execute + + migrated_record = record_class.find_by_id(record.id) + + if migrated_record.respond_to?(:author) + expect(migrated_record.author).to eq(User.ghost) + else + expect(migrated_record.send(author_alias)).to eq(User.ghost) + end + end + + it "blocks the user before migrating #{record_class_name}s to the 'Ghost User'" do + service.execute + + expect(user).to be_blocked + end + end +end diff --git a/spec/support/stub_gitlab_calls.rb b/spec/support/stub_gitlab_calls.rb index a01ef576234..ded2d593059 100644 --- a/spec/support/stub_gitlab_calls.rb +++ b/spec/support/stub_gitlab_calls.rb @@ -27,23 +27,40 @@ module StubGitlabCalls def stub_container_registry_config(registry_settings) allow(Gitlab.config.registry).to receive_messages(registry_settings) - allow(Auth::ContainerRegistryAuthenticationService).to receive(:full_access_token).and_return('token') + allow(Auth::ContainerRegistryAuthenticationService) + .to receive(:full_access_token).and_return('token') end - def stub_container_registry_tags(*tags) - allow_any_instance_of(ContainerRegistry::Client).to receive(:repository_tags).and_return( - { "tags" => tags } - ) - allow_any_instance_of(ContainerRegistry::Client).to receive(:repository_manifest).and_return( - JSON.parse(File.read(Rails.root + 'spec/fixtures/container_registry/tag_manifest.json')) - ) - allow_any_instance_of(ContainerRegistry::Client).to receive(:blob).and_return( - File.read(Rails.root + 'spec/fixtures/container_registry/config_blob.json') - ) + def stub_container_registry_tags(repository: :any, tags:) + repository = any_args if repository == :any + + allow_any_instance_of(ContainerRegistry::Client) + .to receive(:repository_tags).with(repository) + .and_return({ 'tags' => tags }) + + allow_any_instance_of(ContainerRegistry::Client) + .to receive(:repository_manifest).with(repository) + .and_return(stub_container_registry_tag_manifest) + + allow_any_instance_of(ContainerRegistry::Client) + .to receive(:blob).with(repository) + .and_return(stub_container_registry_blob) end private + def stub_container_registry_tag_manifest + fixture_path = 'spec/fixtures/container_registry/tag_manifest.json' + + JSON.parse(File.read(Rails.root + fixture_path)) + end + + def stub_container_registry_blob + fixture_path = 'spec/fixtures/container_registry/config_blob.json' + + File.read(Rails.root + fixture_path) + end + def gitlab_url Gitlab.config.gitlab.url end diff --git a/spec/views/projects/builds/show.html.haml_spec.rb b/spec/views/projects/builds/show.html.haml_spec.rb index 55b64808fb3..0f39df0f250 100644 --- a/spec/views/projects/builds/show.html.haml_spec.rb +++ b/spec/views/projects/builds/show.html.haml_spec.rb @@ -9,7 +9,7 @@ describe 'projects/builds/show', :view do end before do - assign(:build, build) + assign(:build, build.present) assign(:project, project) allow(view).to receive(:can?).and_return(true) diff --git a/spec/views/projects/pipelines/show.html.haml_spec.rb b/spec/views/projects/pipelines/show.html.haml_spec.rb index dca78dec6df..bb39ec8efbf 100644 --- a/spec/views/projects/pipelines/show.html.haml_spec.rb +++ b/spec/views/projects/pipelines/show.html.haml_spec.rb @@ -5,7 +5,13 @@ describe 'projects/pipelines/show' do let(:user) { create(:user) } let(:project) { create(:project, :repository) } - let(:pipeline) { create(:ci_empty_pipeline, project: project, sha: project.commit.id, user: user) } + + let(:pipeline) do + create(:ci_empty_pipeline, + project: project, + sha: project.commit.id, + user: user) + end before do controller.prepend_view_path('app/views/projects') @@ -21,7 +27,7 @@ describe 'projects/pipelines/show' do create(:generic_commit_status, pipeline: pipeline, stage: 'external', name: 'jenkins', stage_idx: 3) assign(:project, project) - assign(:pipeline, pipeline) + assign(:pipeline, pipeline.present(current_user: user)) assign(:commit, project.commit) allow(view).to receive(:can?).and_return(true) diff --git a/spec/workers/trigger_schedule_worker_spec.rb b/spec/workers/trigger_schedule_worker_spec.rb new file mode 100644 index 00000000000..151e1c2f7b9 --- /dev/null +++ b/spec/workers/trigger_schedule_worker_spec.rb @@ -0,0 +1,58 @@ +require 'spec_helper' + +describe TriggerScheduleWorker do + let(:worker) { described_class.new } + + before do + stub_ci_pipeline_to_return_yaml_file + end + + context 'when there is a scheduled trigger within next_run_at' do + before do + trigger_schedule = create(:ci_trigger_schedule, :nightly) + time_future = Time.now + 10.days + allow(Time).to receive(:now).and_return(time_future) + @next_time = Gitlab::Ci::CronParser.new(trigger_schedule.cron, trigger_schedule.cron_timezone).next_time_from(time_future) + end + + it 'creates a new trigger request' do + expect { worker.perform }.to change { Ci::TriggerRequest.count }.by(1) + end + + it 'creates a new pipeline' do + expect { worker.perform }.to change { Ci::Pipeline.count }.by(1) + expect(Ci::Pipeline.last).to be_pending + end + + it 'updates next_run_at' do + expect { worker.perform }.to change { Ci::TriggerSchedule.last.next_run_at }.to(@next_time) + end + end + + context 'when there are no scheduled triggers within next_run_at' do + before { create(:ci_trigger_schedule, :nightly) } + + it 'does not create a new pipeline' do + expect { worker.perform }.not_to change { Ci::Pipeline.count } + end + + it 'does not update next_run_at' do + expect { worker.perform }.not_to change { Ci::TriggerSchedule.last.next_run_at } + end + end + + context 'when next_run_at is nil' do + before do + trigger_schedule = create(:ci_trigger_schedule, :nightly) + trigger_schedule.update_attribute(:next_run_at, nil) + end + + it 'does not create a new pipeline' do + expect { worker.perform }.not_to change { Ci::Pipeline.count } + end + + it 'does not update next_run_at' do + expect { worker.perform }.not_to change { Ci::TriggerSchedule.last.next_run_at } + end + end +end |