diff options
Diffstat (limited to 'spec')
146 files changed, 2544 insertions, 1060 deletions
diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 4e91068ab88..efc3ce74627 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -650,7 +650,7 @@ describe ApplicationController do describe '#access_denied' do controller(described_class) do def index - access_denied!(params[:message]) + access_denied!(params[:message], params[:status]) end end @@ -669,6 +669,12 @@ describe ApplicationController do expect(response).to have_gitlab_http_status(403) end + + it 'renders a status passed to access denied' do + get :index, status: 401 + + expect(response).to have_gitlab_http_status(401) + end end context 'when invalid UTF-8 parameters are received' do diff --git a/spec/controllers/concerns/issuable_collections_spec.rb b/spec/controllers/concerns/issuable_collections_spec.rb index d16a3464495..e93c923fd39 100644 --- a/spec/controllers/concerns/issuable_collections_spec.rb +++ b/spec/controllers/concerns/issuable_collections_spec.rb @@ -60,7 +60,7 @@ describe IssuableCollections do end end - describe '#filter_params' do + describe '#finder_options' do let(:params) do { assignee_id: '1', @@ -84,25 +84,20 @@ describe IssuableCollections do } end - it 'filters params' do + it 'only allows whitelisted params' do allow(controller).to receive(:cookies).and_return({}) - filtered_params = controller.send(:filter_params) + finder_options = controller.send(:finder_options) - expect(filtered_params).to eq({ + expect(finder_options).to eq({ 'assignee_id' => '1', 'assignee_username' => 'user1', 'author_id' => '2', 'author_username' => 'user2', - 'authorized_only' => 'true', - 'due_date' => '2017-01-01', - 'group_id' => '3', - 'iids' => '4', 'label_name' => 'foo', 'milestone_title' => 'bar', 'my_reaction_emoji' => 'thumbsup', - 'non_archived' => 'true', - 'project_id' => '5', + 'due_date' => '2017-01-01', 'scope' => 'all', 'search' => 'baz', 'sort' => 'priority', diff --git a/spec/controllers/profiles/personal_access_tokens_controller_spec.rb b/spec/controllers/profiles/personal_access_tokens_controller_spec.rb index ed08a4c1bf2..f5860d4296b 100644 --- a/spec/controllers/profiles/personal_access_tokens_controller_spec.rb +++ b/spec/controllers/profiles/personal_access_tokens_controller_spec.rb @@ -39,8 +39,10 @@ describe Profiles::PersonalAccessTokensController do let!(:active_personal_access_token) { create(:personal_access_token, user: user) } let!(:inactive_personal_access_token) { create(:personal_access_token, :revoked, user: user) } let!(:impersonation_personal_access_token) { create(:personal_access_token, :impersonation, user: user) } + let(:token_value) { 's3cr3t' } before do + PersonalAccessToken.redis_store!(user.id, token_value) get :index end @@ -56,5 +58,9 @@ describe Profiles::PersonalAccessTokensController do expect(assigns(:active_personal_access_tokens)).not_to include(impersonation_personal_access_token) expect(assigns(:inactive_personal_access_tokens)).not_to include(impersonation_personal_access_token) end + + it "retrieves newly created personal access token value" do + expect(assigns(:new_personal_access_token)).to eql(token_value) + end end end diff --git a/spec/controllers/projects/blob_controller_spec.rb b/spec/controllers/projects/blob_controller_spec.rb index 74771abde71..5fdf7f1229d 100644 --- a/spec/controllers/projects/blob_controller_spec.rb +++ b/spec/controllers/projects/blob_controller_spec.rb @@ -141,28 +141,6 @@ describe Projects::BlobController do expect(lines.first).to have_key('rich_text') end - context 'comment in any diff line feature flag' do - it 'renders context lines when feature disabled' do - stub_feature_flags(comment_in_any_diff_line: false) - - do_get(since: 1, to: 5, offset: 10, from_merge_request: true) - lines = JSON.parse(response.body) - all_context = lines.all? { |line| line['type'] == 'context' } - - expect(all_context).to be(true) - end - - it 'renders unchanged lines when feature enabled' do - stub_feature_flags(comment_in_any_diff_line: true) - - do_get(since: 1, to: 5, offset: 10, from_merge_request: true) - lines = JSON.parse(response.body) - all_unchanged = lines.all? { |line| line['type'].nil? } - - expect(all_unchanged).to be(true) - end - end - context 'when rendering match lines' do it 'adds top match line when "since" is less than 1' do do_get(since: 5, to: 10, offset: 10, from_merge_request: true) diff --git a/spec/controllers/projects/deploy_keys_controller_spec.rb b/spec/controllers/projects/deploy_keys_controller_spec.rb index 73bf169085f..4567a51b88e 100644 --- a/spec/controllers/projects/deploy_keys_controller_spec.rb +++ b/spec/controllers/projects/deploy_keys_controller_spec.rb @@ -27,12 +27,8 @@ describe Projects::DeployKeysController do let(:project2) { create(:project, :internal)} let(:project_private) { create(:project, :private)} - let(:deploy_key_internal) do - create(:deploy_key, key: 'ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQCdMHEHyhRjbhEZVddFn6lTWdgEy5Q6Bz4nwGB76xWZI5YT/1WJOMEW+sL5zYd31kk7sd3FJ5L9ft8zWMWrr/iWXQikC2cqZK24H1xy+ZUmrRuJD4qGAaIVoyyzBL+avL+lF8J5lg6YSw8gwJY/lX64/vnJHUlWw2n5BF8IFOWhiw== dummy@gitlab.com') - end - let(:deploy_key_actual) do - create(:deploy_key, key: 'ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQDNd/UJWhPrpb+b/G5oL109y57yKuCxE+WUGJGYaj7WQKsYRJmLYh1mgjrl+KVyfsWpq4ylOxIfFSnN9xBBFN8mlb0Fma5DC7YsSsibJr3MZ19ZNBprwNcdogET7aW9I0In7Wu5f2KqI6e5W/spJHCy4JVxzVMUvk6Myab0LnJ2iQ== dummy@gitlab.com') - end + let(:deploy_key_internal) { create(:deploy_key) } + let(:deploy_key_actual) { create(:deploy_key) } let!(:deploy_key_public) { create(:deploy_key, public: true) } let!(:deploy_keys_project_internal) do @@ -63,4 +59,145 @@ describe Projects::DeployKeysController do end end end + + describe '/enable/:id' do + let(:deploy_key) { create(:deploy_key) } + let(:project2) { create(:project) } + let!(:deploy_keys_project_internal) do + create(:deploy_keys_project, project: project2, deploy_key: deploy_key) + end + + context 'with anonymous user' do + before do + sign_out(:user) + end + + it 'redirects to login' do + expect do + put :enable, id: deploy_key.id, namespace_id: project.namespace, project_id: project + end.not_to change { DeployKeysProject.count } + + expect(response).to have_http_status(302) + expect(response).to redirect_to(new_user_session_path) + end + end + + context 'with user with no permission' do + before do + sign_in(create(:user)) + end + + it 'returns 404' do + expect do + put :enable, id: deploy_key.id, namespace_id: project.namespace, project_id: project + end.not_to change { DeployKeysProject.count } + + expect(response).to have_http_status(404) + end + end + + context 'with user with permission' do + before do + project2.add_maintainer(user) + end + + it 'returns 302' do + expect do + put :enable, id: deploy_key.id, namespace_id: project.namespace, project_id: project + end.to change { DeployKeysProject.count }.by(1) + + expect(DeployKeysProject.where(project_id: project.id, deploy_key_id: deploy_key.id).count).to eq(1) + expect(response).to have_http_status(302) + expect(response).to redirect_to(namespace_project_settings_repository_path(anchor: 'js-deploy-keys-settings')) + end + + it 'returns 404' do + put :enable, id: 0, namespace_id: project.namespace, project_id: project + + expect(response).to have_http_status(404) + end + end + + context 'with admin' do + before do + sign_in(create(:admin)) + end + + it 'returns 302' do + expect do + put :enable, id: deploy_key.id, namespace_id: project.namespace, project_id: project + end.to change { DeployKeysProject.count }.by(1) + + expect(DeployKeysProject.where(project_id: project.id, deploy_key_id: deploy_key.id).count).to eq(1) + expect(response).to have_http_status(302) + expect(response).to redirect_to(namespace_project_settings_repository_path(anchor: 'js-deploy-keys-settings')) + end + end + end + + describe '/disable/:id' do + let(:deploy_key) { create(:deploy_key) } + let!(:deploy_key_project) { create(:deploy_keys_project, project: project, deploy_key: deploy_key) } + + context 'with anonymous user' do + before do + sign_out(:user) + end + + it 'redirects to login' do + put :disable, id: deploy_key.id, namespace_id: project.namespace, project_id: project + + expect(response).to have_http_status(302) + expect(response).to redirect_to(new_user_session_path) + expect(DeployKey.find(deploy_key.id)).to eq(deploy_key) + end + end + + context 'with user with no permission' do + before do + sign_in(create(:user)) + end + + it 'returns 404' do + put :disable, id: deploy_key.id, namespace_id: project.namespace, project_id: project + + expect(response).to have_http_status(404) + expect(DeployKey.find(deploy_key.id)).to eq(deploy_key) + end + end + + context 'with user with permission' do + it 'returns 302' do + put :disable, id: deploy_key.id, namespace_id: project.namespace, project_id: project + + expect(response).to have_http_status(302) + expect(response).to redirect_to(namespace_project_settings_repository_path(anchor: 'js-deploy-keys-settings')) + + expect { DeployKey.find(deploy_key.id) }.to raise_error(ActiveRecord::RecordNotFound) + end + + it 'returns 404' do + put :disable, id: 0, namespace_id: project.namespace, project_id: project + + expect(response).to have_http_status(404) + end + end + + context 'with admin' do + before do + sign_in(create(:admin)) + end + + it 'returns 302' do + expect do + put :disable, id: deploy_key.id, namespace_id: project.namespace, project_id: project + end.to change { DeployKey.count }.by(-1) + + expect(response).to have_http_status(302) + expect(response).to redirect_to(namespace_project_settings_repository_path(anchor: 'js-deploy-keys-settings')) + + expect { DeployKey.find(deploy_key.id) }.to raise_error(ActiveRecord::RecordNotFound) + end + end + end end diff --git a/spec/controllers/projects/mirrors_controller_spec.rb b/spec/controllers/projects/mirrors_controller_spec.rb index 00c1e617e3a..976f480930c 100644 --- a/spec/controllers/projects/mirrors_controller_spec.rb +++ b/spec/controllers/projects/mirrors_controller_spec.rb @@ -15,6 +15,31 @@ describe Projects::MirrorsController do end.to change { RemoteMirror.count }.to(1) end end + + context 'setting up SSH public-key authentication' do + let(:ssh_mirror_attributes) do + { + 'auth_method' => 'ssh_public_key', + 'url' => 'ssh://git@example.com', + 'ssh_known_hosts' => 'test' + } + end + + it 'processes a successful update' do + sign_in(project.owner) + do_put(project, remote_mirrors_attributes: { '0' => ssh_mirror_attributes }) + + expect(response).to redirect_to(project_settings_repository_path(project, anchor: 'js-push-remote-settings')) + + expect(RemoteMirror.count).to eq(1) + expect(RemoteMirror.first).to have_attributes( + auth_method: 'ssh_public_key', + url: 'ssh://git@example.com', + ssh_public_key: match(/\Assh-rsa /), + ssh_known_hosts: 'test' + ) + end + end end describe '#update' do diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb index 898f3863008..d334a2ff566 100644 --- a/spec/controllers/registrations_controller_spec.rb +++ b/spec/controllers/registrations_controller_spec.rb @@ -49,7 +49,7 @@ describe RegistrationsController do end it 'displays an error when the reCAPTCHA is not solved' do - # Without this, `verify_recaptcha` arbitraily returns true in test env + # Without this, `verify_recaptcha` arbitrarily returns true in test env Recaptcha.configuration.skip_verify_env.delete('test') post(:create, user_params) diff --git a/spec/controllers/root_controller_spec.rb b/spec/controllers/root_controller_spec.rb index 7688538a468..995f803d757 100644 --- a/spec/controllers/root_controller_spec.rb +++ b/spec/controllers/root_controller_spec.rb @@ -98,7 +98,7 @@ describe RootController do it 'redirects to their assigned issues' do get :index - expect(response).to redirect_to issues_dashboard_path(assignee_id: user.id) + expect(response).to redirect_to issues_dashboard_path(assignee_username: user.username) end end @@ -110,7 +110,7 @@ describe RootController do it 'redirects to their assigned merge requests' do get :index - expect(response).to redirect_to merge_requests_dashboard_path(assignee_id: user.id) + expect(response).to redirect_to merge_requests_dashboard_path(assignee_username: user.username) end end diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb index 8e25b61e2f1..c691b3f478b 100644 --- a/spec/controllers/sessions_controller_spec.rb +++ b/spec/controllers/sessions_controller_spec.rb @@ -89,7 +89,7 @@ describe SessionsController do end it 'displays an error when the reCAPTCHA is not solved' do - # Without this, `verify_recaptcha` arbitraily returns true in test env + # Without this, `verify_recaptcha` arbitrarily returns true in test env Recaptcha.configuration.skip_verify_env.delete('test') counter = double(:counter) diff --git a/spec/db/schema_spec.rb b/spec/db/schema_spec.rb new file mode 100644 index 00000000000..e8584846b56 --- /dev/null +++ b/spec/db/schema_spec.rb @@ -0,0 +1,96 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Database schema' do + let(:connection) { ActiveRecord::Base.connection } + let(:tables) { connection.tables } + + # Use if you are certain that this column should not have a foreign key + IGNORED_FK_COLUMNS = { + abuse_reports: %w[reporter_id user_id], + application_settings: %w[performance_bar_allowed_group_id], + audit_events: %w[author_id entity_id], + award_emoji: %w[awardable_id user_id], + chat_names: %w[chat_id service_id team_id user_id], + chat_teams: %w[team_id], + ci_builds: %w[erased_by_id runner_id trigger_request_id user_id], + ci_pipelines: %w[user_id], + ci_runner_projects: %w[runner_id], + ci_trigger_requests: %w[commit_id], + cluster_providers_gcp: %w[gcp_project_id operation_id], + deploy_keys_projects: %w[deploy_key_id], + deployments: %w[deployable_id environment_id user_id], + emails: %w[user_id], + events: %w[target_id], + forked_project_links: %w[forked_from_project_id], + identities: %w[user_id], + issues: %w[last_edited_by_id], + keys: %w[user_id], + label_links: %w[target_id], + lfs_objects_projects: %w[lfs_object_id project_id], + members: %w[source_id created_by_id], + merge_requests: %w[last_edited_by_id], + namespaces: %w[owner_id parent_id], + notes: %w[author_id commit_id noteable_id updated_by_id resolved_by_id discussion_id], + notification_settings: %w[source_id], + oauth_access_grants: %w[resource_owner_id application_id], + oauth_access_tokens: %w[resource_owner_id application_id], + oauth_applications: %w[owner_id], + project_group_links: %w[group_id], + project_statistics: %w[namespace_id], + projects: %w[creator_id namespace_id ci_id], + redirect_routes: %w[source_id], + repository_languages: %w[programming_language_id], + routes: %w[source_id], + sent_notifications: %w[project_id noteable_id recipient_id commit_id in_reply_to_discussion_id], + snippets: %w[author_id], + spam_logs: %w[user_id], + subscriptions: %w[user_id subscribable_id], + taggings: %w[tag_id taggable_id tagger_id], + timelogs: %w[user_id], + todos: %w[target_id commit_id], + uploads: %w[model_id], + user_agent_details: %w[subject_id], + users: %w[color_scheme_id created_by_id theme_id], + users_star_projects: %w[user_id], + web_hooks: %w[service_id] + }.with_indifferent_access.freeze + + context 'for table' do + ActiveRecord::Base.connection.tables.sort.each do |table| + describe table do + let(:indexes) { connection.indexes(table) } + let(:columns) { connection.columns(table) } + let(:foreign_keys) { connection.foreign_keys(table) } + + context 'all foreign keys' do + # for index to be effective, the FK constraint has to be at first place + it 'are indexed' do + first_indexed_column = indexes.map(&:columns).map(&:first) + foreign_keys_columns = foreign_keys.map(&:column) + + expect(first_indexed_column.uniq).to include(*foreign_keys_columns) + end + end + + context 'columns ending with _id' do + let(:column_names) { columns.map(&:name) } + let(:column_names_with_id) { column_names.select { |column_name| column_name.ends_with?('_id') } } + let(:foreign_keys_columns) { foreign_keys.map(&:column) } + let(:ignored_columns) { ignored_fk_columns(table) } + + it 'do have the foreign keys' do + expect(column_names_with_id - ignored_columns).to contain_exactly(*foreign_keys_columns) + end + end + end + end + end + + private + + def ignored_fk_columns(column) + IGNORED_FK_COLUMNS.fetch(column, []) + end +end diff --git a/spec/factories/clusters/kubernetes_namespaces.rb b/spec/factories/clusters/kubernetes_namespaces.rb index 3f10f0ecc74..6ad93fb0f45 100644 --- a/spec/factories/clusters/kubernetes_namespaces.rb +++ b/spec/factories/clusters/kubernetes_namespaces.rb @@ -3,7 +3,6 @@ FactoryBot.define do factory :cluster_kubernetes_namespace, class: Clusters::KubernetesNamespace do association :cluster, :project, :provided_by_gcp - namespace { |n| "environment#{n}" } after(:build) do |kubernetes_namespace| cluster_project = kubernetes_namespace.cluster.cluster_project @@ -13,7 +12,7 @@ FactoryBot.define do end trait :with_token do - service_account_token { Faker::Lorem.characters(10) } + service_account_token { FFaker::Lorem.characters(10) } end end end diff --git a/spec/fast_spec_helper.rb b/spec/fast_spec_helper.rb index fe475e1f7a0..0b5ab16ad71 100644 --- a/spec/fast_spec_helper.rb +++ b/spec/fast_spec_helper.rb @@ -9,3 +9,4 @@ require 'active_support/all' ActiveSupport::Dependencies.autoload_paths << 'lib' ActiveSupport::Dependencies.autoload_paths << 'ee/lib' +ActiveSupport::XmlMini.backend = 'Nokogiri' diff --git a/spec/features/admin/admin_users_impersonation_tokens_spec.rb b/spec/features/admin/admin_users_impersonation_tokens_spec.rb index e16eae219a4..c7860bebb06 100644 --- a/spec/features/admin/admin_users_impersonation_tokens_spec.rb +++ b/spec/features/admin/admin_users_impersonation_tokens_spec.rb @@ -12,6 +12,10 @@ describe 'Admin > Users > Impersonation Tokens', :js do find(".settings-message") end + def created_impersonation_token + find("#created-personal-access-token").value + end + before do sign_in(admin) end @@ -39,6 +43,7 @@ describe 'Admin > Users > Impersonation Tokens', :js do expect(active_impersonation_tokens).to have_text('api') expect(active_impersonation_tokens).to have_text('read_user') expect(PersonalAccessTokensFinder.new(impersonation: true).execute.count).to equal(1) + expect(created_impersonation_token).not_to be_empty end end diff --git a/spec/features/admin/admin_users_spec.rb b/spec/features/admin/admin_users_spec.rb index d32f33ca1e2..f7c7a257075 100644 --- a/spec/features/admin/admin_users_spec.rb +++ b/spec/features/admin/admin_users_spec.rb @@ -130,7 +130,7 @@ describe "Admin::Users" do context 'with regex to match internal user email address set', :js do before do stub_application_setting(user_default_external: true) - stub_application_setting(user_default_internal_regex: '.internal@') + stub_application_setting(user_default_internal_regex: '\.internal@') visit new_admin_user_path end @@ -169,6 +169,22 @@ describe "Admin::Users" do expects_warning_to_be_hidden end + + it 'creates an internal user' do + user_name = 'tester1' + fill_in 'user_email', with: 'test.internal@domain.ch' + fill_in 'user_name', with: 'tester1 name' + fill_in 'user_username', with: user_name + + expects_external_to_be_unchecked + expects_warning_to_be_shown + + click_button 'Create user' + + new_user = User.find_by(username: user_name) + + expect(new_user.external).to be_falsy + end end end end diff --git a/spec/features/atom/dashboard_issues_spec.rb b/spec/features/atom/dashboard_issues_spec.rb index bd4c00d97b1..5fa1a26f1a6 100644 --- a/spec/features/atom/dashboard_issues_spec.rb +++ b/spec/features/atom/dashboard_issues_spec.rb @@ -25,35 +25,35 @@ describe "Dashboard Issues Feed" do it "renders atom feed via personal access token" do personal_access_token = create(:personal_access_token, user: user) - visit issues_dashboard_path(:atom, private_token: personal_access_token.token, assignee_id: user.id) + visit issues_dashboard_path(:atom, private_token: personal_access_token.token, assignee_username: user.username) expect(response_headers['Content-Type']).to have_content('application/atom+xml') expect(body).to have_selector('title', text: "#{user.name} issues") end it "renders atom feed via feed token" do - visit issues_dashboard_path(:atom, feed_token: user.feed_token, assignee_id: user.id) + visit issues_dashboard_path(:atom, feed_token: user.feed_token, assignee_username: user.username) expect(response_headers['Content-Type']).to have_content('application/atom+xml') expect(body).to have_selector('title', text: "#{user.name} issues") end it "renders atom feed with url parameters" do - visit issues_dashboard_path(:atom, feed_token: user.feed_token, state: 'opened', assignee_id: user.id) + visit issues_dashboard_path(:atom, feed_token: user.feed_token, state: 'opened', assignee_username: user.username) link = find('link[type="application/atom+xml"]') params = CGI.parse(URI.parse(link[:href]).query) expect(params).to include('feed_token' => [user.feed_token]) expect(params).to include('state' => ['opened']) - expect(params).to include('assignee_id' => [user.id.to_s]) + expect(params).to include('assignee_username' => [user.username.to_s]) end context "issue with basic fields" do let!(:issue2) { create(:issue, author: user, assignees: [assignee], project: project2, description: 'test desc') } it "renders issue fields" do - visit issues_dashboard_path(:atom, feed_token: user.feed_token, assignee_id: assignee.id) + visit issues_dashboard_path(:atom, feed_token: user.feed_token, assignee_username: assignee.username) entry = find(:xpath, "//feed/entry[contains(summary/text(),'#{issue2.title}')]") @@ -76,7 +76,7 @@ describe "Dashboard Issues Feed" do end it "renders issue label and milestone info" do - visit issues_dashboard_path(:atom, feed_token: user.feed_token, assignee_id: assignee.id) + visit issues_dashboard_path(:atom, feed_token: user.feed_token, assignee_username: assignee.username) entry = find(:xpath, "//feed/entry[contains(summary/text(),'#{issue1.title}')]") diff --git a/spec/features/calendar_spec.rb b/spec/features/calendar_spec.rb index a1f93bd3fbd..8cb9b57a049 100644 --- a/spec/features/calendar_spec.rb +++ b/spec/features/calendar_spec.rb @@ -64,7 +64,7 @@ describe 'Contributions Calendar', :js do end def selected_day_activities(visible: true) - find('.tab-pane#activity .user-calendar-activities', visible: visible).text + find('#js-overview .user-calendar-activities', visible: visible).text end before do @@ -74,16 +74,16 @@ describe 'Contributions Calendar', :js do describe 'calendar day selection' do before do visit user.username - page.find('.js-activity-tab a').click + page.find('.js-overview-tab a').click wait_for_requests end it 'displays calendar' do - expect(find('.tab-pane#activity')).to have_css('.js-contrib-calendar') + expect(find('#js-overview')).to have_css('.js-contrib-calendar') end describe 'select calendar day' do - let(:cells) { page.all('.tab-pane#activity .user-contrib-cell') } + let(:cells) { page.all('#js-overview .user-contrib-cell') } before do cells[0].click @@ -109,7 +109,7 @@ describe 'Contributions Calendar', :js do describe 'deselect calendar day' do before do cells[0].click - page.find('.js-activity-tab a').click + page.find('.js-overview-tab a').click wait_for_requests end @@ -124,7 +124,7 @@ describe 'Contributions Calendar', :js do shared_context 'visit user page' do before do visit user.username - page.find('.js-activity-tab a').click + page.find('.js-overview-tab a').click wait_for_requests end end @@ -133,12 +133,12 @@ describe 'Contributions Calendar', :js do include_context 'visit user page' it 'displays calendar activity square color for 1 contribution' do - expect(find('.tab-pane#activity')).to have_selector(get_cell_color_selector(contribution_count), count: 1) + expect(find('#js-overview')).to have_selector(get_cell_color_selector(contribution_count), count: 1) end it 'displays calendar activity square on the correct date' do today = Date.today.strftime(date_format) - expect(find('.tab-pane#activity')).to have_selector(get_cell_date_selector(contribution_count, today), count: 1) + expect(find('#js-overview')).to have_selector(get_cell_date_selector(contribution_count, today), count: 1) end end @@ -153,7 +153,7 @@ describe 'Contributions Calendar', :js do include_context 'visit user page' it 'displays calendar activity log' do - expect(find('.tab-pane#activity .content_list .event-target-title')).to have_content issue_title + expect(find('#js-overview .overview-content-list .event-target-title')).to have_content issue_title end end end @@ -185,17 +185,17 @@ describe 'Contributions Calendar', :js do include_context 'visit user page' it 'displays calendar activity squares for both days' do - expect(find('.tab-pane#activity')).to have_selector(get_cell_color_selector(1), count: 2) + expect(find('#js-overview')).to have_selector(get_cell_color_selector(1), count: 2) end it 'displays calendar activity square for yesterday' do yesterday = Date.yesterday.strftime(date_format) - expect(find('.tab-pane#activity')).to have_selector(get_cell_date_selector(1, yesterday), count: 1) + expect(find('#js-overview')).to have_selector(get_cell_date_selector(1, yesterday), count: 1) end it 'displays calendar activity square for today' do today = Date.today.strftime(date_format) - expect(find('.tab-pane#activity')).to have_selector(get_cell_date_selector(1, today), count: 1) + expect(find('#js-overview')).to have_selector(get_cell_date_selector(1, today), count: 1) end end end diff --git a/spec/features/dashboard/issuables_counter_spec.rb b/spec/features/dashboard/issuables_counter_spec.rb index b431f72fcc9..fbc2e5cc3d3 100644 --- a/spec/features/dashboard/issuables_counter_spec.rb +++ b/spec/features/dashboard/issuables_counter_spec.rb @@ -45,11 +45,11 @@ describe 'Navigation bar counter', :use_clean_rails_memory_store_caching do end def issues_path - issues_dashboard_path(assignee_id: user.id) + issues_dashboard_path(assignee_username: user.username) end def merge_requests_path - merge_requests_dashboard_path(assignee_id: user.id) + merge_requests_dashboard_path(assignee_username: user.username) end def expect_counters(issuable_type, count) diff --git a/spec/features/dashboard/issues_filter_spec.rb b/spec/features/dashboard/issues_filter_spec.rb index 95e2610dd4a..c0434f767bb 100644 --- a/spec/features/dashboard/issues_filter_spec.rb +++ b/spec/features/dashboard/issues_filter_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' describe 'Dashboard Issues filtering', :js do include Spec::Support::Helpers::Features::SortingHelpers + include FilteredSearchHelpers let(:user) { create(:user) } let(:project) { create(:project) } @@ -25,27 +26,21 @@ describe 'Dashboard Issues filtering', :js do context 'filtering by milestone' do it 'shows all issues with no milestone' do - show_milestone_dropdown - - click_link 'No Milestone' + input_filtered_search("milestone:none") expect(page).to have_issuable_counts(open: 1, closed: 0, all: 1) expect(page).to have_selector('.issue', count: 1) end it 'shows all issues with the selected milestone' do - show_milestone_dropdown - - page.within '.dropdown-content' do - click_link milestone.title - end + input_filtered_search("milestone:%\"#{milestone.title}\"") expect(page).to have_issuable_counts(open: 1, closed: 0, all: 1) expect(page).to have_selector('.issue', count: 1) end it 'updates atom feed link' do - visit_issues(milestone_title: '', assignee_id: user.id) + visit_issues(milestone_title: '', assignee_username: user.username) link = find('.nav-controls a[title="Subscribe to RSS feed"]') params = CGI.parse(URI.parse(link[:href]).query) @@ -54,10 +49,10 @@ describe 'Dashboard Issues filtering', :js do expect(params).to include('feed_token' => [user.feed_token]) expect(params).to include('milestone_title' => ['']) - expect(params).to include('assignee_id' => [user.id.to_s]) + expect(params).to include('assignee_username' => [user.username.to_s]) expect(auto_discovery_params).to include('feed_token' => [user.feed_token]) expect(auto_discovery_params).to include('milestone_title' => ['']) - expect(auto_discovery_params).to include('assignee_id' => [user.id.to_s]) + expect(auto_discovery_params).to include('assignee_username' => [user.username.to_s]) end end @@ -66,10 +61,7 @@ describe 'Dashboard Issues filtering', :js do let!(:label_link) { create(:label_link, label: label, target: issue) } it 'shows all issues with the selected label' do - page.within '.labels-filter' do - find('.dropdown').click - click_link label.title - end + input_filtered_search("label:~#{label.title}") page.within 'ul.content-list' do expect(page).to have_content issue.title @@ -80,12 +72,12 @@ describe 'Dashboard Issues filtering', :js do context 'sorting' do before do - visit_issues(assignee_id: user.id) + visit_issues(assignee_username: user.username) end it 'remembers last sorting value' do sort_by('Created date') - visit_issues(assignee_id: user.id) + visit_issues(assignee_username: user.username) expect(find('.issues-filters')).to have_content('Created date') end @@ -98,11 +90,6 @@ describe 'Dashboard Issues filtering', :js do end end - def show_milestone_dropdown - click_button 'Milestone' - expect(page).to have_selector('.dropdown-content', visible: true) - end - def visit_issues(*args) visit issues_dashboard_path(*args) end diff --git a/spec/features/dashboard/issues_spec.rb b/spec/features/dashboard/issues_spec.rb index 4ae062f242a..9957bec0f0b 100644 --- a/spec/features/dashboard/issues_spec.rb +++ b/spec/features/dashboard/issues_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' RSpec.describe 'Dashboard Issues' do + include FilteredSearchHelpers + let(:current_user) { create :user } let(:user) { current_user } # Shared examples depend on this being available let!(:public_project) { create(:project, :public) } @@ -14,7 +16,7 @@ RSpec.describe 'Dashboard Issues' do before do [project, project_with_issues_disabled].each { |project| project.add_maintainer(current_user) } sign_in(current_user) - visit issues_dashboard_path(assignee_id: current_user.id) + visit issues_dashboard_path(assignee_username: current_user.username) end describe 'issues' do @@ -24,26 +26,9 @@ RSpec.describe 'Dashboard Issues' do expect(page).not_to have_content(other_issue.title) end - it 'shows checkmark when unassigned is selected for assignee', :js do - find('.js-assignee-search').click - find('li', text: 'Unassigned').click - find('.js-assignee-search').click - - expect(find('li[data-user-id="0"] a.is-active')).to be_visible - end - it 'shows issues when current user is author', :js do - execute_script("document.querySelector('#assignee_id').value=''") - find('.js-author-search', match: :first).click - - expect(find('li[data-user-id="null"] a.is-active')).to be_visible - - find('.dropdown-menu-author li a', match: :first, text: current_user.to_reference).click - find('.js-author-search', match: :first).click - - page.within '.dropdown-menu-user' do - expect(find('.dropdown-menu-author li a.is-active', match: :first, text: current_user.to_reference)).to be_visible - end + reset_filters + input_filtered_search("author:#{current_user.to_reference}") expect(page).to have_content(authored_issue.title) expect(page).to have_content(authored_issue_on_public_project.title) @@ -53,7 +38,7 @@ RSpec.describe 'Dashboard Issues' do it 'state filter tabs work' do find('#state-closed').click - expect(page).to have_current_path(issues_dashboard_url(assignee_id: current_user.id, state: 'closed'), url: true) + expect(page).to have_current_path(issues_dashboard_url(assignee_username: current_user.username, state: 'closed'), url: true) end it_behaves_like "it has an RSS button with current_user's feed token" diff --git a/spec/features/dashboard/label_filter_spec.rb b/spec/features/dashboard/label_filter_spec.rb index 6802974c2ee..2d4659d380f 100644 --- a/spec/features/dashboard/label_filter_spec.rb +++ b/spec/features/dashboard/label_filter_spec.rb @@ -1,6 +1,11 @@ require 'spec_helper' describe 'Dashboard > label filter', :js do + include FilteredSearchHelpers + + let(:filtered_search) { find('.filtered-search') } + let(:filter_dropdown) { find("#js-dropdown-label .filter-dropdown") } + let(:user) { create(:user) } let(:project) { create(:project, name: 'test', namespace: user.namespace) } let(:project2) { create(:project, name: 'test2', path: 'test2', namespace: user.namespace) } @@ -13,17 +18,15 @@ describe 'Dashboard > label filter', :js do sign_in(user) visit issues_dashboard_path + + init_label_search end context 'duplicate labels' do it 'removes duplicate labels' do - page.within('.labels-filter') do - click_button 'Label' - end + filtered_search.send_keys('bu') - page.within('.dropdown-menu-labels') do - expect(page).to have_selector('.dropdown-content a', text: 'bug', count: 1) - end + expect(filter_dropdown).to have_selector('.filter-dropdown-item', text: 'bug', count: 1) end end end diff --git a/spec/features/dashboard/merge_requests_spec.rb b/spec/features/dashboard/merge_requests_spec.rb index f51142f5790..282bf542e77 100644 --- a/spec/features/dashboard/merge_requests_spec.rb +++ b/spec/features/dashboard/merge_requests_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe 'Dashboard Merge Requests' do include Spec::Support::Helpers::Features::SortingHelpers - include FilterItemSelectHelper + include FilteredSearchHelpers include ProjectForksHelper let(:current_user) { create :user } @@ -36,7 +36,7 @@ describe 'Dashboard Merge Requests' do context 'no merge requests exist' do it 'shows an empty state' do - visit merge_requests_dashboard_path(assignee_id: current_user.id) + visit merge_requests_dashboard_path(assignee_username: current_user.username) expect(page).to have_selector('.empty-state') end @@ -79,7 +79,7 @@ describe 'Dashboard Merge Requests' do end before do - visit merge_requests_dashboard_path(assignee_id: current_user.id) + visit merge_requests_dashboard_path(assignee_username: current_user.username) end it 'shows assigned merge requests' do @@ -92,8 +92,8 @@ describe 'Dashboard Merge Requests' do end it 'shows authored merge requests', :js do - filter_item_select('Any Assignee', '.js-assignee-search') - filter_item_select(current_user.to_reference, '.js-author-search') + reset_filters + input_filtered_search("author:#{current_user.to_reference}") expect(page).to have_content(authored_merge_request.title) expect(page).to have_content(authored_merge_request_from_fork.title) @@ -104,8 +104,7 @@ describe 'Dashboard Merge Requests' do end it 'shows error message without filter', :js do - filter_item_select('Any Assignee', '.js-assignee-search') - filter_item_select('Any Author', '.js-author-search') + reset_filters expect(page).to have_content('Please select at least one filter to see results') end @@ -113,7 +112,7 @@ describe 'Dashboard Merge Requests' do it 'shows sorted merge requests' do sort_by('Created date') - visit merge_requests_dashboard_path(assignee_id: current_user.id) + visit merge_requests_dashboard_path(assignee_username: current_user.username) expect(find('.issues-filters')).to have_content('Created date') end diff --git a/spec/features/dashboard/milestone_filter_spec.rb b/spec/features/dashboard/milestone_filter_spec.rb deleted file mode 100644 index 00373050aeb..00000000000 --- a/spec/features/dashboard/milestone_filter_spec.rb +++ /dev/null @@ -1,77 +0,0 @@ -require 'spec_helper' - -describe 'Dashboard > milestone filter', :js do - include FilterItemSelectHelper - - let(:user) { create(:user) } - let(:project) { create(:project, name: 'test', namespace: user.namespace) } - let(:milestone) { create(:milestone, title: 'v1.0', project: project) } - let(:milestone2) { create(:milestone, title: 'v2.0', project: project) } - let!(:issue) { create :issue, author: user, project: project, milestone: milestone } - let!(:issue2) { create :issue, author: user, project: project, milestone: milestone2 } - - dropdown_toggle_button = '.js-milestone-select' - - before do - sign_in(user) - end - - context 'default state' do - it 'shows issues with Any Milestone' do - visit issues_dashboard_path(author_id: user.id) - - page.all('.issue-info').each do |issue_info| - expect(issue_info.text).to match(/v\d.0/) - end - end - end - - context 'filtering by milestone' do - before do - visit issues_dashboard_path(author_id: user.id) - filter_item_select('v1.0', dropdown_toggle_button) - find(dropdown_toggle_button).click - wait_for_requests - end - - it 'shows issues with Milestone v1.0' do - expect(find('.issues-list')).to have_selector('.issue', count: 1) - expect(find('.milestone-filter .dropdown-content')).to have_selector('a.is-active', count: 1) - end - - it 'should not change active Milestone unless clicked' do - page.within '.milestone-filter' do - expect(find('.dropdown-content')).to have_selector('a.is-active', count: 1) - - find('.dropdown-menu-close').click - - expect(page).not_to have_selector('.dropdown.open') - - find(dropdown_toggle_button).click - - expect(find('.dropdown-content')).to have_selector('a.is-active', count: 1) - expect(find('.dropdown-content a.is-active')).to have_content('v1.0') - end - end - end - - context 'with milestone filter in URL' do - before do - visit issues_dashboard_path(author_id: user.id, milestone_title: milestone.title) - find(dropdown_toggle_button).click - wait_for_requests - end - - it 'has milestone selected' do - expect(find('.milestone-filter .dropdown-content')).to have_css('.is-active', text: milestone.title) - end - - it 'removes milestone filter from URL after clicking "Any Milestone"' do - expect(current_url).to include("milestone_title=#{milestone.title}") - - find('.milestone-filter .dropdown-content li', text: 'Any Milestone').click - - expect(current_url).not_to include('milestone_title') - end - end -end diff --git a/spec/features/issues/filtered_search/dropdown_emoji_spec.rb b/spec/features/issues/filtered_search/dropdown_emoji_spec.rb index c42fcd92a36..97dd0afd002 100644 --- a/spec/features/issues/filtered_search/dropdown_emoji_spec.rb +++ b/spec/features/issues/filtered_search/dropdown_emoji_spec.rb @@ -20,7 +20,7 @@ describe 'Dropdown emoji', :js do end def dropdown_emoji_size - page.all('#js-dropdown-my-reaction .filter-dropdown .filter-dropdown-item').size + all('gl-emoji[data-name]').size end def click_emoji(text) diff --git a/spec/features/issues/form_spec.rb b/spec/features/issues/form_spec.rb index 1456a2f0375..f2e4c5779df 100644 --- a/spec/features/issues/form_spec.rb +++ b/spec/features/issues/form_spec.rb @@ -27,7 +27,7 @@ describe 'New/edit issue', :js do before do # Using `allow_any_instance_of`/`and_wrap_original`, `original` would # somehow refer to the very block we defined to _wrap_ that method, instead of - # the original method, resulting in infinite recurison when called. + # the original method, resulting in infinite recursion when called. # This is likely a bug with helper modules included into dynamically generated view classes. # To work around this, we have to hold on to and call to the original implementation manually. original_issue_dropdown_options = FormHelper.instance_method(:issue_assignees_dropdown_options) diff --git a/spec/features/issues/gfm_autocomplete_spec.rb b/spec/features/issues/gfm_autocomplete_spec.rb index 605860b90cd..7c591dacce5 100644 --- a/spec/features/issues/gfm_autocomplete_spec.rb +++ b/spec/features/issues/gfm_autocomplete_spec.rb @@ -1,6 +1,10 @@ require 'rails_helper' describe 'GFM autocomplete', :js do + let(:issue_xss_title) { 'This will execute alert<img src=x onerror=alert(2)<img src=x onerror=alert(1)>' } + let(:user_xss_title) { 'eve <img src=x onerror=alert(2)<img src=x onerror=alert(1)>' } + + let(:user_xss) { create(:user, name: user_xss_title, username: 'xss.user') } let(:user) { create(:user, name: '💃speciąl someone💃', username: 'someone.special') } let(:project) { create(:project) } let(:label) { create(:label, project: project, title: 'special+') } @@ -9,6 +13,8 @@ describe 'GFM autocomplete', :js do before do project.add_maintainer(user) + project.add_maintainer(user_xss) + sign_in(user) visit project_issue_path(project, issue) @@ -35,9 +41,8 @@ describe 'GFM autocomplete', :js do expect(page).to have_selector('.atwho-container') end - it 'opens autocomplete menu when field starts with text with item escaping HTML characters' do - alert_title = 'This will execute alert<img src=x onerror=alert(2)<img src=x onerror=alert(1)>' - create(:issue, project: project, title: alert_title) + it 'opens autocomplete menu for Issues when field starts with text with item escaping HTML characters' do + create(:issue, project: project, title: issue_xss_title) page.within '.timeline-content-form' do find('#note-body').native.send_keys('#') @@ -46,7 +51,19 @@ describe 'GFM autocomplete', :js do expect(page).to have_selector('.atwho-container') page.within '.atwho-container #at-view-issues' do - expect(page.all('li').first.text).to include(alert_title) + expect(page.all('li').first.text).to include(issue_xss_title) + end + end + + it 'opens autocomplete menu for Username when field starts with text with item escaping HTML characters' do + page.within '.timeline-content-form' do + find('#note-body').native.send_keys('@ev') + end + + expect(page).to have_selector('.atwho-container') + + page.within '.atwho-container #at-view-users' do + expect(find('li').text).to have_content(user_xss.username) end end @@ -107,7 +124,7 @@ describe 'GFM autocomplete', :js do wait_for_requests - expect(find('#at-view-64')).to have_selector('.cur:first-of-type') + expect(find('#at-view-users')).to have_selector('.cur:first-of-type') end it 'includes items for assignee dropdowns with non-ASCII characters in name' do @@ -120,7 +137,7 @@ describe 'GFM autocomplete', :js do wait_for_requests - expect(find('#at-view-64')).to have_content(user.name) + expect(find('#at-view-users')).to have_content(user.name) end it 'selects the first item for non-assignee dropdowns if a query is entered' do diff --git a/spec/features/issues/user_creates_branch_and_merge_request_spec.rb b/spec/features/issues/user_creates_branch_and_merge_request_spec.rb index 3dfcbc2fcb8..32bc851f00f 100644 --- a/spec/features/issues/user_creates_branch_and_merge_request_spec.rb +++ b/spec/features/issues/user_creates_branch_and_merge_request_spec.rb @@ -55,11 +55,11 @@ describe 'User creates branch and merge request on issue page', :js do test_branch_name_checking(input_branch_name) test_source_checking(input_source) - # The button inside dropdown should be disabled if any errors occured. + # The button inside dropdown should be disabled if any errors occurred. expect(page).to have_button('Create branch', disabled: true) end - # The top level button should be disabled if any errors occured. + # The top level button should be disabled if any errors occurred. expect(page).to have_button('Create branch', disabled: true) end @@ -76,7 +76,7 @@ describe 'User creates branch and merge request on issue page', :js do visit project_issue_path(project, issue) - expect(page).to have_content('created branch 1-cherry-coloured-funk') + expect(page).to have_content("created merge request !1 to address this issue") expect(page).to have_content('mentioned in merge request !1') end @@ -106,7 +106,7 @@ describe 'User creates branch and merge request on issue page', :js do visit project_issue_path(project, issue) - expect(page).to have_content('created branch custom-branch-name') + expect(page).to have_content("created merge request !1 to address this issue") expect(page).to have_content('mentioned in merge request !1') end diff --git a/spec/features/issues/user_sorts_issues_spec.rb b/spec/features/issues/user_sorts_issues_spec.rb index 7d261ec7dae..4771d2c6d28 100644 --- a/spec/features/issues/user_sorts_issues_spec.rb +++ b/spec/features/issues/user_sorts_issues_spec.rb @@ -26,7 +26,7 @@ describe "User sorts issues" do click_link('Milestone') end - visit(issues_dashboard_path(assignee_id: user.id)) + visit(issues_dashboard_path(assignee_username: user.username)) expect(find('.issues-filters a.is-active')).to have_content('Milestone') diff --git a/spec/features/issues/user_uses_quick_actions_spec.rb b/spec/features/issues/user_uses_quick_actions_spec.rb index 5926e442f24..27cffdc5f8b 100644 --- a/spec/features/issues/user_uses_quick_actions_spec.rb +++ b/spec/features/issues/user_uses_quick_actions_spec.rb @@ -303,5 +303,63 @@ describe 'Issues > User uses quick actions', :js do end end end + + describe 'create a merge request starting from an issue' do + let(:project) { create(:project, :public, :repository) } + let(:issue) { create(:issue, project: project) } + + def expect_mr_quickaction(success) + expect(page).to have_content 'Commands applied' + + if success + expect(page).to have_content 'created merge request' + else + expect(page).not_to have_content 'created merge request' + end + end + + it "doesn't create a merge request when the branch name is invalid" do + add_note("/create_merge_request invalid branch name") + + wait_for_requests + + expect_mr_quickaction(false) + end + + it "doesn't create a merge request when a branch with that name already exists" do + add_note("/create_merge_request feature") + + wait_for_requests + + expect_mr_quickaction(false) + end + + it 'creates a new merge request using issue iid and title as branch name when the branch name is empty' do + add_note("/create_merge_request") + + wait_for_requests + + expect_mr_quickaction(true) + + created_mr = project.merge_requests.last + expect(created_mr.source_branch).to eq(issue.to_branch_name) + + visit project_merge_request_path(project, created_mr) + expect(page).to have_content %{WIP: Resolve "#{issue.title}"} + end + + it 'creates a merge request using the given branch name' do + branch_name = '1-feature' + add_note("/create_merge_request #{branch_name}") + + expect_mr_quickaction(true) + + created_mr = project.merge_requests.last + expect(created_mr.source_branch).to eq(branch_name) + + visit project_merge_request_path(project, created_mr) + expect(page).to have_content %{WIP: Resolve "#{issue.title}"} + end + end end end diff --git a/spec/features/merge_requests/user_sorts_merge_requests_spec.rb b/spec/features/merge_requests/user_sorts_merge_requests_spec.rb index 82cfe600d52..e163868e8e7 100644 --- a/spec/features/merge_requests/user_sorts_merge_requests_spec.rb +++ b/spec/features/merge_requests/user_sorts_merge_requests_spec.rb @@ -25,7 +25,7 @@ describe 'User sorts merge requests' do click_link('Milestone') end - visit(merge_requests_dashboard_path(assignee_id: user.id)) + visit(merge_requests_dashboard_path(assignee_username: user.username)) expect(find('.issues-filters a.is-active')).to have_content('Milestone') @@ -41,7 +41,7 @@ describe 'User sorts merge requests' do it 'fallbacks to issuable_sort cookie key when remembering the sorting option' do set_cookie('issuable_sort', 'milestone') - visit(merge_requests_dashboard_path(assignee_id: user.id)) + visit(merge_requests_dashboard_path(assignee_username: user.username)) expect(find('.issues-filters a.is-active')).to have_content('Milestone') end diff --git a/spec/features/profiles/personal_access_tokens_spec.rb b/spec/features/profiles/personal_access_tokens_spec.rb index 8461cd0027c..dee213a11d4 100644 --- a/spec/features/profiles/personal_access_tokens_spec.rb +++ b/spec/features/profiles/personal_access_tokens_spec.rb @@ -43,10 +43,12 @@ describe 'Profile > Personal Access Tokens', :js do check "read_user" 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') expect(active_personal_access_tokens).to have_text('read_user') + expect(created_personal_access_token).not_to be_empty end context "when creation fails" do @@ -57,6 +59,7 @@ describe 'Profile > Personal Access Tokens', :js do expect { click_on "Create personal access token" }.not_to change { PersonalAccessToken.count } expect(page).to have_content("Name cannot be nil") + expect(page).not_to have_selector("#created-personal-access-token") end end end diff --git a/spec/features/projects/jobs_spec.rb b/spec/features/projects/jobs_spec.rb index a1323699969..99a7fbb63bd 100644 --- a/spec/features/projects/jobs_spec.rb +++ b/spec/features/projects/jobs_spec.rb @@ -719,7 +719,7 @@ describe 'Jobs', :clean_gitlab_redis_shared_state do context 'on mobile', :js do let(:job) { create(:ci_build, pipeline: pipeline) } - it 'renders collpased sidebar' do + it 'renders collapsed sidebar' do page.current_window.resize_to(600, 800) visit project_job_path(project, job) @@ -738,7 +738,7 @@ describe 'Jobs', :clean_gitlab_redis_shared_state do wait_for_requests expect(page).to have_css('.js-job-sidebar.right-sidebar-expanded') - expect(page).not_to have_css('.js-job-sidebar.right-sidebar-collpased') + expect(page).not_to have_css('.js-job-sidebar.right-sidebar-collapsed') end end diff --git a/spec/features/projects/settings/repository_settings_spec.rb b/spec/features/projects/settings/repository_settings_spec.rb index 377a75cbcb3..401aac9478d 100644 --- a/spec/features/projects/settings/repository_settings_spec.rb +++ b/spec/features/projects/settings/repository_settings_spec.rb @@ -132,6 +132,27 @@ describe 'Projects > Settings > Repository settings' do it 'shows push mirror settings', :js do expect(page).to have_selector('#mirror_direction') end + + it 'generates an SSH public key on submission', :js do + fill_in 'url', with: 'ssh://user@localhost/project.git' + select 'SSH public key', from: 'Authentication method' + + direction_select = find('#mirror_direction') + + # In CE, this select box is disabled, but in EE, it is enabled + if direction_select.disabled? + expect(direction_select.value).to eq('push') + else + direction_select.select('Push') + end + + Sidekiq::Testing.fake! do + click_button 'Mirror repository' + end + + expect(page).to have_content('Mirroring settings were successfully updated') + expect(page).to have_selector('[title="Copy SSH public key"]') + end end end end diff --git a/spec/features/search/user_uses_header_search_field_spec.rb b/spec/features/search/user_uses_header_search_field_spec.rb index af38f77c0c6..444de26733f 100644 --- a/spec/features/search/user_uses_header_search_field_spec.rb +++ b/spec/features/search/user_uses_header_search_field_spec.rb @@ -21,13 +21,17 @@ describe 'User uses header search field' do it 'shows assigned issues' do find('.search-input-container .dropdown-menu').click_link('Issues assigned to me') - expect(find('.js-assignee-search')).to have_content(user.name) + expect(page).to have_selector('.filtered-search') + expect_tokens([assignee_token(user.name)]) + expect_filtered_search_input_empty end it 'shows created issues' do find('.search-input-container .dropdown-menu').click_link("Issues I've created") - expect(find('.js-author-search')).to have_content(user.name) + expect(page).to have_selector('.filtered-search') + expect_tokens([author_token(user.name)]) + expect_filtered_search_input_empty end end @@ -37,13 +41,17 @@ describe 'User uses header search field' do it 'shows assigned merge requests' do find('.search-input-container .dropdown-menu').click_link('Merge requests assigned to me') - expect(find('.js-assignee-search')).to have_content(user.name) + expect(page).to have_selector('.filtered-search') + expect_tokens([assignee_token(user.name)]) + expect_filtered_search_input_empty end it 'shows created merge requests' do find('.search-input-container .dropdown-menu').click_link("Merge requests I've created") - expect(find('.js-author-search')).to have_content(user.name) + expect(page).to have_selector('.filtered-search') + expect_tokens([author_token(user.name)]) + expect_filtered_search_input_empty end end end diff --git a/spec/features/users/login_spec.rb b/spec/features/users/login_spec.rb index 44758f862a8..ad856bd062e 100644 --- a/spec/features/users/login_spec.rb +++ b/spec/features/users/login_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' describe 'Login' do include TermsHelper + include UserLoginHelper before do stub_authentication_activity_metrics(debug: true) @@ -546,29 +547,6 @@ describe 'Login' do ensure_tab_pane_correctness(false) end end - - def ensure_tab_pane_correctness(visit_path = true) - if visit_path - visit new_user_session_path - end - - ensure_tab_pane_counts - ensure_one_active_tab - ensure_one_active_pane - end - - def ensure_tab_pane_counts - tabs_count = page.all('[role="tab"]').size - expect(page).to have_selector('[role="tabpanel"]', count: tabs_count) - end - - def ensure_one_active_tab - expect(page).to have_selector('ul.new-session-tabs > li > a.active', count: 1) - end - - def ensure_one_active_pane - expect(page).to have_selector('.tab-pane.active', count: 1) - end end context 'when terms are enforced' do diff --git a/spec/features/users/overview_spec.rb b/spec/features/users/overview_spec.rb index b0ff53f9ccb..34ed771340f 100644 --- a/spec/features/users/overview_spec.rb +++ b/spec/features/users/overview_spec.rb @@ -54,15 +54,15 @@ describe 'Overview tab on a user profile', :js do end end - describe 'user has 10 activities' do + describe 'user has 11 activities' do before do - 10.times { push_code_contribution } + 11.times { push_code_contribution } end include_context 'visit overview tab' - it 'displays 5 entries in the list of activities' do - expect(find('#js-overview')).to have_selector('.event-item', count: 5) + it 'displays 10 entries in the list of activities' do + expect(find('#js-overview')).to have_selector('.event-item', count: 10) end it 'shows a link to the activity list' do diff --git a/spec/finders/group_descendants_finder_spec.rb b/spec/finders/group_descendants_finder_spec.rb index c64abdc3619..c28fd7cad11 100644 --- a/spec/finders/group_descendants_finder_spec.rb +++ b/spec/finders/group_descendants_finder_spec.rb @@ -74,6 +74,13 @@ describe GroupDescendantsFinder do end end + it 'sorts elements by latest created as default' do + project1 = create(:project, namespace: group, created_at: 1.hour.ago) + project2 = create(:project, namespace: group) + + expect(subject.execute).to eq([project2, project1]) + end + context 'sorting by name' do let!(:project1) { create(:project, namespace: group, name: 'a', path: 'project-a') } let!(:project2) { create(:project, namespace: group, name: 'z', path: 'project-z') } diff --git a/spec/finders/pending_todos_finder_spec.rb b/spec/finders/pending_todos_finder_spec.rb index 32fad5e225f..b41b1b46a93 100644 --- a/spec/finders/pending_todos_finder_spec.rb +++ b/spec/finders/pending_todos_finder_spec.rb @@ -46,7 +46,7 @@ describe PendingTodosFinder do create(:todo, :pending, user: user, target: note) - todos = described_class.new(user, target_type: issue.class).execute + todos = described_class.new(user, target_type: issue.class.name).execute expect(todos).to eq([todo]) end diff --git a/spec/finders/pipelines_finder_spec.rb b/spec/finders/pipelines_finder_spec.rb index c6e832ad69b..c2c304589c9 100644 --- a/spec/finders/pipelines_finder_spec.rb +++ b/spec/finders/pipelines_finder_spec.rb @@ -225,7 +225,7 @@ describe PipelinesFinder do end end - context 'when the project has limited access to piplines' do + context 'when the project has limited access to pipelines' do let(:project) { create(:project, :private, :repository) } let(:current_user) { create(:user) } let!(:pipelines) { create_list(:ci_pipeline, 2, project: project) } diff --git a/spec/fixtures/authentication/saml2_response.xml b/spec/fixtures/authentication/saml2_response.xml new file mode 100644 index 00000000000..67dea7209e9 --- /dev/null +++ b/spec/fixtures/authentication/saml2_response.xml @@ -0,0 +1,56 @@ +<?xml version="1.0" encoding="UTF-8"?> +<saml2p:Response xmlns:saml2p="urn:oasis:names:tc:SAML:2.0:protocol" xmlns:xs="http://www.w3.org/2001/XMLSchema" Destination="https://example.hello.com/access/saml" ID="jVFQbyEpSfUwqhZtJtarIaGoshwuAQMDwLoiMhzJXsv" InResponseTo="cfeooghajnhofcmogakmlhpkohnmikicnfhdnjlc" IssueInstant="2011-06-21T13:54:38.661Z" Version="2.0"> + <saml2:Issuer xmlns:saml2="urn:oasis:names:tc:SAML:2.0:assertion">https://idm.orademo.com</saml2:Issuer> + <ds:Signature xmlns:ds="http://www.w3.org/2000/09/xmldsig#"> + <ds:SignedInfo> + <ds:CanonicalizationMethod Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/> + <ds:SignatureMethod Algorithm="http://www.w3.org/2000/09/xmldsig#rsa-sha1"/> + <ds:Reference URI="#jVFQbyEpSfUwqhZtJtarIaGoshwuAQMDwLoiMhzJXsv"> + <ds:Transforms> + <ds:Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature"/> + <ds:Transform Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"> + <ec:InclusiveNamespaces xmlns:ec="http://www.w3.org/2001/10/xml-exc-c14n#" PrefixList="xs"/> + </ds:Transform> + </ds:Transforms> + <ds:DigestMethod Algorithm="http://www.w3.org/2000/09/xmldsig#sha1"/> + <ds:DigestValue>uHuSry39P16Yh7srS32xESmj4Lw=</ds:DigestValue> + </ds:Reference> + </ds:SignedInfo> + <ds:SignatureValue>fdghdfggfd=</ds:SignatureValue> + <ds:KeyInfo> + <ds:X509Data> + <ds:X509Certificate>dfghjkl</ds:X509Certificate> + </ds:X509Data> + </ds:KeyInfo> + </ds:Signature> + <saml2p:Status> + <saml2p:StatusCode Value="urn:oasis:names:tc:SAML:2.0:status:Success"/> + </saml2p:Status> + <saml2:Assertion xmlns:saml2="urn:oasis:names:tc:SAML:2.0:assertion" ID="emmCjammnYdAbMWDuMAJeZvQIMBayeeYqqwvQoDclKE" IssueInstant="2011-06-21T13:54:38.676Z" Version="2.0"> + <saml2:Issuer>https://idm.orademo.com</saml2:Issuer> + <saml2:Subject> + <saml2:NameID Format="urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress" NameQualifier="idp.example.org">someone@example.org</saml2:NameID> + <saml2:SubjectConfirmation Method="urn:oasis:names:tc:SAML:2.0:cm:bearer"> + <saml2:SubjectConfirmationData InResponseTo="cfeooghajnhofcmogakmlhpkohnmikicnfhdnjlc" NotOnOrAfter="2011-06-21T14:09:38.676Z" Recipient="https://example.hello.com/access/saml"/> + </saml2:SubjectConfirmation> + </saml2:Subject> + <saml2:Conditions NotBefore="2011-06-21T13:54:38.683Z" NotOnOrAfter="2011-06-21T14:09:38.683Z"> + <saml2:AudienceRestriction> + <saml2:Audience>hello.com</saml2:Audience> + </saml2:AudienceRestriction> + </saml2:Conditions> + <saml2:AuthnStatement AuthnInstant="2011-06-21T13:54:38.685Z" SessionIndex="perdkjfskdjfksdiertusfsdfsddeurtherukjdfgkdffg"> + <saml2:AuthnContext> + <saml2:AuthnContextClassRef>urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport</saml2:AuthnContextClassRef> + </saml2:AuthnContext> + </saml2:AuthnStatement> + <saml2:AttributeStatement> + <saml2:Attribute Name="FirstName"> + <saml2:AttributeValue xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string">Someone</saml2:AttributeValue> + </saml2:Attribute> + <saml2:Attribute Name="LastName"> + <saml2:AttributeValue xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string">Special</saml2:AttributeValue> + </saml2:Attribute> + </saml2:AttributeStatement> + </saml2:Assertion> +</saml2p:Response> diff --git a/spec/fixtures/trace/sample_trace b/spec/fixtures/trace/sample_trace index 7bfe3f83b7b..3d8beb0dec2 100644 --- a/spec/fixtures/trace/sample_trace +++ b/spec/fixtures/trace/sample_trace @@ -2334,12 +2334,12 @@ Boards::Lists::MoveService keeps position of lists when list type is closed when list type is set to label keeps position of lists when new position is nil - keeps position of lists when new positon is equal to old position - keeps position of lists when new positon is negative - keeps position of lists when new positon is equal to number of labels lists - keeps position of lists when new positon is greater than number of labels lists - increments position of intermediate lists when new positon is equal to first position - decrements position of intermediate lists when new positon is equal to last position + keeps position of lists when new position is equal to old position + keeps position of lists when new position is negative + keeps position of lists when new position is equal to number of labels lists + keeps position of lists when new position is greater than number of labels lists + increments position of intermediate lists when new position is equal to first position + decrements position of intermediate lists when new position is equal to last position decrements position of intermediate lists when new position is greater than old position increments position of intermediate lists when new position is lower than old position when board parent is a group @@ -2347,12 +2347,12 @@ Boards::Lists::MoveService keeps position of lists when list type is closed when list type is set to label keeps position of lists when new position is nil - keeps position of lists when new positon is equal to old position - keeps position of lists when new positon is negative - keeps position of lists when new positon is equal to number of labels lists - keeps position of lists when new positon is greater than number of labels lists - increments position of intermediate lists when new positon is equal to first position - decrements position of intermediate lists when new positon is equal to last position + keeps position of lists when new position is equal to old position + keeps position of lists when new position is negative + keeps position of lists when new position is equal to number of labels lists + keeps position of lists when new position is greater than number of labels lists + increments position of intermediate lists when new position is equal to first position + decrements position of intermediate lists when new position is equal to last position decrements position of intermediate lists when new position is greater than old position increments position of intermediate lists when new position is lower than old position diff --git a/spec/helpers/auth_helper_spec.rb b/spec/helpers/auth_helper_spec.rb index 120b23e66ac..f0c2e4768ec 100644 --- a/spec/helpers/auth_helper_spec.rb +++ b/spec/helpers/auth_helper_spec.rb @@ -42,6 +42,16 @@ describe AuthHelper do end end + describe 'form_based_auth_provider_has_active_class?' do + it 'selects main LDAP server' do + allow(helper).to receive(:auth_providers) { [:twitter, :ldapprimary, :ldapsecondary, :kerberos] } + expect(helper.form_based_auth_provider_has_active_class?(:twitter)).to be(false) + expect(helper.form_based_auth_provider_has_active_class?(:ldapprimary)).to be(true) + expect(helper.form_based_auth_provider_has_active_class?(:ldapsecondary)).to be(false) + expect(helper.form_based_auth_provider_has_active_class?(:kerberos)).to be(false) + end + end + describe 'enabled_button_based_providers' do before do allow(helper).to receive(:auth_providers) { [:twitter, :github] } diff --git a/spec/helpers/profiles_helper_spec.rb b/spec/helpers/profiles_helper_spec.rb index 9a2372de69f..8e336469c27 100644 --- a/spec/helpers/profiles_helper_spec.rb +++ b/spec/helpers/profiles_helper_spec.rb @@ -4,12 +4,17 @@ describe ProfilesHelper do describe '#commit_email_select_options' do it 'returns an array with private commit email along with all the verified emails' do user = create(:user) + create(:email, user: user) + confirmed_email1 = create(:email, :confirmed, user: user) + confirmed_email2 = create(:email, :confirmed, user: user) + private_email = user.private_commit_email - verified_emails = user.verified_emails - [private_email] emails = [ ["Use a private email - #{private_email}", Gitlab::PrivateCommitEmail::TOKEN], - verified_emails + user.email, + confirmed_email1.email, + confirmed_email2.email ] expect(helper.commit_email_select_options(user)).to match_array(emails) diff --git a/spec/helpers/search_helper_spec.rb b/spec/helpers/search_helper_spec.rb index 8bfd520528f..4945749f524 100644 --- a/spec/helpers/search_helper_spec.rb +++ b/spec/helpers/search_helper_spec.rb @@ -135,5 +135,40 @@ describe SearchHelper do expect(search_filter_input_options('')[:data]['base-endpoint']).to eq("/groups#{group_path(@group)}") end end + + context 'dashboard' do + it 'does not include group-id and project-id' do + expect(search_filter_input_options('')[:data]['project-id']).to eq(nil) + expect(search_filter_input_options('')[:data]['group-id']).to eq(nil) + end + + it 'includes dashboard base-endpoint' do + expect(search_filter_input_options('')[:data]['base-endpoint']).to eq("/dashboard") + end + end + end + + describe 'search_history_storage_prefix' do + context 'project' do + it 'returns project full_path' do + @project = create(:project, :repository) + + expect(search_history_storage_prefix).to eq(@project.full_path) + end + end + + context 'group' do + it 'returns group full_path' do + @group = create(:group, :nested, name: 'group-name') + + expect(search_history_storage_prefix).to eq(@group.full_path) + end + end + + context 'dashboard' do + it 'returns dashboard' do + expect(search_history_storage_prefix).to eq("dashboard") + end + end end end diff --git a/spec/javascripts/behaviors/shortcuts/shortcuts_issuable_spec.js b/spec/javascripts/behaviors/shortcuts/shortcuts_issuable_spec.js index bc25549cbed..b709b937180 100644 --- a/spec/javascripts/behaviors/shortcuts/shortcuts_issuable_spec.js +++ b/spec/javascripts/behaviors/shortcuts/shortcuts_issuable_spec.js @@ -1,3 +1,7 @@ +/* eslint-disable + no-underscore-dangle +*/ + import $ from 'jquery'; import initCopyAsGFM from '~/behaviors/markdown/copy_as_gfm'; import ShortcutsIssuable from '~/behaviors/shortcuts/shortcuts_issuable'; @@ -27,13 +31,17 @@ describe('ShortcutsIssuable', function() { describe('replyWithSelectedText', () => { // Stub window.gl.utils.getSelectedFragment to return a node with the provided HTML. - const stubSelection = html => { - window.gl.utils.getSelectedFragment = () => { + const stubSelection = (html, invalidNode) => { + ShortcutsIssuable.__Rewire__('getSelectedFragment', () => { + const documentFragment = document.createDocumentFragment(); const node = document.createElement('div'); + node.innerHTML = html; + if (!invalidNode) node.className = 'md'; - return node; - }; + documentFragment.appendChild(node); + return documentFragment; + }); }; describe('with empty selection', () => { it('does not return an error', () => { @@ -105,5 +113,133 @@ describe('ShortcutsIssuable', function() { ); }); }); + + describe('with an invalid selection', () => { + beforeEach(() => { + stubSelection('<p>Selected text.</p>', true); + }); + + it('does not add anything to the input', () => { + ShortcutsIssuable.replyWithSelectedText(true); + + expect($(FORM_SELECTOR).val()).toBe(''); + }); + + it('triggers `focus`', () => { + const spy = spyOn(document.querySelector(FORM_SELECTOR), 'focus'); + ShortcutsIssuable.replyWithSelectedText(true); + + expect(spy).toHaveBeenCalled(); + }); + }); + + describe('with a semi-valid selection', () => { + beforeEach(() => { + stubSelection('<div class="md">Selected text.</div><p>Invalid selected text.</p>', true); + }); + + it('only adds the valid part to the input', () => { + ShortcutsIssuable.replyWithSelectedText(true); + + expect($(FORM_SELECTOR).val()).toBe('> Selected text.\n\n'); + }); + + it('triggers `focus`', () => { + const spy = spyOn(document.querySelector(FORM_SELECTOR), 'focus'); + ShortcutsIssuable.replyWithSelectedText(true); + + expect(spy).toHaveBeenCalled(); + }); + + it('triggers `input`', () => { + let triggered = false; + $(FORM_SELECTOR).on('input', () => { + triggered = true; + }); + + ShortcutsIssuable.replyWithSelectedText(true); + + expect(triggered).toBe(true); + }); + }); + + describe('with a selection in a valid block', () => { + beforeEach(() => { + ShortcutsIssuable.__Rewire__('getSelectedFragment', () => { + const documentFragment = document.createDocumentFragment(); + const node = document.createElement('div'); + const originalNode = document.createElement('body'); + originalNode.innerHTML = `<div class="issue"> + <div class="otherElem">Text...</div> + <div class="md"><p><em>Selected text.</em></p></div> + </div>`; + documentFragment.originalNodes = [originalNode.querySelector('em')]; + + node.innerHTML = '<em>Selected text.</em>'; + + documentFragment.appendChild(node); + + return documentFragment; + }); + }); + + it('adds the quoted selection to the input', () => { + ShortcutsIssuable.replyWithSelectedText(true); + + expect($(FORM_SELECTOR).val()).toBe('> _Selected text._\n\n'); + }); + + it('triggers `focus`', () => { + const spy = spyOn(document.querySelector(FORM_SELECTOR), 'focus'); + ShortcutsIssuable.replyWithSelectedText(true); + + expect(spy).toHaveBeenCalled(); + }); + + it('triggers `input`', () => { + let triggered = false; + $(FORM_SELECTOR).on('input', () => { + triggered = true; + }); + + ShortcutsIssuable.replyWithSelectedText(true); + + expect(triggered).toBe(true); + }); + }); + + describe('with a selection in an invalid block', () => { + beforeEach(() => { + ShortcutsIssuable.__Rewire__('getSelectedFragment', () => { + const documentFragment = document.createDocumentFragment(); + const node = document.createElement('div'); + const originalNode = document.createElement('body'); + originalNode.innerHTML = `<div class="issue"> + <div class="otherElem"><div><b>Selected text.</b></div></div> + <div class="md"><p><em>Valid text</em></p></div> + </div>`; + documentFragment.originalNodes = [originalNode.querySelector('b')]; + + node.innerHTML = '<b>Selected text.</b>'; + + documentFragment.appendChild(node); + + return documentFragment; + }); + }); + + it('does not add anything to the input', () => { + ShortcutsIssuable.replyWithSelectedText(true); + + expect($(FORM_SELECTOR).val()).toBe(''); + }); + + it('triggers `focus`', () => { + const spy = spyOn(document.querySelector(FORM_SELECTOR), 'focus'); + ShortcutsIssuable.replyWithSelectedText(true); + + expect(spy).toHaveBeenCalled(); + }); + }); }); }); diff --git a/spec/javascripts/diffs/components/commit_item_spec.js b/spec/javascripts/diffs/components/commit_item_spec.js index 7606847ada9..8b2ca6506c4 100644 --- a/spec/javascripts/diffs/components/commit_item_spec.js +++ b/spec/javascripts/diffs/components/commit_item_spec.js @@ -21,7 +21,7 @@ const getAvatarElement = vm => vm.$el.querySelector('.user-avatar-link'); const getCommitterElement = vm => vm.$el.querySelector('.commiter'); const getCommitActionsElement = vm => vm.$el.querySelector('.commit-actions'); -describe('diffs/components/commit_widget', () => { +describe('diffs/components/commit_item', () => { const Component = Vue.extend(CommitItem); const timeago = getTimeago(); const { commit } = getDiffWithCommit(); @@ -37,15 +37,15 @@ describe('diffs/components/commit_widget', () => { it('renders commit title', () => { const titleElement = getTitleElement(vm); - expect(titleElement).toHaveAttr('href', commit.commitUrl); - expect(titleElement).toHaveText(commit.titleHtml); + expect(titleElement).toHaveAttr('href', commit.commit_url); + expect(titleElement).toHaveText(commit.title_html); }); it('renders commit description', () => { const descElement = getDescElement(vm); const descExpandElement = getDescExpandElement(vm); - const expected = commit.descriptionHtml.replace(/
/g, ''); + const expected = commit.description_html.replace(/
/g, ''); expect(trimText(descElement.innerHTML)).toEqual(trimText(expected)); expect(descExpandElement).not.toBeNull(); @@ -56,7 +56,7 @@ describe('diffs/components/commit_widget', () => { const labelElement = shaElement.querySelector('.label'); const buttonElement = shaElement.querySelector('button'); - expect(labelElement.textContent).toEqual(commit.shortId); + expect(labelElement.textContent).toEqual(commit.short_id); expect(buttonElement).toHaveData('clipboard-text', commit.id); }); @@ -64,27 +64,27 @@ describe('diffs/components/commit_widget', () => { const avatarElement = getAvatarElement(vm); const imgElement = avatarElement.querySelector('img'); - expect(avatarElement).toHaveAttr('href', commit.author.webUrl); + expect(avatarElement).toHaveAttr('href', commit.author.web_url); expect(imgElement).toHaveClass('s36'); expect(imgElement).toHaveAttr('alt', commit.author.name); - expect(imgElement).toHaveAttr('src', commit.author.avatarUrl); + expect(imgElement).toHaveAttr('src', commit.author.avatar_url); }); it('renders committer text', () => { const committerElement = getCommitterElement(vm); const nameElement = committerElement.querySelector('a'); - const expectTimeText = timeago.format(commit.authoredDate); + const expectTimeText = timeago.format(commit.authored_date); const expectedText = `${commit.author.name} authored ${expectTimeText}`; expect(trimText(committerElement.textContent)).toEqual(expectedText); - expect(nameElement).toHaveAttr('href', commit.author.webUrl); + expect(nameElement).toHaveAttr('href', commit.author.web_url); expect(nameElement).toHaveText(commit.author.name); }); describe('without commit description', () => { beforeEach(done => { - vm.commit.descriptionHtml = ''; + vm.commit.description_html = ''; vm.$nextTick() .then(done) @@ -103,9 +103,9 @@ describe('diffs/components/commit_widget', () => { describe('with no matching user', () => { beforeEach(done => { vm.commit.author = null; - vm.commit.authorEmail = TEST_AUTHOR_EMAIL; - vm.commit.authorName = TEST_AUTHOR_NAME; - vm.commit.authorGravatarUrl = TEST_AUTHOR_GRAVATAR; + vm.commit.author_email = TEST_AUTHOR_EMAIL; + vm.commit.author_name = TEST_AUTHOR_NAME; + vm.commit.author_gravatar_url = TEST_AUTHOR_GRAVATAR; vm.$nextTick() .then(done) @@ -132,7 +132,7 @@ describe('diffs/components/commit_widget', () => { describe('with signature', () => { beforeEach(done => { - vm.commit.signatureHtml = TEST_SIGNATURE_HTML; + vm.commit.signature_html = TEST_SIGNATURE_HTML; vm.$nextTick() .then(done) @@ -148,7 +148,7 @@ describe('diffs/components/commit_widget', () => { describe('with pipeline status', () => { beforeEach(done => { - vm.commit.pipelineStatusPath = TEST_PIPELINE_STATUS_PATH; + vm.commit.pipeline_status_path = TEST_PIPELINE_STATUS_PATH; vm.$nextTick() .then(done) diff --git a/spec/javascripts/diffs/components/commit_widget_spec.js b/spec/javascripts/diffs/components/commit_widget_spec.js index 951eb57255d..2b60bd232ed 100644 --- a/spec/javascripts/diffs/components/commit_widget_spec.js +++ b/spec/javascripts/diffs/components/commit_widget_spec.js @@ -19,6 +19,6 @@ describe('diffs/components/commit_widget', () => { const commitElement = vm.$el.querySelector('li.commit'); expect(commitElement).not.toBeNull(); - expect(commitElement).toContainText(commit.shortId); + expect(commitElement).toContainText(commit.short_id); }); }); diff --git a/spec/javascripts/diffs/components/diff_content_spec.js b/spec/javascripts/diffs/components/diff_content_spec.js index 36bd042f3c4..c25f6167163 100644 --- a/spec/javascripts/diffs/components/diff_content_spec.js +++ b/spec/javascripts/diffs/components/diff_content_spec.js @@ -56,14 +56,14 @@ describe('DiffContent', () => { describe('image diff', () => { beforeEach(done => { - vm.diffFile.newPath = GREEN_BOX_IMAGE_URL; - vm.diffFile.newSha = 'DEF'; - vm.diffFile.oldPath = RED_BOX_IMAGE_URL; - vm.diffFile.oldSha = 'ABC'; - vm.diffFile.viewPath = ''; + vm.diffFile.new_path = GREEN_BOX_IMAGE_URL; + vm.diffFile.new_sha = 'DEF'; + vm.diffFile.old_path = RED_BOX_IMAGE_URL; + vm.diffFile.old_sha = 'ABC'; + vm.diffFile.view_path = ''; vm.diffFile.discussions = [{ ...discussionsMockData }]; vm.$store.state.diffs.commentForms.push({ - fileHash: vm.diffFile.fileHash, + fileHash: vm.diffFile.file_hash, x: 10, y: 20, width: 100, @@ -113,10 +113,10 @@ describe('DiffContent', () => { describe('file diff', () => { it('should have download buttons in place', done => { const el = vm.$el; - vm.diffFile.newPath = 'test.abc'; - vm.diffFile.newSha = 'DEF'; - vm.diffFile.oldPath = 'test.abc'; - vm.diffFile.oldSha = 'ABC'; + vm.diffFile.new_path = 'test.abc'; + vm.diffFile.new_sha = 'DEF'; + vm.diffFile.old_path = 'test.abc'; + vm.diffFile.old_sha = 'ABC'; vm.$nextTick(() => { expect(el.querySelectorAll('.js-diff-inline-view').length).toEqual(0); diff --git a/spec/javascripts/diffs/components/diff_file_header_spec.js b/spec/javascripts/diffs/components/diff_file_header_spec.js index 0192d583c6c..9530b50c729 100644 --- a/spec/javascripts/diffs/components/diff_file_header_spec.js +++ b/spec/javascripts/diffs/components/diff_file_header_spec.js @@ -3,7 +3,6 @@ import Vuex from 'vuex'; import diffsModule from '~/diffs/store/modules'; import notesModule from '~/notes/stores/modules'; import DiffFileHeader from '~/diffs/components/diff_file_header.vue'; -import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils'; import { mountComponentWithStore } from 'spec/helpers/vue_mount_component_helper'; Vue.use(Vuex); @@ -24,9 +23,9 @@ describe('diff_file_header', () => { }); beforeEach(() => { - const diffFile = convertObjectPropsToCamelCase(diffDiscussionMock.diff_file, { deep: true }); + const diffFile = diffDiscussionMock.diff_file; props = { - diffFile, + diffFile: { ...diffFile }, canCurrentUserFork: false, }; }); @@ -62,8 +61,8 @@ describe('diff_file_header', () => { beforeEach(() => { props.discussionPath = 'link://to/discussion'; Object.assign(props.diffFile, { - submoduleLink: 'link://to/submodule', - submoduleTreeUrl: 'some://tree/url', + submodule_link: 'link://to/submodule', + submodule_tree_url: 'some://tree/url', }); }); @@ -80,18 +79,18 @@ describe('diff_file_header', () => { vm = mountComponentWithStore(Component, { props, store }); - expect(vm.titleLink).toBe(props.diffFile.submoduleTreeUrl); + expect(vm.titleLink).toBe(props.diffFile.submodule_tree_url); }); it('returns the submoduleLink for submodules without submoduleTreeUrl', () => { Object.assign(props.diffFile, { submodule: true, - submoduleTreeUrl: null, + submodule_tree_url: null, }); vm = mountComponentWithStore(Component, { props, store }); - expect(vm.titleLink).toBe(props.diffFile.submoduleLink); + expect(vm.titleLink).toBe(props.diffFile.submodule_link); }); it('sets the correct path to the discussion', () => { @@ -107,7 +106,7 @@ describe('diff_file_header', () => { beforeEach(() => { Object.assign(props.diffFile, { blob: { id: 'b10b1db10b1d' }, - filePath: 'path/to/file', + file_path: 'path/to/file', }); }); @@ -116,7 +115,7 @@ describe('diff_file_header', () => { vm = mountComponentWithStore(Component, { props, store }); - expect(vm.filePath).toBe(props.diffFile.filePath); + expect(vm.filePath).toBe(props.diffFile.file_path); }); it('appends the truncated blob id for submodules', () => { @@ -125,14 +124,14 @@ describe('diff_file_header', () => { vm = mountComponentWithStore(Component, { props, store }); expect(vm.filePath).toBe( - `${props.diffFile.filePath} @ ${props.diffFile.blob.id.substr(0, 8)}`, + `${props.diffFile.file_path} @ ${props.diffFile.blob.id.substr(0, 8)}`, ); }); }); describe('titleTag', () => { it('returns a link tag if fileHash is set', () => { - props.diffFile.fileHash = 'some hash'; + props.diffFile.file_hash = 'some hash'; vm = mountComponentWithStore(Component, { props, store }); @@ -140,7 +139,7 @@ describe('diff_file_header', () => { }); it('returns a span tag if fileHash is not set', () => { - props.diffFile.fileHash = null; + props.diffFile.file_hash = null; vm = mountComponentWithStore(Component, { props, store }); @@ -151,8 +150,8 @@ describe('diff_file_header', () => { describe('isUsingLfs', () => { beforeEach(() => { Object.assign(props.diffFile, { - storedExternally: true, - externalStorage: 'lfs', + stored_externally: true, + external_storage: 'lfs', }); }); @@ -163,7 +162,7 @@ describe('diff_file_header', () => { }); it('returns false if file is not stored externally', () => { - props.diffFile.storedExternally = false; + props.diffFile.stored_externally = false; vm = mountComponentWithStore(Component, { props, store }); @@ -171,7 +170,7 @@ describe('diff_file_header', () => { }); it('returns false if file is not stored in LFS', () => { - props.diffFile.externalStorage = 'not lfs'; + props.diffFile.external_storage = 'not lfs'; vm = mountComponentWithStore(Component, { props, store }); @@ -200,7 +199,7 @@ describe('diff_file_header', () => { describe('viewFileButtonText', () => { it('contains the truncated content SHA', () => { const dummySha = 'deebd00f is no SHA'; - props.diffFile.contentSha = dummySha; + props.diffFile.content_sha = dummySha; vm = mountComponentWithStore(Component, { props, store }); @@ -212,7 +211,7 @@ describe('diff_file_header', () => { describe('viewReplacedFileButtonText', () => { it('contains the truncated base SHA', () => { const dummySha = 'deadabba sings no more'; - props.diffFile.diffRefs.baseSha = dummySha; + props.diffFile.diff_refs.base_sha = dummySha; vm = mountComponentWithStore(Component, { props, store }); @@ -281,32 +280,32 @@ describe('diff_file_header', () => { const filePaths = () => vm.$el.querySelectorAll('.file-title-name'); it('displays the path of a added file', () => { - props.diffFile.renamedFile = false; + props.diffFile.renamed_file = false; vm = mountComponentWithStore(Component, { props, store }); expect(filePaths()).toHaveLength(1); - expect(filePaths()[0]).toHaveText(props.diffFile.filePath); + expect(filePaths()[0]).toHaveText(props.diffFile.file_path); }); it('displays path for deleted file', () => { - props.diffFile.renamedFile = false; - props.diffFile.deletedFile = true; + props.diffFile.renamed_file = false; + props.diffFile.deleted_file = true; vm = mountComponentWithStore(Component, { props, store }); expect(filePaths()).toHaveLength(1); - expect(filePaths()[0]).toHaveText(`${props.diffFile.filePath} deleted`); + expect(filePaths()[0]).toHaveText(`${props.diffFile.file_path} deleted`); }); it('displays old and new path if the file was renamed', () => { - props.diffFile.renamedFile = true; + props.diffFile.renamed_file = true; vm = mountComponentWithStore(Component, { props, store }); expect(filePaths()).toHaveLength(2); - expect(filePaths()[0]).toHaveText(props.diffFile.oldPath); - expect(filePaths()[1]).toHaveText(props.diffFile.newPath); + expect(filePaths()[0]).toHaveText(props.diffFile.old_path); + expect(filePaths()[1]).toHaveText(props.diffFile.new_path); }); }); @@ -323,19 +322,19 @@ describe('diff_file_header', () => { describe('file mode', () => { it('it displays old and new file mode if it changed', () => { - props.diffFile.modeChanged = true; + props.diffFile.mode_changed = true; vm = mountComponentWithStore(Component, { props, store }); const { fileMode } = vm.$refs; expect(fileMode).not.toBe(undefined); - expect(fileMode).toContainText(props.diffFile.aMode); - expect(fileMode).toContainText(props.diffFile.bMode); + expect(fileMode).toContainText(props.diffFile.a_mode); + expect(fileMode).toContainText(props.diffFile.b_mode); }); it('does not display the file mode if it has not changed', () => { - props.diffFile.modeChanged = false; + props.diffFile.mode_changed = false; vm = mountComponentWithStore(Component, { props, store }); @@ -350,8 +349,8 @@ describe('diff_file_header', () => { it('displays the LFS label for files stored in LFS', () => { Object.assign(props.diffFile, { - storedExternally: true, - externalStorage: 'lfs', + stored_externally: true, + external_storage: 'lfs', }); vm = mountComponentWithStore(Component, { props, store }); @@ -361,7 +360,7 @@ describe('diff_file_header', () => { }); it('does not display the LFS label for files stored in repository', () => { - props.diffFile.storedExternally = false; + props.diffFile.stored_externally = false; vm = mountComponentWithStore(Component, { props, store }); @@ -378,7 +377,7 @@ describe('diff_file_header', () => { it('should show edit button when file is editable', () => { props.addMergeRequestButtons = true; - props.diffFile.editPath = '/'; + props.diffFile.edit_path = '/'; vm = mountComponentWithStore(Component, { props, store }); expect(vm.$el.querySelector('.js-edit-blob')).toContainText('Edit'); @@ -386,8 +385,8 @@ describe('diff_file_header', () => { it('should not show edit button when file is deleted', () => { props.addMergeRequestButtons = true; - props.diffFile.deletedFile = true; - props.diffFile.editPath = '/'; + props.diffFile.deleted_file = true; + props.diffFile.edit_path = '/'; vm = mountComponentWithStore(Component, { props, store }); expect(vm.$el.querySelector('.js-edit-blob')).toEqual(null); @@ -397,7 +396,7 @@ describe('diff_file_header', () => { describe('addMergeRequestButtons', () => { beforeEach(() => { props.addMergeRequestButtons = true; - props.diffFile.editPath = ''; + props.diffFile.edit_path = ''; }); describe('view on environment button', () => { @@ -405,8 +404,8 @@ describe('diff_file_header', () => { const title = 'url.title'; it('displays link to external url', () => { - props.diffFile.externalUrl = url; - props.diffFile.formattedExternalUrl = title; + props.diffFile.external_url = url; + props.diffFile.formatted_external_url = title; vm = mountComponentWithStore(Component, { props, store }); @@ -415,8 +414,8 @@ describe('diff_file_header', () => { }); it('hides link if no external url', () => { - props.diffFile.externalUrl = ''; - props.diffFile.formattedExternalUrl = title; + props.diffFile.external_url = ''; + props.diffFile.formattedExternal_url = title; vm = mountComponentWithStore(Component, { props, store }); @@ -434,11 +433,11 @@ describe('diff_file_header', () => { path: 'lib/base.js', name: 'base.js', mode: '100644', - readableText: true, + readable_text: true, icon: 'file-text-o', }; propsCopy.addMergeRequestButtons = true; - propsCopy.diffFile.deletedFile = true; + propsCopy.diffFile.deleted_file = true; vm = mountComponentWithStore(Component, { props: propsCopy, @@ -459,11 +458,11 @@ describe('diff_file_header', () => { path: 'lib/base.js', name: 'base.js', mode: '100644', - readableText: true, + readable_text: true, icon: 'file-text-o', }; propsCopy.addMergeRequestButtons = true; - propsCopy.diffFile.deletedFile = true; + propsCopy.diffFile.deleted_file = true; const discussionGetter = () => [diffDiscussionMock]; const notesModuleMock = notesModule(); diff --git a/spec/javascripts/diffs/components/diff_file_spec.js b/spec/javascripts/diffs/components/diff_file_spec.js index 882ad3685a2..51bb4807960 100644 --- a/spec/javascripts/diffs/components/diff_file_spec.js +++ b/spec/javascripts/diffs/components/diff_file_spec.js @@ -17,14 +17,14 @@ describe('DiffFile', () => { describe('template', () => { it('should render component with file header, file content components', () => { const el = vm.$el; - const { fileHash, filePath } = vm.file; + const { file_hash, file_path } = vm.file; - expect(el.id).toEqual(fileHash); + expect(el.id).toEqual(file_hash); expect(el.classList.contains('diff-file')).toEqual(true); expect(el.querySelectorAll('.diff-content.hidden').length).toEqual(0); expect(el.querySelector('.js-file-title')).toBeDefined(); - expect(el.querySelector('.file-title-name').innerText.indexOf(filePath)).toBeGreaterThan(-1); + expect(el.querySelector('.file-title-name').innerText.indexOf(file_path)).toBeGreaterThan(-1); expect(el.querySelector('.js-syntax-highlight')).toBeDefined(); expect(vm.file.renderIt).toEqual(false); @@ -52,7 +52,7 @@ describe('DiffFile', () => { it('should have collapsed text and link', done => { vm.file.renderIt = true; vm.file.collapsed = false; - vm.file.highlightedDiffLines = null; + vm.file.highlighted_diff_lines = null; vm.$nextTick(() => { expect(vm.$el.innerText).toContain('This diff is collapsed'); @@ -90,8 +90,8 @@ describe('DiffFile', () => { describe('too large diff', () => { it('should have too large warning and blob link', done => { const BLOB_LINK = '/file/view/path'; - vm.file.tooLarge = true; - vm.file.viewPath = BLOB_LINK; + vm.file.too_large = true; + vm.file.view_path = BLOB_LINK; vm.$nextTick(() => { expect(vm.$el.innerText).toContain( @@ -107,4 +107,26 @@ describe('DiffFile', () => { }); }); }); + + describe('watch collapsed', () => { + it('calls handleLoadCollapsedDiff if collapsed changed & file has no lines', done => { + spyOn(vm, 'handleLoadCollapsedDiff'); + + vm.file.highlighted_diff_lines = undefined; + vm.file.parallel_diff_lines = []; + vm.file.collapsed = true; + + vm.$nextTick() + .then(() => { + vm.file.collapsed = false; + + return vm.$nextTick(); + }) + .then(() => { + expect(vm.handleLoadCollapsedDiff).toHaveBeenCalled(); + }) + .then(done) + .catch(done.fail); + }); + }); }); diff --git a/spec/javascripts/diffs/components/diff_line_gutter_content_spec.js b/spec/javascripts/diffs/components/diff_line_gutter_content_spec.js index 6972e0ee913..038db8eaa7c 100644 --- a/spec/javascripts/diffs/components/diff_line_gutter_content_spec.js +++ b/spec/javascripts/diffs/components/diff_line_gutter_content_spec.js @@ -11,16 +11,16 @@ describe('DiffLineGutterContent', () => { const cmp = Vue.extend(DiffLineGutterContent); const props = Object.assign({}, options); props.line = { - lineCode: 'LC_42', + line_code: 'LC_42', type: 'new', - oldLine: null, - newLine: 1, - discussions: [], + old_line: null, + new_line: 1, + discussions: [{ ...discussionsMockData }], text: '+<span id="LC1" class="line" lang="plaintext"> - Bad dates</span>\n', - richText: '+<span id="LC1" class="line" lang="plaintext"> - Bad dates</span>\n', - metaData: null, + rich_text: '+<span id="LC1" class="line" lang="plaintext"> - Bad dates</span>\n', + meta_data: null, }; - props.fileHash = getDiffFileMock().fileHash; + props.fileHash = getDiffFileMock().file_hash; props.contextLinesPath = '/context/lines/path'; return createComponentWithStore(cmp, store, props).$mount(); @@ -37,7 +37,7 @@ describe('DiffLineGutterContent', () => { it('should return # if there is no lineCode', () => { const component = createComponent(); - component.line.lineCode = ''; + component.line.line_code = ''; expect(component.lineHref).toEqual('#'); }); @@ -46,6 +46,7 @@ describe('DiffLineGutterContent', () => { describe('discussions, hasDiscussions, shouldShowAvatarsOnGutter', () => { it('should return empty array when there is no discussion', () => { const component = createComponent(); + component.line.discussions = []; expect(component.hasDiscussions).toEqual(false); expect(component.shouldShowAvatarsOnGutter).toEqual(false); @@ -54,8 +55,8 @@ describe('DiffLineGutterContent', () => { it('should return discussions for the given lineCode', () => { const cmp = Vue.extend(DiffLineGutterContent); const props = { - line: getDiffFileMock().highlightedDiffLines[1], - fileHash: getDiffFileMock().fileHash, + line: getDiffFileMock().highlighted_diff_lines[1], + fileHash: getDiffFileMock().file_hash, showCommentButton: true, contextLinesPath: '/context/lines/path', }; @@ -104,10 +105,10 @@ describe('DiffLineGutterContent', () => { it('should render user avatars', () => { const component = createComponent({ showCommentButton: true, - lineCode: getDiffFileMock().highlightedDiffLines[1].lineCode, + lineCode: getDiffFileMock().highlighted_diff_lines[1].line_code, }); - expect(component.$el.querySelector('.diff-comment-avatar-holders')).toBeDefined(); + expect(component.$el.querySelector('.diff-comment-avatar-holders')).not.toBe(null); }); }); }); diff --git a/spec/javascripts/diffs/components/diff_line_note_form_spec.js b/spec/javascripts/diffs/components/diff_line_note_form_spec.js index c39b54d9cc9..81b66cf7c9b 100644 --- a/spec/javascripts/diffs/components/diff_line_note_form_spec.js +++ b/spec/javascripts/diffs/components/diff_line_note_form_spec.js @@ -13,10 +13,10 @@ describe('DiffLineNoteForm', () => { beforeEach(() => { diffFile = getDiffFileMock(); - diffLines = diffFile.highlightedDiffLines; + diffLines = diffFile.highlighted_diff_lines; component = createComponentWithStore(Vue.extend(DiffLineNoteForm), store, { - diffFileHash: diffFile.fileHash, + diffFileHash: diffFile.file_hash, diffLines, line: diffLines[0], noteTargetLine: diffLines[0], @@ -61,7 +61,7 @@ describe('DiffLineNoteForm', () => { expect(window.confirm).not.toHaveBeenCalled(); component.$nextTick(() => { expect(component.cancelCommentForm).toHaveBeenCalledWith({ - lineCode: diffLines[0].lineCode, + lineCode: diffLines[0].line_code, }); expect(component.resetAutoSave).toHaveBeenCalled(); diff --git a/spec/javascripts/diffs/components/inline_diff_view_spec.js b/spec/javascripts/diffs/components/inline_diff_view_spec.js index 705558e860b..2316ee29106 100644 --- a/spec/javascripts/diffs/components/inline_diff_view_spec.js +++ b/spec/javascripts/diffs/components/inline_diff_view_spec.js @@ -1,4 +1,5 @@ import Vue from 'vue'; +import '~/behaviors/markdown/render_gfm'; import InlineDiffView from '~/diffs/components/inline_diff_view.vue'; import store from '~/mr_notes/stores'; import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper'; @@ -10,14 +11,16 @@ describe('InlineDiffView', () => { const getDiffFileMock = () => Object.assign({}, diffFileMockData); const getDiscussionsMockData = () => [Object.assign({}, discussionsMockData)]; - beforeEach(() => { + beforeEach(done => { const diffFile = getDiffFileMock(); store.dispatch('diffs/setInlineDiffViewType'); component = createComponentWithStore(Vue.extend(InlineDiffView), store, { diffFile, - diffLines: diffFile.highlightedDiffLines, + diffLines: diffFile.highlighted_diff_lines, }).$mount(); + + Vue.nextTick(done); }); describe('template', () => { @@ -32,7 +35,7 @@ describe('InlineDiffView', () => { it('should render discussions', done => { const el = component.$el; - component.$store.dispatch('setInitialNotes', getDiscussionsMockData()); + component.diffLines[1].discussions = getDiscussionsMockData(); Vue.nextTick(() => { expect(el.querySelectorAll('.notes_holder').length).toEqual(1); diff --git a/spec/javascripts/diffs/components/parallel_diff_view_spec.js b/spec/javascripts/diffs/components/parallel_diff_view_spec.js index 091e01868d3..6f6b1c41915 100644 --- a/spec/javascripts/diffs/components/parallel_diff_view_spec.js +++ b/spec/javascripts/diffs/components/parallel_diff_view_spec.js @@ -14,7 +14,7 @@ describe('ParallelDiffView', () => { component = createComponentWithStore(Vue.extend(ParallelDiffView), store, { diffFile, - diffLines: diffFile.parallelDiffLines, + diffLines: diffFile.parallel_diff_lines, }).$mount(); }); diff --git a/spec/javascripts/diffs/mock_data/diff_file.js b/spec/javascripts/diffs/mock_data/diff_file.js index be194ab414f..031c9842f2f 100644 --- a/spec/javascripts/diffs/mock_data/diff_file.js +++ b/spec/javascripts/diffs/mock_data/diff_file.js @@ -1,130 +1,130 @@ export default { submodule: false, - submoduleLink: null, + submodule_link: null, blob: { id: '9e10516ca50788acf18c518a231914a21e5f16f7', path: 'CHANGELOG', name: 'CHANGELOG', mode: '100644', - readableText: true, + readable_text: true, icon: 'file-text-o', }, - blobPath: 'CHANGELOG', - blobName: 'CHANGELOG', - blobIcon: '<i aria-hidden="true" data-hidden="true" class="fa fa-file-text-o fa-fw"></i>', - fileHash: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a', - filePath: 'CHANGELOG', - newFile: false, - deletedFile: false, - renamedFile: false, - oldPath: 'CHANGELOG', - newPath: 'CHANGELOG', - modeChanged: false, - aMode: '100644', - bMode: '100644', + blob_path: 'CHANGELOG', + blob_name: 'CHANGELOG', + blob_icon: '<i aria-hidden="true" data-hidden="true" class="fa fa-file-text-o fa-fw"></i>', + file_hash: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a', + file_path: 'CHANGELOG', + new_file: false, + deleted_file: false, + renamed_file: false, + old_path: 'CHANGELOG', + new_path: 'CHANGELOG', + mode_changed: false, + a_mode: '100644', + b_mode: '100644', text: true, viewer: { name: 'text', }, - addedLines: 2, - removedLines: 0, - diffRefs: { - baseSha: 'e63f41fe459e62e1228fcef60d7189127aeba95a', - startSha: 'd9eaefe5a676b820c57ff18cf5b68316025f7962', - headSha: 'c48ee0d1bf3b30453f5b32250ce03134beaa6d13', + added_lines: 2, + removed_lines: 0, + diff_refs: { + base_sha: 'e63f41fe459e62e1228fcef60d7189127aeba95a', + start_sha: 'd9eaefe5a676b820c57ff18cf5b68316025f7962', + head_sha: 'c48ee0d1bf3b30453f5b32250ce03134beaa6d13', }, - contentSha: 'c48ee0d1bf3b30453f5b32250ce03134beaa6d13', - storedExternally: null, - externalStorage: null, - oldPathHtml: 'CHANGELOG', - newPathHtml: 'CHANGELOG', - editPath: '/gitlab-org/gitlab-test/edit/spooky-stuff/CHANGELOG', - viewPath: '/gitlab-org/gitlab-test/blob/spooky-stuff/CHANGELOG', - replacedViewPath: null, + content_sha: 'c48ee0d1bf3b30453f5b32250ce03134beaa6d13', + stored_externally: null, + external_storage: null, + old_path_html: 'CHANGELOG', + new_path_html: 'CHANGELOG', + edit_path: '/gitlab-org/gitlab-test/edit/spooky-stuff/CHANGELOG', + view_path: '/gitlab-org/gitlab-test/blob/spooky-stuff/CHANGELOG', + replaced_view_path: null, collapsed: false, renderIt: false, - tooLarge: false, - contextLinesPath: + too_large: false, + context_lines_path: '/gitlab-org/gitlab-test/blob/c48ee0d1bf3b30453f5b32250ce03134beaa6d13/CHANGELOG/diff', - highlightedDiffLines: [ + highlighted_diff_lines: [ { - lineCode: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_1', + line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_1', type: 'new', - oldLine: null, - newLine: 1, + old_line: null, + new_line: 1, discussions: [], text: '+<span id="LC1" class="line" lang="plaintext"> - Bad dates</span>\n', - richText: '+<span id="LC1" class="line" lang="plaintext"> - Bad dates</span>\n', - metaData: null, + rich_text: '+<span id="LC1" class="line" lang="plaintext"> - Bad dates</span>\n', + meta_data: null, }, { - lineCode: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_2', + line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_2', type: 'new', - oldLine: null, - newLine: 2, + old_line: null, + new_line: 2, discussions: [], text: '+<span id="LC2" class="line" lang="plaintext"></span>\n', - richText: '+<span id="LC2" class="line" lang="plaintext"></span>\n', - metaData: null, + rich_text: '+<span id="LC2" class="line" lang="plaintext"></span>\n', + meta_data: null, }, { - lineCode: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_3', + line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_3', type: null, - oldLine: 1, - newLine: 3, + old_line: 1, + new_line: 3, discussions: [], text: ' <span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n', - richText: ' <span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n', - metaData: null, + rich_text: ' <span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n', + meta_data: null, }, { - lineCode: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_2_4', + line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_2_4', type: null, - oldLine: 2, - newLine: 4, + old_line: 2, + new_line: 4, discussions: [], text: ' <span id="LC4" class="line" lang="plaintext"></span>\n', - richText: ' <span id="LC4" class="line" lang="plaintext"></span>\n', - metaData: null, + rich_text: ' <span id="LC4" class="line" lang="plaintext"></span>\n', + meta_data: null, }, { - lineCode: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_3_5', + line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_3_5', type: null, - oldLine: 3, - newLine: 5, + old_line: 3, + new_line: 5, discussions: [], text: ' <span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n', - richText: ' <span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n', - metaData: null, + rich_text: ' <span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n', + meta_data: null, }, { - lineCode: null, + line_code: null, type: 'match', - oldLine: null, - newLine: null, + old_line: null, + new_line: null, discussions: [], text: '', - richText: '', - metaData: { - oldPos: 3, - newPos: 5, + rich_text: '', + meta_data: { + old_pos: 3, + new_pos: 5, }, }, ], - parallelDiffLines: [ + parallel_diff_lines: [ { left: { type: 'empty-cell', }, right: { - lineCode: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_1', + line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_1', type: 'new', - oldLine: null, - newLine: 1, + old_line: null, + new_line: 1, discussions: [], text: '+<span id="LC1" class="line" lang="plaintext"> - Bad dates</span>\n', - richText: '<span id="LC1" class="line" lang="plaintext"> - Bad dates</span>\n', - metaData: null, + rich_text: '<span id="LC1" class="line" lang="plaintext"> - Bad dates</span>\n', + meta_data: null, }, }, { @@ -132,107 +132,107 @@ export default { type: 'empty-cell', }, right: { - lineCode: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_2', + line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_2', type: 'new', - oldLine: null, - newLine: 2, + old_line: null, + new_line: 2, discussions: [], text: '+<span id="LC2" class="line" lang="plaintext"></span>\n', - richText: '<span id="LC2" class="line" lang="plaintext"></span>\n', - metaData: null, + rich_text: '<span id="LC2" class="line" lang="plaintext"></span>\n', + meta_data: null, }, }, { left: { - lineCode: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_3', + line_Code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_3', type: null, - oldLine: 1, - newLine: 3, + old_line: 1, + new_line: 3, discussions: [], text: ' <span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n', - richText: '<span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n', - metaData: null, + rich_text: '<span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n', + meta_data: null, }, right: { - lineCode: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_3', + line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_3', type: null, - oldLine: 1, - newLine: 3, + old_line: 1, + new_line: 3, discussions: [], text: ' <span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n', - richText: '<span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n', - metaData: null, + rich_text: '<span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n', + meta_data: null, }, }, { left: { - lineCode: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_2_4', + line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_2_4', type: null, - oldLine: 2, - newLine: 4, + old_line: 2, + new_line: 4, discussions: [], text: ' <span id="LC4" class="line" lang="plaintext"></span>\n', - richText: '<span id="LC4" class="line" lang="plaintext"></span>\n', - metaData: null, + rich_text: '<span id="LC4" class="line" lang="plaintext"></span>\n', + meta_data: null, }, right: { - lineCode: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_2_4', + line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_2_4', type: null, - oldLine: 2, - newLine: 4, + old_line: 2, + new_line: 4, discussions: [], text: ' <span id="LC4" class="line" lang="plaintext"></span>\n', - richText: '<span id="LC4" class="line" lang="plaintext"></span>\n', - metaData: null, + rich_text: '<span id="LC4" class="line" lang="plaintext"></span>\n', + meta_data: null, }, }, { left: { - lineCode: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_3_5', + line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_3_5', type: null, - oldLine: 3, - newLine: 5, + old_line: 3, + new_line: 5, discussions: [], text: ' <span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n', - richText: '<span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n', - metaData: null, + rich_text: '<span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n', + meta_data: null, }, right: { - lineCode: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_3_5', + line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_3_5', type: null, - oldLine: 3, - newLine: 5, + old_line: 3, + new_line: 5, discussions: [], text: ' <span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n', - richText: '<span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n', - metaData: null, + rich_text: '<span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n', + meta_data: null, }, }, { left: { - lineCode: null, + line_code: null, type: 'match', - oldLine: null, - newLine: null, + old_line: null, + new_line: null, discussions: [], text: '', - richText: '', - metaData: { - oldPos: 3, - newPos: 5, + rich_text: '', + meta_data: { + old_pos: 3, + new_pos: 5, }, }, right: { - lineCode: null, + line_code: null, type: 'match', - oldLine: null, - newLine: null, + old_line: null, + new_line: null, discussions: [], text: '', - richText: '', - metaData: { - oldPos: 3, - newPos: 5, + rich_text: '', + meta_data: { + old_pos: 3, + new_pos: 5, }, }, }, diff --git a/spec/javascripts/diffs/mock_data/diff_with_commit.js b/spec/javascripts/diffs/mock_data/diff_with_commit.js index bee04fa4932..d646294ee84 100644 --- a/spec/javascripts/diffs/mock_data/diff_with_commit.js +++ b/spec/javascripts/diffs/mock_data/diff_with_commit.js @@ -1,9 +1,7 @@ -import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils'; - const FIXTURE = 'merge_request_diffs/with_commit.json'; preloadFixtures(FIXTURE); export default function getDiffWithCommit() { - return convertObjectPropsToCamelCase(getJSONFixture(FIXTURE), { deep: true }); + return getJSONFixture(FIXTURE); } diff --git a/spec/javascripts/diffs/store/actions_spec.js b/spec/javascripts/diffs/store/actions_spec.js index 17d0f31bdd3..acd95a3dd8b 100644 --- a/spec/javascripts/diffs/store/actions_spec.js +++ b/spec/javascripts/diffs/store/actions_spec.js @@ -97,46 +97,46 @@ describe('DiffsStoreActions', () => { const state = { diffFiles: [ { - fileHash: 'ABC', - parallelDiffLines: [ + file_hash: 'ABC', + parallel_diff_lines: [ { left: { - lineCode: 'ABC_1_1', + line_code: 'ABC_1_1', discussions: [], }, right: { - lineCode: 'ABC_1_1', + line_code: 'ABC_1_1', discussions: [], }, }, ], - highlightedDiffLines: [ + highlighted_diff_lines: [ { - lineCode: 'ABC_1_1', + line_code: 'ABC_1_1', discussions: [], - oldLine: 5, - newLine: null, + old_line: 5, + new_line: null, }, ], - diffRefs: { - baseSha: 'abc', - headSha: 'def', - startSha: 'ghi', + diff_refs: { + base_sha: 'abc', + head_sha: 'def', + start_sha: 'ghi', }, - newPath: 'file1', - oldPath: 'file2', + new_path: 'file1', + old_path: 'file2', }, ], }; const diffPosition = { - baseSha: 'abc', - headSha: 'def', - startSha: 'ghi', - newLine: null, - newPath: 'file1', - oldLine: 5, - oldPath: 'file2', + base_sha: 'abc', + head_sha: 'def', + start_sha: 'ghi', + new_line: null, + new_path: 'file1', + old_line: 5, + old_path: 'file2', }; const singleDiscussion = { @@ -145,7 +145,7 @@ describe('DiffsStoreActions', () => { diff_file: { file_hash: 'ABC', }, - fileHash: 'ABC', + file_hash: 'ABC', resolvable: true, position: diffPosition, original_position: diffPosition, @@ -164,24 +164,22 @@ describe('DiffsStoreActions', () => { discussion: singleDiscussion, diffPositionByLineCode: { ABC_1_1: { - baseSha: 'abc', - headSha: 'def', - startSha: 'ghi', - newLine: null, - newPath: 'file1', - oldLine: 5, - oldPath: 'file2', - lineCode: 'ABC_1_1', - positionType: 'text', + base_sha: 'abc', + head_sha: 'def', + start_sha: 'ghi', + new_line: null, + new_path: 'file1', + old_line: 5, + old_path: 'file2', + line_code: 'ABC_1_1', + position_type: 'text', }, }, }, }, ], [], - () => { - done(); - }, + done, ); }); }); @@ -191,11 +189,11 @@ describe('DiffsStoreActions', () => { const state = { diffFiles: [ { - fileHash: 'ABC', - parallelDiffLines: [ + file_hash: 'ABC', + parallel_diff_lines: [ { left: { - lineCode: 'ABC_1_1', + line_code: 'ABC_1_1', discussions: [ { id: 1, @@ -203,14 +201,14 @@ describe('DiffsStoreActions', () => { ], }, right: { - lineCode: 'ABC_1_1', + line_code: 'ABC_1_1', discussions: [], }, }, ], - highlightedDiffLines: [ + highlighted_diff_lines: [ { - lineCode: 'ABC_1_1', + line_code: 'ABC_1_1', discussions: [], }, ], @@ -219,7 +217,7 @@ describe('DiffsStoreActions', () => { }; const singleDiscussion = { id: '1', - fileHash: 'ABC', + file_hash: 'ABC', line_code: 'ABC_1_1', }; @@ -238,9 +236,7 @@ describe('DiffsStoreActions', () => { }, ], [], - () => { - done(); - }, + done, ); }); }); @@ -420,7 +416,7 @@ describe('DiffsStoreActions', () => { const getters = { getDiffFileDiscussions: jasmine.createSpy().and.returnValue([{ id: 1 }]), diffHasAllExpandedDiscussions: jasmine.createSpy().and.returnValue(true), - diffHasAllCollpasedDiscussions: jasmine.createSpy().and.returnValue(false), + diffHasAllCollapsedDiscussions: jasmine.createSpy().and.returnValue(false), }; const dispatch = jasmine.createSpy('dispatch'); @@ -438,7 +434,7 @@ describe('DiffsStoreActions', () => { const getters = { getDiffFileDiscussions: jasmine.createSpy().and.returnValue([{ id: 1 }]), diffHasAllExpandedDiscussions: jasmine.createSpy().and.returnValue(false), - diffHasAllCollpasedDiscussions: jasmine.createSpy().and.returnValue(true), + diffHasAllCollapsedDiscussions: jasmine.createSpy().and.returnValue(true), }; const dispatch = jasmine.createSpy(); @@ -456,7 +452,7 @@ describe('DiffsStoreActions', () => { const getters = { getDiffFileDiscussions: jasmine.createSpy().and.returnValue([{ expanded: false, id: 1 }]), diffHasAllExpandedDiscussions: jasmine.createSpy().and.returnValue(false), - diffHasAllCollpasedDiscussions: jasmine.createSpy().and.returnValue(false), + diffHasAllCollapsedDiscussions: jasmine.createSpy().and.returnValue(false), }; const dispatch = jasmine.createSpy(); diff --git a/spec/javascripts/diffs/store/getters_spec.js b/spec/javascripts/diffs/store/getters_spec.js index 9c3a38fd526..eef95c823fb 100644 --- a/spec/javascripts/diffs/store/getters_spec.js +++ b/spec/javascripts/diffs/store/getters_spec.js @@ -106,13 +106,13 @@ describe('Diffs Module Getters', () => { }); }); - describe('diffHasAllCollpasedDiscussions', () => { + describe('diffHasAllCollapsedDiscussions', () => { it('returns true when all discussions are collapsed', () => { discussionMock.diff_file.file_hash = diffFileMock.fileHash; discussionMock.expanded = false; expect( - getters.diffHasAllCollpasedDiscussions(localState, { + getters.diffHasAllCollapsedDiscussions(localState, { getDiffFileDiscussions: () => [discussionMock], })(diffFileMock), ).toEqual(true); @@ -120,7 +120,7 @@ describe('Diffs Module Getters', () => { it('returns false when there are no discussions', () => { expect( - getters.diffHasAllCollpasedDiscussions(localState, { + getters.diffHasAllCollapsedDiscussions(localState, { getDiffFileDiscussions: () => [], })(diffFileMock), ).toEqual(false); @@ -130,7 +130,7 @@ describe('Diffs Module Getters', () => { discussionMock1.expanded = false; expect( - getters.diffHasAllCollpasedDiscussions(localState, { + getters.diffHasAllCollapsedDiscussions(localState, { getDiffFileDiscussions: () => [discussionMock, discussionMock1], })(diffFileMock), ).toEqual(false); @@ -195,12 +195,12 @@ describe('Diffs Module Getters', () => { discussionMock.expanded = true; line.left = { - lineCode: 'ABC', + line_code: 'ABC', discussions: [discussionMock], }; line.right = { - lineCode: 'DEF', + line_code: 'DEF', discussions: [discussionMock1], }; }); @@ -259,7 +259,7 @@ describe('Diffs Module Getters', () => { describe('getDiffFileDiscussions', () => { it('returns an array with discussions when fileHash matches and the discussion belongs to a diff', () => { - discussionMock.diff_file.file_hash = diffFileMock.fileHash; + discussionMock.diff_file.file_hash = diffFileMock.file_hash; expect( getters.getDiffFileDiscussions(localState, {}, {}, { discussions: [discussionMock] })( @@ -279,10 +279,10 @@ describe('Diffs Module Getters', () => { describe('getDiffFileByHash', () => { it('returns file by hash', () => { const fileA = { - fileHash: '123', + file_hash: '123', }; const fileB = { - fileHash: '456', + file_hash: '456', }; localState.diffFiles = [fileA, fileB]; diff --git a/spec/javascripts/diffs/store/mutations_spec.js b/spec/javascripts/diffs/store/mutations_spec.js index 8821cde76f4..598d723c940 100644 --- a/spec/javascripts/diffs/store/mutations_spec.js +++ b/spec/javascripts/diffs/store/mutations_spec.js @@ -37,7 +37,7 @@ describe('DiffsStoreMutations', () => { mutations[types.SET_DIFF_DATA](state, diffMock); - const firstLine = state.diffFiles[0].parallelDiffLines[0]; + const firstLine = state.diffFiles[0].parallel_diff_lines[0]; expect(firstLine.right.text).toBeUndefined(); expect(state.diffFiles[0].renderIt).toEqual(true); @@ -98,19 +98,19 @@ describe('DiffsStoreMutations', () => { it('should call utils.addContextLines with proper params', () => { const options = { lineNumbers: { oldLineNumber: 1, newLineNumber: 2 }, - contextLines: [{ oldLine: 1, newLine: 1, lineCode: 'ff9200_1_1', discussions: [] }], + contextLines: [{ old_line: 1, new_line: 1, line_code: 'ff9200_1_1', discussions: [] }], fileHash: 'ff9200', params: { bottom: true, }, }; const diffFile = { - fileHash: options.fileHash, - highlightedDiffLines: [], - parallelDiffLines: [], + file_hash: options.fileHash, + highlighted_diff_lines: [], + parallel_diff_lines: [], }; const state = { diffFiles: [diffFile] }; - const lines = [{ oldLine: 1, newLine: 1 }]; + const lines = [{ old_line: 1, new_line: 1 }]; const findDiffFileSpy = spyOnDependency(mutations, 'findDiffFile').and.returnValue(diffFile); const removeMatchLineSpy = spyOnDependency(mutations, 'removeMatchLine'); @@ -133,8 +133,8 @@ describe('DiffsStoreMutations', () => { ); expect(addContextLinesSpy).toHaveBeenCalledWith({ - inlineLines: diffFile.highlightedDiffLines, - parallelLines: diffFile.parallelDiffLines, + inlineLines: diffFile.highlighted_diff_lines, + parallelLines: diffFile.parallel_diff_lines, contextLines: options.contextLines, bottom: options.params.bottom, lineNumbers: options.lineNumbers, @@ -144,54 +144,50 @@ describe('DiffsStoreMutations', () => { describe('ADD_COLLAPSED_DIFFS', () => { it('should update the state with the given data for the given file hash', () => { - const spy = spyOnDependency(mutations, 'convertObjectPropsToCamelCase').and.callThrough(); - const fileHash = 123; - const state = { diffFiles: [{}, { fileHash, existingField: 0 }] }; - const data = { diff_files: [{ file_hash: fileHash, extra_field: 1, existingField: 1 }] }; + const state = { diffFiles: [{}, { file_hash: fileHash, existing_field: 0 }] }; + const data = { diff_files: [{ file_hash: fileHash, extra_field: 1, existing_field: 1 }] }; mutations[types.ADD_COLLAPSED_DIFFS](state, { file: state.diffFiles[1], data }); - expect(spy).toHaveBeenCalledWith(data, { deep: true }); - - expect(state.diffFiles[1].fileHash).toEqual(fileHash); - expect(state.diffFiles[1].existingField).toEqual(1); - expect(state.diffFiles[1].extraField).toEqual(1); + expect(state.diffFiles[1].file_hash).toEqual(fileHash); + expect(state.diffFiles[1].existing_field).toEqual(1); + expect(state.diffFiles[1].extra_field).toEqual(1); }); }); describe('SET_LINE_DISCUSSIONS_FOR_FILE', () => { it('should add discussions to the given line', () => { const diffPosition = { - baseSha: 'ed13df29948c41ba367caa757ab3ec4892509910', - headSha: 'b921914f9a834ac47e6fd9420f78db0f83559130', - newLine: null, - newPath: '500-lines-4.txt', - oldLine: 5, - oldPath: '500-lines-4.txt', - startSha: 'ed13df29948c41ba367caa757ab3ec4892509910', + base_sha: 'ed13df29948c41ba367caa757ab3ec4892509910', + head_sha: 'b921914f9a834ac47e6fd9420f78db0f83559130', + new_line: null, + new_path: '500-lines-4.txt', + old_line: 5, + old_path: '500-lines-4.txt', + start_sha: 'ed13df29948c41ba367caa757ab3ec4892509910', }; const state = { latestDiff: true, diffFiles: [ { - fileHash: 'ABC', - parallelDiffLines: [ + file_hash: 'ABC', + parallel_diff_lines: [ { left: { - lineCode: 'ABC_1', + line_code: 'ABC_1', discussions: [], }, right: { - lineCode: 'ABC_1', + line_code: 'ABC_1', discussions: [], }, }, ], - highlightedDiffLines: [ + highlighted_diff_lines: [ { - lineCode: 'ABC_1', + line_code: 'ABC_1', discussions: [], }, ], @@ -206,7 +202,7 @@ describe('DiffsStoreMutations', () => { original_position: diffPosition, position: diffPosition, diff_file: { - file_hash: state.diffFiles[0].fileHash, + file_hash: state.diffFiles[0].file_hash, }, }; @@ -219,46 +215,46 @@ describe('DiffsStoreMutations', () => { diffPositionByLineCode, }); - expect(state.diffFiles[0].parallelDiffLines[0].left.discussions.length).toEqual(1); - expect(state.diffFiles[0].parallelDiffLines[0].left.discussions[0].id).toEqual(1); - expect(state.diffFiles[0].parallelDiffLines[0].right.discussions).toEqual([]); + expect(state.diffFiles[0].parallel_diff_lines[0].left.discussions.length).toEqual(1); + expect(state.diffFiles[0].parallel_diff_lines[0].left.discussions[0].id).toEqual(1); + expect(state.diffFiles[0].parallel_diff_lines[0].right.discussions).toEqual([]); - expect(state.diffFiles[0].highlightedDiffLines[0].discussions.length).toEqual(1); - expect(state.diffFiles[0].highlightedDiffLines[0].discussions[0].id).toEqual(1); + expect(state.diffFiles[0].highlighted_diff_lines[0].discussions.length).toEqual(1); + expect(state.diffFiles[0].highlighted_diff_lines[0].discussions[0].id).toEqual(1); }); it('should add legacy discussions to the given line', () => { const diffPosition = { - baseSha: 'ed13df29948c41ba367caa757ab3ec4892509910', - headSha: 'b921914f9a834ac47e6fd9420f78db0f83559130', - newLine: null, - newPath: '500-lines-4.txt', - oldLine: 5, - oldPath: '500-lines-4.txt', - startSha: 'ed13df29948c41ba367caa757ab3ec4892509910', - lineCode: 'ABC_1', + base_sha: 'ed13df29948c41ba367caa757ab3ec4892509910', + head_sha: 'b921914f9a834ac47e6fd9420f78db0f83559130', + new_line: null, + new_path: '500-lines-4.txt', + old_line: 5, + old_path: '500-lines-4.txt', + start_sha: 'ed13df29948c41ba367caa757ab3ec4892509910', + line_code: 'ABC_1', }; const state = { latestDiff: true, diffFiles: [ { - fileHash: 'ABC', - parallelDiffLines: [ + file_hash: 'ABC', + parallel_diff_lines: [ { left: { - lineCode: 'ABC_1', + line_code: 'ABC_1', discussions: [], }, right: { - lineCode: 'ABC_1', + line_code: 'ABC_1', discussions: [], }, }, ], - highlightedDiffLines: [ + highlighted_diff_lines: [ { - lineCode: 'ABC_1', + line_code: 'ABC_1', discussions: [], }, ], @@ -271,7 +267,7 @@ describe('DiffsStoreMutations', () => { diff_discussion: true, active: true, diff_file: { - file_hash: state.diffFiles[0].fileHash, + file_hash: state.diffFiles[0].file_hash, }, }; @@ -284,11 +280,11 @@ describe('DiffsStoreMutations', () => { diffPositionByLineCode, }); - expect(state.diffFiles[0].parallelDiffLines[0].left.discussions.length).toEqual(1); - expect(state.diffFiles[0].parallelDiffLines[0].left.discussions[0].id).toEqual(1); + expect(state.diffFiles[0].parallel_diff_lines[0].left.discussions.length).toEqual(1); + expect(state.diffFiles[0].parallel_diff_lines[0].left.discussions[0].id).toEqual(1); - expect(state.diffFiles[0].highlightedDiffLines[0].discussions.length).toEqual(1); - expect(state.diffFiles[0].highlightedDiffLines[0].discussions[0].id).toEqual(1); + expect(state.diffFiles[0].highlighted_diff_lines[0].discussions.length).toEqual(1); + expect(state.diffFiles[0].highlighted_diff_lines[0].discussions[0].id).toEqual(1); }); }); @@ -297,11 +293,11 @@ describe('DiffsStoreMutations', () => { const state = { diffFiles: [ { - fileHash: 'ABC', - parallelDiffLines: [ + file_hash: 'ABC', + parallel_diff_lines: [ { left: { - lineCode: 'ABC_1', + line_code: 'ABC_1', discussions: [ { id: 1, @@ -314,14 +310,14 @@ describe('DiffsStoreMutations', () => { ], }, right: { - lineCode: 'ABC_1', + line_code: 'ABC_1', discussions: [], }, }, ], - highlightedDiffLines: [ + highlighted_diff_lines: [ { - lineCode: 'ABC_1', + line_code: 'ABC_1', discussions: [ { id: 1, @@ -343,8 +339,8 @@ describe('DiffsStoreMutations', () => { lineCode: 'ABC_1', }); - expect(state.diffFiles[0].parallelDiffLines[0].left.discussions.length).toEqual(0); - expect(state.diffFiles[0].highlightedDiffLines[0].discussions.length).toEqual(0); + expect(state.diffFiles[0].parallel_diff_lines[0].left.discussions.length).toEqual(0); + expect(state.diffFiles[0].highlighted_diff_lines[0].discussions.length).toEqual(0); }); }); diff --git a/spec/javascripts/diffs/store/utils_spec.js b/spec/javascripts/diffs/store/utils_spec.js index f49dee3696d..d4ef17c5ef8 100644 --- a/spec/javascripts/diffs/store/utils_spec.js +++ b/spec/javascripts/diffs/store/utils_spec.js @@ -18,7 +18,7 @@ const getDiffFileMock = () => Object.assign({}, diffFileMockData); describe('DiffsStoreUtils', () => { describe('findDiffFile', () => { - const files = [{ fileHash: 1, name: 'one' }]; + const files = [{ file_hash: 1, name: 'one' }]; it('should return correct file', () => { expect(utils.findDiffFile(files, 1).name).toEqual('one'); @@ -41,13 +41,13 @@ describe('DiffsStoreUtils', () => { describe('findIndexInInlineLines', () => { it('should return correct index for given line numbers', () => { - expectSet(utils.findIndexInInlineLines, getDiffFileMock().highlightedDiffLines); + expectSet(utils.findIndexInInlineLines, getDiffFileMock().highlighted_diff_lines); }); }); describe('findIndexInParallelLines', () => { it('should return correct index for given line numbers', () => { - expectSet(utils.findIndexInParallelLines, getDiffFileMock().parallelDiffLines, {}); + expectSet(utils.findIndexInParallelLines, getDiffFileMock().parallel_diff_lines, {}); }); }); }); @@ -56,33 +56,39 @@ describe('DiffsStoreUtils', () => { it('should remove match line properly by regarding the bottom parameter', () => { const diffFile = getDiffFileMock(); const lineNumbers = { oldLineNumber: 3, newLineNumber: 5 }; - const inlineIndex = utils.findIndexInInlineLines(diffFile.highlightedDiffLines, lineNumbers); - const parallelIndex = utils.findIndexInParallelLines(diffFile.parallelDiffLines, lineNumbers); - const atInlineIndex = diffFile.highlightedDiffLines[inlineIndex]; - const atParallelIndex = diffFile.parallelDiffLines[parallelIndex]; + const inlineIndex = utils.findIndexInInlineLines( + diffFile.highlighted_diff_lines, + lineNumbers, + ); + const parallelIndex = utils.findIndexInParallelLines( + diffFile.parallel_diff_lines, + lineNumbers, + ); + const atInlineIndex = diffFile.highlighted_diff_lines[inlineIndex]; + const atParallelIndex = diffFile.parallel_diff_lines[parallelIndex]; utils.removeMatchLine(diffFile, lineNumbers, false); - expect(diffFile.highlightedDiffLines[inlineIndex]).not.toEqual(atInlineIndex); - expect(diffFile.parallelDiffLines[parallelIndex]).not.toEqual(atParallelIndex); + expect(diffFile.highlighted_diff_lines[inlineIndex]).not.toEqual(atInlineIndex); + expect(diffFile.parallel_diff_lines[parallelIndex]).not.toEqual(atParallelIndex); utils.removeMatchLine(diffFile, lineNumbers, true); - expect(diffFile.highlightedDiffLines[inlineIndex + 1]).not.toEqual(atInlineIndex); - expect(diffFile.parallelDiffLines[parallelIndex + 1]).not.toEqual(atParallelIndex); + expect(diffFile.highlighted_diff_lines[inlineIndex + 1]).not.toEqual(atInlineIndex); + expect(diffFile.parallel_diff_lines[parallelIndex + 1]).not.toEqual(atParallelIndex); }); }); describe('addContextLines', () => { it('should add context lines properly with bottom parameter', () => { const diffFile = getDiffFileMock(); - const inlineLines = diffFile.highlightedDiffLines; - const parallelLines = diffFile.parallelDiffLines; + const inlineLines = diffFile.highlighted_diff_lines; + const parallelLines = diffFile.parallel_diff_lines; const lineNumbers = { oldLineNumber: 3, newLineNumber: 5 }; const contextLines = [{ lineNumber: 42 }]; const options = { inlineLines, parallelLines, contextLines, lineNumbers, bottom: true }; - const inlineIndex = utils.findIndexInInlineLines(diffFile.highlightedDiffLines, lineNumbers); - const parallelIndex = utils.findIndexInParallelLines(diffFile.parallelDiffLines, lineNumbers); + const inlineIndex = utils.findIndexInInlineLines(inlineLines, lineNumbers); + const parallelIndex = utils.findIndexInParallelLines(parallelLines, lineNumbers); const normalizedParallelLine = { left: options.contextLines[0], right: options.contextLines[0], @@ -112,30 +118,30 @@ describe('DiffsStoreUtils', () => { noteableType: MERGE_REQUEST_NOTEABLE_TYPE, diffFile, noteTargetLine: { - lineCode: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_3', - metaData: null, - newLine: 3, - oldLine: 1, + line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_3', + meta_data: null, + new_line: 3, + old_line: 1, }, diffViewType: PARALLEL_DIFF_VIEW_TYPE, linePosition: LINE_POSITION_LEFT, }; const position = JSON.stringify({ - base_sha: diffFile.diffRefs.baseSha, - start_sha: diffFile.diffRefs.startSha, - head_sha: diffFile.diffRefs.headSha, - old_path: diffFile.oldPath, - new_path: diffFile.newPath, + base_sha: diffFile.diff_refs.base_sha, + start_sha: diffFile.diff_refs.start_sha, + head_sha: diffFile.diff_refs.head_sha, + old_path: diffFile.old_path, + new_path: diffFile.new_path, position_type: TEXT_DIFF_POSITION_TYPE, - old_line: options.noteTargetLine.oldLine, - new_line: options.noteTargetLine.newLine, + old_line: options.noteTargetLine.old_line, + new_line: options.noteTargetLine.new_line, }); const postData = { view: options.diffViewType, line_type: options.linePosition === LINE_POSITION_RIGHT ? NEW_LINE_TYPE : OLD_LINE_TYPE, - merge_request_diff_head_sha: diffFile.diffRefs.headSha, + merge_request_diff_head_sha: diffFile.diff_refs.head_sha, in_reply_to_discussion_id: '', note_project_id: '', target_type: options.noteableType, @@ -146,7 +152,7 @@ describe('DiffsStoreUtils', () => { noteable_id: options.noteableData.id, commit_id: '', type: DIFF_NOTE_TYPE, - line_code: options.noteTargetLine.lineCode, + line_code: options.noteTargetLine.line_code, note: options.note, position, }, @@ -160,8 +166,8 @@ describe('DiffsStoreUtils', () => { it('should create legacy note form data', () => { const diffFile = getDiffFileMock(); - delete diffFile.diffRefs.startSha; - delete diffFile.diffRefs.headSha; + delete diffFile.diff_refs.start_sha; + delete diffFile.diff_refs.head_sha; noteableDataMock.targetType = MERGE_REQUEST_NOTEABLE_TYPE; @@ -171,24 +177,24 @@ describe('DiffsStoreUtils', () => { noteableType: MERGE_REQUEST_NOTEABLE_TYPE, diffFile, noteTargetLine: { - lineCode: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_3', - metaData: null, - newLine: 3, - oldLine: 1, + line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_3', + meta_data: null, + new_line: 3, + old_line: 1, }, diffViewType: PARALLEL_DIFF_VIEW_TYPE, linePosition: LINE_POSITION_LEFT, }; const position = JSON.stringify({ - base_sha: diffFile.diffRefs.baseSha, + base_sha: diffFile.diff_refs.base_sha, start_sha: undefined, head_sha: undefined, - old_path: diffFile.oldPath, - new_path: diffFile.newPath, + old_path: diffFile.old_path, + new_path: diffFile.new_path, position_type: TEXT_DIFF_POSITION_TYPE, - old_line: options.noteTargetLine.oldLine, - new_line: options.noteTargetLine.newLine, + old_line: options.noteTargetLine.old_line, + new_line: options.noteTargetLine.new_line, }); const postData = { @@ -205,7 +211,7 @@ describe('DiffsStoreUtils', () => { noteable_id: options.noteableData.id, commit_id: '', type: LEGACY_DIFF_NOTE_TYPE, - line_code: options.noteTargetLine.lineCode, + line_code: options.noteTargetLine.line_code, note: options.note, position, }, @@ -225,61 +231,61 @@ describe('DiffsStoreUtils', () => { const lines = [{ type: null }, { type: MATCH_LINE_TYPE }]; const linesWithReferences = utils.addLineReferences(lines, lineNumbers, true); - expect(linesWithReferences[0].oldLine).toEqual(lineNumbers.oldLineNumber + 1); - expect(linesWithReferences[0].newLine).toEqual(lineNumbers.newLineNumber + 1); - expect(linesWithReferences[1].metaData.oldPos).toEqual(4); - expect(linesWithReferences[1].metaData.newPos).toEqual(5); + expect(linesWithReferences[0].old_line).toEqual(lineNumbers.oldLineNumber + 1); + expect(linesWithReferences[0].new_line).toEqual(lineNumbers.newLineNumber + 1); + expect(linesWithReferences[1].meta_data.old_pos).toEqual(4); + expect(linesWithReferences[1].meta_data.new_pos).toEqual(5); }); it('should add correct line references when bottom falsy', () => { const lines = [{ type: null }, { type: MATCH_LINE_TYPE }, { type: null }]; const linesWithReferences = utils.addLineReferences(lines, lineNumbers); - expect(linesWithReferences[0].oldLine).toEqual(0); - expect(linesWithReferences[0].newLine).toEqual(1); - expect(linesWithReferences[1].metaData.oldPos).toEqual(2); - expect(linesWithReferences[1].metaData.newPos).toEqual(3); + expect(linesWithReferences[0].old_line).toEqual(0); + expect(linesWithReferences[0].new_line).toEqual(1); + expect(linesWithReferences[1].meta_data.old_pos).toEqual(2); + expect(linesWithReferences[1].meta_data.new_pos).toEqual(3); }); }); describe('trimFirstCharOfLineContent', () => { it('trims the line when it starts with a space', () => { - expect(utils.trimFirstCharOfLineContent({ richText: ' diff' })).toEqual({ + expect(utils.trimFirstCharOfLineContent({ rich_text: ' diff' })).toEqual({ discussions: [], - richText: 'diff', + rich_text: 'diff', }); }); it('trims the line when it starts with a +', () => { - expect(utils.trimFirstCharOfLineContent({ richText: '+diff' })).toEqual({ + expect(utils.trimFirstCharOfLineContent({ rich_text: '+diff' })).toEqual({ discussions: [], - richText: 'diff', + rich_text: 'diff', }); }); it('trims the line when it starts with a -', () => { - expect(utils.trimFirstCharOfLineContent({ richText: '-diff' })).toEqual({ + expect(utils.trimFirstCharOfLineContent({ rich_text: '-diff' })).toEqual({ discussions: [], - richText: 'diff', + rich_text: 'diff', }); }); it('does not trims the line when it starts with a letter', () => { - expect(utils.trimFirstCharOfLineContent({ richText: 'diff' })).toEqual({ + expect(utils.trimFirstCharOfLineContent({ rich_text: 'diff' })).toEqual({ discussions: [], - richText: 'diff', + rich_text: 'diff', }); }); it('does not modify the provided object', () => { const lineObj = { discussions: [], - richText: ' diff', + rich_text: ' diff', }; utils.trimFirstCharOfLineContent(lineObj); - expect(lineObj).toEqual({ discussions: [], richText: ' diff' }); + expect(lineObj).toEqual({ discussions: [], rich_text: ' diff' }); }); it('handles a undefined or null parameter', () => { @@ -289,33 +295,33 @@ describe('DiffsStoreUtils', () => { describe('prepareDiffData', () => { it('sets the renderIt and collapsed attribute on files', () => { - const preparedDiff = { diffFiles: [getDiffFileMock()] }; + const preparedDiff = { diff_files: [getDiffFileMock()] }; utils.prepareDiffData(preparedDiff); - const firstParallelDiffLine = preparedDiff.diffFiles[0].parallelDiffLines[2]; + const firstParallelDiffLine = preparedDiff.diff_files[0].parallel_diff_lines[2]; expect(firstParallelDiffLine.left.discussions.length).toBe(0); expect(firstParallelDiffLine.left).not.toHaveAttr('text'); expect(firstParallelDiffLine.right.discussions.length).toBe(0); expect(firstParallelDiffLine.right).not.toHaveAttr('text'); - const firstParallelChar = firstParallelDiffLine.right.richText.charAt(0); + const firstParallelChar = firstParallelDiffLine.right.rich_text.charAt(0); expect(firstParallelChar).not.toBe(' '); expect(firstParallelChar).not.toBe('+'); expect(firstParallelChar).not.toBe('-'); - const checkLine = preparedDiff.diffFiles[0].highlightedDiffLines[0]; + const checkLine = preparedDiff.diff_files[0].highlighted_diff_lines[0]; expect(checkLine.discussions.length).toBe(0); expect(checkLine).not.toHaveAttr('text'); - const firstChar = checkLine.richText.charAt(0); + const firstChar = checkLine.rich_text.charAt(0); expect(firstChar).not.toBe(' '); expect(firstChar).not.toBe('+'); expect(firstChar).not.toBe('-'); - expect(preparedDiff.diffFiles[0].renderIt).toBeTruthy(); - expect(preparedDiff.diffFiles[0].collapsed).toBeFalsy(); + expect(preparedDiff.diff_files[0].renderIt).toBeTruthy(); + expect(preparedDiff.diff_files[0].collapsed).toBeFalsy(); }); }); @@ -398,7 +404,7 @@ describe('DiffsStoreUtils', () => { discussion, diffPosition: { ...diffPosition, - lineCode: 'ABC_1', + line_code: 'ABC_1', }, latestDiff: true, }), @@ -429,36 +435,36 @@ describe('DiffsStoreUtils', () => { beforeAll(() => { files = [ { - newPath: 'app/index.js', - deletedFile: false, - newFile: false, - removedLines: 10, - addedLines: 0, - fileHash: 'test', + new_path: 'app/index.js', + deleted_file: false, + new_file: false, + removed_lines: 10, + added_lines: 0, + file_hash: 'test', }, { - newPath: 'app/test/index.js', - deletedFile: false, - newFile: true, - removedLines: 0, - addedLines: 0, - fileHash: 'test', + new_path: 'app/test/index.js', + deleted_file: false, + new_file: true, + removed_lines: 0, + added_lines: 0, + file_hash: 'test', }, { - newPath: 'app/test/filepathneedstruncating.js', - deletedFile: false, - newFile: true, - removedLines: 0, - addedLines: 0, - fileHash: 'test', + new_path: 'app/test/filepathneedstruncating.js', + deleted_file: false, + new_file: true, + removed_lines: 0, + added_lines: 0, + file_hash: 'test', }, { - newPath: 'package.json', - deletedFile: true, - newFile: false, - removedLines: 0, - addedLines: 0, - fileHash: 'test', + new_path: 'package.json', + deleted_file: true, + new_file: false, + removed_lines: 0, + added_lines: 0, + file_hash: 'test', }, ]; }); diff --git a/spec/javascripts/filtered_search/filtered_search_visual_tokens_spec.js b/spec/javascripts/filtered_search/filtered_search_visual_tokens_spec.js index 0c1d5f5b0b4..4f561df7943 100644 --- a/spec/javascripts/filtered_search/filtered_search_visual_tokens_spec.js +++ b/spec/javascripts/filtered_search/filtered_search_visual_tokens_spec.js @@ -754,6 +754,50 @@ describe('Filtered Search Visual Tokens', () => { expect(updateLabelTokenColorSpy.calls.count()).toBe(0); expect(updateUserTokenAppearanceSpy.calls.count()).toBe(0); }); + + it('does not update user token appearance for `none` filter', () => { + const { tokenNameElement } = findElements(authorToken); + + const tokenName = tokenNameElement.innerText; + const tokenValue = 'none'; + + subject.renderVisualTokenValue(authorToken, tokenName, tokenValue); + + expect(updateUserTokenAppearanceSpy.calls.count()).toBe(0); + }); + + it('does not update user token appearance for `any` filter', () => { + const { tokenNameElement } = findElements(authorToken); + + const tokenName = tokenNameElement.innerText; + const tokenValue = 'any'; + + subject.renderVisualTokenValue(authorToken, tokenName, tokenValue); + + expect(updateUserTokenAppearanceSpy.calls.count()).toBe(0); + }); + + it('does not update label token color for `none` filter', () => { + const { tokenNameElement } = findElements(bugLabelToken); + + const tokenName = tokenNameElement.innerText; + const tokenValue = 'none'; + + subject.renderVisualTokenValue(bugLabelToken, tokenName, tokenValue); + + expect(updateLabelTokenColorSpy.calls.count()).toBe(0); + }); + + it('does not update label token color for `any` filter', () => { + const { tokenNameElement } = findElements(bugLabelToken); + + const tokenName = tokenNameElement.innerText; + const tokenValue = 'any'; + + subject.renderVisualTokenValue(bugLabelToken, tokenName, tokenValue); + + expect(updateLabelTokenColorSpy.calls.count()).toBe(0); + }); }); describe('updateUserTokenAppearance', () => { @@ -763,19 +807,6 @@ describe('Filtered Search Visual Tokens', () => { spyOn(UsersCache, 'retrieve').and.callFake(username => usersCacheSpy(username)); }); - it('ignores special value "none"', done => { - usersCacheSpy = username => { - expect(username).toBe('none'); - done.fail('Should not resolve "none"!'); - }; - const { tokenValueContainer, tokenValueElement } = findElements(authorToken); - - subject - .updateUserTokenAppearance(tokenValueContainer, tokenValueElement, 'none') - .then(done) - .catch(done.fail); - }); - it('ignores error if UsersCache throws', done => { spyOn(window, 'Flash'); const dummyError = new Error('Earth rotated backwards'); diff --git a/spec/javascripts/ide/stores/actions/merge_request_spec.js b/spec/javascripts/ide/stores/actions/merge_request_spec.js index 3a4e0d7507f..d8e9260e932 100644 --- a/spec/javascripts/ide/stores/actions/merge_request_spec.js +++ b/spec/javascripts/ide/stores/actions/merge_request_spec.js @@ -262,16 +262,28 @@ describe('IDE store merge request actions', () => { bar: {}, }; - spyOn(store, 'dispatch').and.callFake(type => { + const originalDispatch = store.dispatch; + + spyOn(store, 'dispatch').and.callFake((type, payload) => { switch (type) { case 'getMergeRequestData': return Promise.resolve(testMergeRequest); case 'getMergeRequestChanges': return Promise.resolve(testMergeRequestChanges); - default: + case 'getFiles': + case 'getMergeRequestVersions': + case 'getBranchData': + case 'setFileMrChange': return Promise.resolve(); + default: + return originalDispatch(type, payload); } }); + spyOn(service, 'getFileData').and.callFake(() => + Promise.resolve({ + headers: {}, + }), + ); }); it('dispatch actions for merge request data', done => { @@ -303,7 +315,17 @@ describe('IDE store merge request actions', () => { }); it('updates activity bar view and gets file data, if changes are found', done => { - testMergeRequestChanges.changes = [{ new_path: 'foo' }, { new_path: 'bar' }]; + store.state.entries.foo = { + url: 'test', + }; + store.state.entries.bar = { + url: 'test', + }; + + testMergeRequestChanges.changes = [ + { new_path: 'foo', path: 'foo' }, + { new_path: 'bar', path: 'bar' }, + ]; openMergeRequest(store, mr) .then(() => { @@ -321,8 +343,11 @@ describe('IDE store merge request actions', () => { expect(store.dispatch).toHaveBeenCalledWith('getFileData', { path: change.new_path, makeFileActive: i === 0, + openFile: true, }); }); + + expect(store.state.openFiles.length).toBe(testMergeRequestChanges.changes.length); }) .then(done) .catch(done.fail); diff --git a/spec/javascripts/ide/stores/modules/pipelines/actions_spec.js b/spec/javascripts/ide/stores/modules/pipelines/actions_spec.js index d85354c3681..c9c09ee9afe 100644 --- a/spec/javascripts/ide/stores/modules/pipelines/actions_spec.js +++ b/spec/javascripts/ide/stores/modules/pipelines/actions_spec.js @@ -77,7 +77,7 @@ describe('IDE pipelines actions', () => { { type: 'setErrorMessage', payload: { - text: 'An error occured whilst fetching the latest pipline.', + text: 'An error occured whilst fetching the latest pipeline.', action: jasmine.any(Function), actionText: 'Please try again', actionPayload: null, diff --git a/spec/javascripts/jobs/components/empty_state_spec.js b/spec/javascripts/jobs/components/empty_state_spec.js index 0a39709221c..a2df79bdda0 100644 --- a/spec/javascripts/jobs/components/empty_state_spec.js +++ b/spec/javascripts/jobs/components/empty_state_spec.js @@ -84,6 +84,7 @@ describe('Empty State', () => { vm = mountComponent(Component, { ...props, content, + action: null, }); expect(vm.$el.querySelector('.js-job-empty-state-action')).toBeNull(); diff --git a/spec/javascripts/lib/utils/common_utils_spec.js b/spec/javascripts/lib/utils/common_utils_spec.js index 0fb90c3b78c..1c7691f865a 100644 --- a/spec/javascripts/lib/utils/common_utils_spec.js +++ b/spec/javascripts/lib/utils/common_utils_spec.js @@ -425,14 +425,16 @@ describe('common_utils', () => { }); it('rejects the backOff promise after timing out', done => { - commonUtils.backOff(next => next(), 64000).catch(errBackoffResp => { - const timeouts = window.setTimeout.calls.allArgs().map(([, timeout]) => timeout); + commonUtils + .backOff(next => next(), 64000) + .catch(errBackoffResp => { + const timeouts = window.setTimeout.calls.allArgs().map(([, timeout]) => timeout); - expect(timeouts).toEqual([2000, 4000, 8000, 16000, 32000, 32000]); - expect(errBackoffResp instanceof Error).toBe(true); - expect(errBackoffResp.message).toBe('BACKOFF_TIMEOUT'); - done(); - }); + expect(timeouts).toEqual([2000, 4000, 8000, 16000, 32000, 32000]); + expect(errBackoffResp instanceof Error).toBe(true); + expect(errBackoffResp.message).toBe('BACKOFF_TIMEOUT'); + done(); + }); }); }); diff --git a/spec/javascripts/lib/utils/text_markdown_spec.js b/spec/javascripts/lib/utils/text_markdown_spec.js index b9e805628f8..f71d27eb4e4 100644 --- a/spec/javascripts/lib/utils/text_markdown_spec.js +++ b/spec/javascripts/lib/utils/text_markdown_spec.js @@ -86,6 +86,29 @@ describe('init markdown', () => { expect(textArea.value).toEqual(`${initialValue}* `); }); + + it('places the cursor inside the tags', () => { + const start = 'lorem '; + const end = ' ipsum'; + const tag = '*'; + + textArea.value = `${start}${end}`; + textArea.setSelectionRange(start.length, start.length); + + insertMarkdownText({ + textArea, + text: textArea.value, + tag, + blockTag: null, + selected: '', + wrap: true, + }); + + expect(textArea.value).toEqual(`${start}**${end}`); + + // cursor placement should be between tags + expect(textArea.selectionStart).toBe(start.length + tag.length); + }); }); describe('with selection', () => { @@ -98,16 +121,22 @@ describe('init markdown', () => { }); it('applies the tag to the selected value', () => { + const selectedIndex = text.indexOf(selected); + const tag = '*'; + insertMarkdownText({ textArea, text: textArea.value, - tag: '*', + tag, blockTag: null, selected, wrap: true, }); expect(textArea.value).toEqual(text.replace(selected, `*${selected}*`)); + + // cursor placement should be after selection + 2 tag lengths + expect(textArea.selectionStart).toBe(selectedIndex + selected.length + 2 * tag.length); }); it('replaces the placeholder in the tag', () => { diff --git a/spec/javascripts/notes/components/diff_with_note_spec.js b/spec/javascripts/notes/components/diff_with_note_spec.js index 0c16103714a..95461396f10 100644 --- a/spec/javascripts/notes/components/diff_with_note_spec.js +++ b/spec/javascripts/notes/components/diff_with_note_spec.js @@ -1,6 +1,5 @@ import Vue from 'vue'; import DiffWithNote from '~/notes/components/diff_with_note.vue'; -import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils'; import { createStore } from '~/mr_notes/stores'; import { mountComponentWithStore } from 'spec/helpers'; @@ -11,7 +10,7 @@ describe('diff_with_note', () => { let store; let vm; const diffDiscussionMock = getJSONFixture(discussionFixture)[0]; - const diffDiscussion = convertObjectPropsToCamelCase(diffDiscussionMock); + const diffDiscussion = diffDiscussionMock; const Component = Vue.extend(DiffWithNote); const props = { discussion: diffDiscussion, @@ -65,7 +64,7 @@ describe('diff_with_note', () => { describe('image diff', () => { beforeEach(() => { const imageDiffDiscussionMock = getJSONFixture(imageDiscussionFixture)[0]; - props.discussion = convertObjectPropsToCamelCase(imageDiffDiscussionMock); + props.discussion = imageDiffDiscussionMock; }); it('shows image diff', () => { diff --git a/spec/javascripts/performance_bar/components/detailed_metric_spec.js b/spec/javascripts/performance_bar/components/detailed_metric_spec.js index a3b93280b4b..e91685e50c5 100644 --- a/spec/javascripts/performance_bar/components/detailed_metric_spec.js +++ b/spec/javascripts/performance_bar/components/detailed_metric_spec.js @@ -67,7 +67,7 @@ describe('detailedMetric', () => { vm.$el .querySelectorAll('.performance-bar-modal td:nth-child(3)') .forEach((request, index) => { - expect(request.innerText).toContain(requestDetails[index].request); + expect(request.innerText).toEqual(requestDetails[index].request); }); }); diff --git a/spec/javascripts/pipelines/graph/job_item_spec.js b/spec/javascripts/pipelines/graph/job_item_spec.js index 41b614cc95e..88e1789184d 100644 --- a/spec/javascripts/pipelines/graph/job_item_spec.js +++ b/spec/javascripts/pipelines/graph/job_item_spec.js @@ -140,14 +140,12 @@ describe('pipeline graph job item', () => { }); describe('tooltip placement', () => { - const tooltipBoundary = 'a[data-boundary="viewport"]'; - it('does not set tooltip boundary by default', () => { component = mountComponent(JobComponent, { job: mockJob, }); - expect(component.$el.querySelector(tooltipBoundary)).toBeNull(); + expect(component.tooltipBoundary).toBeNull(); }); it('sets tooltip boundary to viewport for small dropdowns', () => { @@ -156,7 +154,7 @@ describe('pipeline graph job item', () => { dropdownLength: 1, }); - expect(component.$el.querySelector(tooltipBoundary)).not.toBeNull(); + expect(component.tooltipBoundary).toEqual('viewport'); }); it('does not set tooltip boundary for large lists', () => { @@ -165,7 +163,7 @@ describe('pipeline graph job item', () => { dropdownLength: 7, }); - expect(component.$el.querySelector(tooltipBoundary)).toBeNull(); + expect(component.tooltipBoundary).toBeNull(); }); }); diff --git a/spec/javascripts/search_autocomplete_spec.js b/spec/javascripts/search_autocomplete_spec.js index 7530fd2a43b..7a4ca587313 100644 --- a/spec/javascripts/search_autocomplete_spec.js +++ b/spec/javascripts/search_autocomplete_spec.js @@ -1,4 +1,4 @@ -/* eslint-disable no-var, one-var, no-unused-expressions, consistent-return, no-param-reassign, default-case, no-return-assign, object-shorthand, prefer-template, vars-on-top */ +/* eslint-disable no-var, one-var, no-unused-expressions, consistent-return, no-param-reassign, default-case, no-return-assign, object-shorthand, vars-on-top */ import $ from 'jquery'; import '~/gl_dropdown'; @@ -109,16 +109,16 @@ describe('Search autocomplete dropdown', () => { assertLinks = function(list, issuesPath, mrsPath) { if (issuesPath) { - const issuesAssignedToMeLink = `a[href="${issuesPath}/?assignee_id=${userId}"]`; - const issuesIHaveCreatedLink = `a[href="${issuesPath}/?author_id=${userId}"]`; + const issuesAssignedToMeLink = `a[href="${issuesPath}/?assignee_username=${userName}"]`; + const issuesIHaveCreatedLink = `a[href="${issuesPath}/?author_username=${userName}"]`; expect(list.find(issuesAssignedToMeLink).length).toBe(1); expect(list.find(issuesAssignedToMeLink).text()).toBe('Issues assigned to me'); expect(list.find(issuesIHaveCreatedLink).length).toBe(1); expect(list.find(issuesIHaveCreatedLink).text()).toBe("Issues I've created"); } - const mrsAssignedToMeLink = `a[href="${mrsPath}/?assignee_id=${userId}"]`; - const mrsIHaveCreatedLink = `a[href="${mrsPath}/?author_id=${userId}"]`; + const mrsAssignedToMeLink = `a[href="${mrsPath}/?assignee_username=${userName}"]`; + const mrsIHaveCreatedLink = `a[href="${mrsPath}/?author_username=${userName}"]`; expect(list.find(mrsAssignedToMeLink).length).toBe(1); expect(list.find(mrsAssignedToMeLink).text()).toBe('Merge requests assigned to me'); @@ -186,7 +186,7 @@ describe('Search autocomplete dropdown', () => { widget.searchInput.val('help'); widget.searchInput.triggerHandler('focus'); list = widget.wrap.find('.dropdown-menu').find('ul'); - link = "a[href='" + projectIssuesPath + '/?assignee_id=' + userId + "']"; + link = `a[href='${projectIssuesPath}/?assignee_username=${userName}']`; expect(list.find(link).length).toBe(0); }); diff --git a/spec/javascripts/shared/popover_spec.js b/spec/javascripts/shared/popover_spec.js index 85bde075b77..cc2b2014d38 100644 --- a/spec/javascripts/shared/popover_spec.js +++ b/spec/javascripts/shared/popover_spec.js @@ -112,8 +112,8 @@ describe('popover', () => { length: 0, }; - spyOn($.fn, 'init').and.callFake( - selector => (selector === '.popover:hover' ? fakeJquery : $.fn), + spyOn($.fn, 'init').and.callFake(selector => + selector === '.popover:hover' ? fakeJquery : $.fn, ); spyOn(togglePopover, 'call'); mouseleave(); @@ -126,8 +126,8 @@ describe('popover', () => { length: 1, }; - spyOn($.fn, 'init').and.callFake( - selector => (selector === '.popover:hover' ? fakeJquery : $.fn), + spyOn($.fn, 'init').and.callFake(selector => + selector === '.popover:hover' ? fakeJquery : $.fn, ); spyOn(togglePopover, 'call'); mouseleave(); diff --git a/spec/javascripts/signin_tabs_memoizer_spec.js b/spec/javascripts/signin_tabs_memoizer_spec.js index b688a299052..52da6a79939 100644 --- a/spec/javascripts/signin_tabs_memoizer_spec.js +++ b/spec/javascripts/signin_tabs_memoizer_spec.js @@ -51,8 +51,8 @@ describe('SigninTabsMemoizer', () => { const fakeTab = { click: () => {}, }; - spyOn(document, 'querySelector').and.callFake( - selector => (selector === `${tabSelector} a[href="#bogus"]` ? null : fakeTab), + spyOn(document, 'querySelector').and.callFake(selector => + selector === `${tabSelector} a[href="#bogus"]` ? null : fakeTab, ); spyOn(fakeTab, 'click'); diff --git a/spec/javascripts/vue_mr_widget/components/deployment_spec.js b/spec/javascripts/vue_mr_widget/components/deployment_spec.js index ebbcaeb6f30..056b4df8fdc 100644 --- a/spec/javascripts/vue_mr_widget/components/deployment_spec.js +++ b/spec/javascripts/vue_mr_widget/components/deployment_spec.js @@ -41,7 +41,7 @@ describe('Deployment component', () => { describe('', () => { beforeEach(() => { - vm = mountComponent(Component, { deployment: { ...deploymentMockData } }); + vm = mountComponent(Component, { deployment: { ...deploymentMockData }, showMetrics: true }); }); describe('deployTimeago', () => { @@ -174,11 +174,31 @@ describe('Deployment component', () => { }); }); + describe('with showMetrics enabled', () => { + beforeEach(() => { + vm = mountComponent(Component, { deployment: { ...deploymentMockData }, showMetrics: true }); + }); + + it('shows metrics', () => { + expect(vm.$el).toContainElement('.js-mr-memory-usage'); + }); + }); + + describe('with showMetrics disabled', () => { + beforeEach(() => { + vm = mountComponent(Component, { deployment: { ...deploymentMockData }, showMetrics: false }); + }); + + it('hides metrics', () => { + expect(vm.$el).not.toContainElement('.js-mr-memory-usage'); + }); + }); + describe('without changes', () => { beforeEach(() => { delete deploymentMockData.changes; - vm = mountComponent(Component, { deployment: { ...deploymentMockData } }); + vm = mountComponent(Component, { deployment: { ...deploymentMockData }, showMetrics: true }); }); it('renders the link to the review app without dropdown', () => { @@ -192,6 +212,7 @@ describe('Deployment component', () => { beforeEach(() => { vm = mountComponent(Component, { deployment: Object.assign({}, deploymentMockData, { status: 'running' }), + showMetrics: true, }); }); @@ -208,6 +229,7 @@ describe('Deployment component', () => { beforeEach(() => { vm = mountComponent(Component, { deployment: Object.assign({}, deploymentMockData, { status: 'success' }), + showMetrics: true, }); }); @@ -220,6 +242,7 @@ describe('Deployment component', () => { beforeEach(() => { vm = mountComponent(Component, { deployment: Object.assign({}, deploymentMockData, { status: 'failed' }), + showMetrics: true, }); }); diff --git a/spec/javascripts/vue_mr_widget/components/mr_widget_rebase_spec.js b/spec/javascripts/vue_mr_widget/components/mr_widget_rebase_spec.js index 14d6e8d7556..300133dc602 100644 --- a/spec/javascripts/vue_mr_widget/components/mr_widget_rebase_spec.js +++ b/spec/javascripts/vue_mr_widget/components/mr_widget_rebase_spec.js @@ -44,7 +44,10 @@ describe('Merge request widget rebase component', () => { .textContent.trim(); expect(text).toContain('Fast-forward merge is not possible.'); - expect(text).toContain('Rebase the source branch onto the target branch or merge target'); + expect(text.replace(/\s\s+/g, ' ')).toContain( + 'Rebase the source branch onto the target branch or merge target', + ); + expect(text).toContain('branch into source branch to allow this merge request to be merged.'); }); @@ -78,7 +81,7 @@ describe('Merge request widget rebase component', () => { expect(text).toContain('Fast-forward merge is not possible.'); expect(text).toContain('Rebase the source branch onto'); expect(text).toContain('foo'); - expect(text).toContain('to allow this merge request to be merged.'); + expect(text.replace(/\s\s+/g, ' ')).toContain('to allow this merge request to be merged.'); }); }); diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_missing_branch_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_missing_branch_spec.js index 096301837c4..5fd8093bf5c 100644 --- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_missing_branch_spec.js +++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_missing_branch_spec.js @@ -33,7 +33,7 @@ describe('MRWidgetMissingBranch', () => { expect(el.classList.contains('mr-widget-body')).toBeTruthy(); expect(el.querySelector('button').getAttribute('disabled')).toBeTruthy(); - expect(content).toContain('source branch does not exist.'); + expect(content.replace(/\s\s+/g, ' ')).toContain('source branch does not exist.'); expect(content).toContain('Please restore it or use a different source branch'); }); }); diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_nothing_to_merge_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_nothing_to_merge_spec.js index babb8cea0ab..bd0bd36ebc2 100644 --- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_nothing_to_merge_spec.js +++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_nothing_to_merge_spec.js @@ -19,7 +19,9 @@ describe('NothingToMerge', () => { "Currently there are no changes in this merge request's source branch", ); - expect(vm.$el.innerText).toContain('Please push new commits or use a different branch.'); + expect(vm.$el.innerText.replace(/\s\s+/g, ' ')).toContain( + 'Please push new commits or use a different branch.', + ); }); it('should not show new blob link if there is no link available', () => { diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_wip_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_wip_spec.js index 88937df2f7b..7b1d589dcf8 100644 --- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_wip_spec.js +++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_wip_spec.js @@ -85,7 +85,9 @@ describe('Wip', () => { expect(el.innerText).toContain('This is a Work in Progress'); expect(el.querySelector('button').getAttribute('disabled')).toBeTruthy(); expect(el.querySelector('button').innerText).toContain('Merge'); - expect(el.querySelector('.js-remove-wip').innerText).toContain('Resolve WIP status'); + expect(el.querySelector('.js-remove-wip').innerText.replace(/\s\s+/g, ' ')).toContain( + 'Resolve WIP status', + ); }); it('should not show removeWIP button is user cannot update MR', done => { diff --git a/spec/javascripts/vue_shared/components/diff_viewer/viewers/image_diff_viewer_spec.js b/spec/javascripts/vue_shared/components/diff_viewer/viewers/image_diff_viewer_spec.js index 2d3e178d249..7f2e246d656 100644 --- a/spec/javascripts/vue_shared/components/diff_viewer/viewers/image_diff_viewer_spec.js +++ b/spec/javascripts/vue_shared/components/diff_viewer/viewers/image_diff_viewer_spec.js @@ -97,6 +97,26 @@ describe('ImageDiffViewer', () => { }); }); + it('renders image diff for renamed', done => { + vm = new Vue({ + components: { + imageDiffViewer, + }, + template: ` + <image-diff-viewer diff-mode="renamed" new-path="${GREEN_BOX_IMAGE_URL}" old-path=""> + <span slot="image-overlay" class="overlay">test</span> + </image-diff-viewer> + `, + }).$mount(); + + setTimeout(() => { + expect(vm.$el.querySelector('img').getAttribute('src')).toBe(GREEN_BOX_IMAGE_URL); + expect(vm.$el.querySelector('.overlay')).not.toBe(null); + + done(); + }); + }); + describe('swipeMode', () => { beforeEach(done => { createComponent({ diff --git a/spec/javascripts/vue_shared/components/expand_button_spec.js b/spec/javascripts/vue_shared/components/expand_button_spec.js index 98fee9a74a5..2af4abc299a 100644 --- a/spec/javascripts/vue_shared/components/expand_button_spec.js +++ b/spec/javascripts/vue_shared/components/expand_button_spec.js @@ -18,7 +18,7 @@ describe('expand button', () => { vm.$destroy(); }); - it('renders a collpased button', () => { + it('renders a collapsed button', () => { expect(vm.$children[0].iconTestClass).toEqual('ic-ellipsis_h'); }); diff --git a/spec/lib/gitlab/auth/saml/auth_hash_spec.rb b/spec/lib/gitlab/auth/saml/auth_hash_spec.rb index 76f49e778fb..3620e1afe25 100644 --- a/spec/lib/gitlab/auth/saml/auth_hash_spec.rb +++ b/spec/lib/gitlab/auth/saml/auth_hash_spec.rb @@ -82,6 +82,17 @@ describe Gitlab::Auth::Saml::AuthHash do end end + context 'with SAML 2.0 response_object' do + before do + auth_hash_data[:extra][:response_object] = { document: + saml_xml(File.read('spec/fixtures/authentication/saml2_response.xml')) } + end + + it 'can extract authn_context' do + expect(saml_auth_hash.authn_context).to eq 'urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport' + end + end + context 'without response_object' do it 'returns an empty string' do expect(saml_auth_hash.authn_context).to be_nil diff --git a/spec/lib/gitlab/ci/build/policy/changes_spec.rb b/spec/lib/gitlab/ci/build/policy/changes_spec.rb index ab401108c84..523d00c1272 100644 --- a/spec/lib/gitlab/ci/build/policy/changes_spec.rb +++ b/spec/lib/gitlab/ci/build/policy/changes_spec.rb @@ -49,6 +49,12 @@ describe Gitlab::Ci::Build::Policy::Changes do expect(policy).to be_satisfied_by(pipeline, seed) end + it 'is satisfied by matching a pattern with a glob' do + policy = described_class.new(%w[some/**/*.{rb,txt}]) + + expect(policy).to be_satisfied_by(pipeline, seed) + end + it 'is not satisfied when pattern does not match path' do policy = described_class.new(%w[some/*.rb]) @@ -61,6 +67,12 @@ describe Gitlab::Ci::Build::Policy::Changes do expect(policy).not_to be_satisfied_by(pipeline, seed) end + it 'is not satified when pattern with glob does not match' do + policy = described_class.new(%w[invalid/*.{md,rake}]) + + expect(policy).not_to be_satisfied_by(pipeline, seed) + end + context 'when pipelines does not run for a branch update' do before do pipeline.before_sha = Gitlab::Git::BLANK_SHA diff --git a/spec/lib/gitlab/ci/build/policy/refs_spec.rb b/spec/lib/gitlab/ci/build/policy/refs_spec.rb index 7211187e511..553fc0fb9bf 100644 --- a/spec/lib/gitlab/ci/build/policy/refs_spec.rb +++ b/spec/lib/gitlab/ci/build/policy/refs_spec.rb @@ -16,7 +16,7 @@ describe Gitlab::Ci::Build::Policy::Refs do end end - context 'when maching tags' do + context 'when matching tags' do context 'when pipeline runs for a tag' do let(:pipeline) do build_stubbed(:ci_pipeline, ref: 'feature', tag: true) @@ -56,10 +56,10 @@ describe Gitlab::Ci::Build::Policy::Refs do end end - context 'when maching a source' do + context 'when matching a source' do let(:pipeline) { build_stubbed(:ci_pipeline, source: :push) } - it 'is satisifed when provided source keyword matches' do + it 'is satisfied when provided source keyword matches' do expect(described_class.new(%w[pushes])) .to be_satisfied_by(pipeline) end diff --git a/spec/lib/gitlab/ci/config/entry/artifacts_spec.rb b/spec/lib/gitlab/ci/config/entry/artifacts_spec.rb index d48aac15f28..bd1f2c92844 100644 --- a/spec/lib/gitlab/ci/config/entry/artifacts_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/artifacts_spec.rb @@ -8,7 +8,7 @@ describe Gitlab::Ci::Config::Entry::Artifacts do let(:config) { { paths: %w[public/] } } describe '#value' do - it 'returns artifacs configuration' do + it 'returns artifacts configuration' do expect(entry.value).to eq config end end diff --git a/spec/lib/gitlab/ci/config/entry/policy_spec.rb b/spec/lib/gitlab/ci/config/entry/policy_spec.rb index bef93fe7af7..83001b7fdd8 100644 --- a/spec/lib/gitlab/ci/config/entry/policy_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/policy_spec.rb @@ -58,7 +58,7 @@ describe Gitlab::Ci::Config::Entry::Policy do end context 'when using complex policy' do - context 'when specifiying refs policy' do + context 'when specifying refs policy' do let(:config) { { refs: ['master'] } } it 'is a correct configuraton' do diff --git a/spec/lib/gitlab/ci/config/entry/reports_spec.rb b/spec/lib/gitlab/ci/config/entry/reports_spec.rb index 1140bfdf6c3..38943138cbf 100644 --- a/spec/lib/gitlab/ci/config/entry/reports_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/reports_spec.rb @@ -19,7 +19,7 @@ describe Gitlab::Ci::Config::Entry::Reports do shared_examples 'a valid entry' do |keyword, file| describe '#value' do - it 'returns artifacs configuration' do + it 'returns artifacts configuration' do expect(entry.value).to eq({ "#{keyword}": [file] } ) end end diff --git a/spec/lib/gitlab/ci/variables/collection/item_spec.rb b/spec/lib/gitlab/ci/variables/collection/item_spec.rb index 46874662edd..e1e0582cd11 100644 --- a/spec/lib/gitlab/ci/variables/collection/item_spec.rb +++ b/spec/lib/gitlab/ci/variables/collection/item_spec.rb @@ -36,7 +36,7 @@ describe Gitlab::Ci::Variables::Collection::Item do shared_examples 'raises error for invalid type' do it do expect { described_class.new(key: variable_key, value: variable_value) } - .to raise_error ArgumentError, /`value` must be of type String, while it was:/ + .to raise_error ArgumentError, /`#{variable_key}` must be of type String, while it was:/ end end @@ -46,7 +46,7 @@ describe Gitlab::Ci::Variables::Collection::Item do let(:variable_value) { nil } let(:expected_value) { nil } - it_behaves_like 'creates variable' + it_behaves_like 'raises error for invalid type' end context "when it's an empty string" do diff --git a/spec/lib/gitlab/database/migration_helpers_spec.rb b/spec/lib/gitlab/database/migration_helpers_spec.rb index 4e83b27e4a5..23f27939dd2 100644 --- a/spec/lib/gitlab/database/migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migration_helpers_spec.rb @@ -1338,7 +1338,12 @@ describe Gitlab::Database::MigrationHelpers do end describe '#index_exists_by_name?' do - it 'returns true if an index exists' do + # TODO: remove rails5-only after removing rails4 tests + # rails 4 can not handle multiple indexes on the same column set if + # index was added by 't.index' - t.index is used by default in schema.rb in + # rails 5. Let's run this test only in rails 5 env: + # see https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/21492#note_113602758 + it 'returns true if an index exists', :rails5 do expect(model.index_exists_by_name?(:projects, 'index_projects_on_path')) .to be_truthy end diff --git a/spec/lib/gitlab/file_detector_spec.rb b/spec/lib/gitlab/file_detector_spec.rb index edab53247e9..4ba9094b24e 100644 --- a/spec/lib/gitlab/file_detector_spec.rb +++ b/spec/lib/gitlab/file_detector_spec.rb @@ -15,14 +15,22 @@ describe Gitlab::FileDetector do describe '.type_of' do it 'returns the type of a README file' do - %w[README readme INDEX index].each do |filename| + filenames = Gitlab::MarkupHelper::PLAIN_FILENAMES + Gitlab::MarkupHelper::PLAIN_FILENAMES.map(&:upcase) + extensions = Gitlab::MarkupHelper::EXTENSIONS + Gitlab::MarkupHelper::EXTENSIONS.map(&:upcase) + + filenames.each do |filename| expect(described_class.type_of(filename)).to eq(:readme) - %w[.md .adoc .rst].each do |extname| - expect(described_class.type_of(filename + extname)).to eq(:readme) + + extensions.each do |extname| + expect(described_class.type_of("#{filename}.#{extname}")).to eq(:readme) end end end + it 'returns nil for a README.rb file' do + expect(described_class.type_of('README.rb')).to be_nil + end + it 'returns nil for a README file in a directory' do expect(described_class.type_of('foo/README.md')).to be_nil end diff --git a/spec/lib/gitlab/git/commit_spec.rb b/spec/lib/gitlab/git/commit_spec.rb index 9ef27081f98..6be35eee0fd 100644 --- a/spec/lib/gitlab/git/commit_spec.rb +++ b/spec/lib/gitlab/git/commit_spec.rb @@ -94,7 +94,7 @@ describe Gitlab::Git::Commit, :seed_helper do context 'body_size less than threshold' do let(:body_size) { 123 } - it 'fetches commit message seperately' do + it 'fetches commit message separately' do expect(described_class).to receive(:get_message).with(repository, id) commit.safe_message diff --git a/spec/lib/gitlab/git/remote_mirror_spec.rb b/spec/lib/gitlab/git/remote_mirror_spec.rb new file mode 100644 index 00000000000..dc63eef7814 --- /dev/null +++ b/spec/lib/gitlab/git/remote_mirror_spec.rb @@ -0,0 +1,28 @@ +require 'spec_helper' + +describe Gitlab::Git::RemoteMirror do + describe '#update' do + let(:project) { create(:project, :repository) } + let(:repository) { project.repository } + let(:ref_name) { 'foo' } + let(:options) { { only_branches_matching: ['master'], ssh_key: 'KEY', known_hosts: 'KNOWN HOSTS' } } + + subject(:remote_mirror) { described_class.new(repository, ref_name, **options) } + + it 'delegates to the Gitaly client' do + expect(repository.gitaly_remote_client) + .to receive(:update_remote_mirror) + .with(ref_name, ['master'], ssh_key: 'KEY', known_hosts: 'KNOWN HOSTS') + + remote_mirror.update + end + + it 'wraps gitaly errors' do + expect(repository.gitaly_remote_client) + .to receive(:update_remote_mirror) + .and_raise(StandardError) + + expect { remote_mirror.update }.to raise_error(StandardError) + end + end +end diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 54291e847d8..1fe73c12fc0 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -1095,12 +1095,26 @@ describe Gitlab::Git::Repository, :seed_helper do end it 'returns no Gitaly::DiffStats when there is a nil SHA' do + expect_any_instance_of(Gitlab::GitalyClient::CommitService) + .not_to receive(:diff_stats) + collection = repository.diff_stats(nil, 'master') expect(collection).to be_a(Gitlab::Git::DiffStatsCollection) expect(collection).to be_a(Enumerable) expect(collection.to_a).to be_empty end + + it 'returns no Gitaly::DiffStats when there is a BLANK_SHA' do + expect_any_instance_of(Gitlab::GitalyClient::CommitService) + .not_to receive(:diff_stats) + + collection = repository.diff_stats(Gitlab::Git::BLANK_SHA, 'master') + + expect(collection).to be_a(Gitlab::Git::DiffStatsCollection) + expect(collection).to be_a(Enumerable) + expect(collection.to_a).to be_empty + end end describe "#ls_files" do diff --git a/spec/lib/gitlab/git/tag_spec.rb b/spec/lib/gitlab/git/tag_spec.rb index 2d9db576a6c..c5bad062c2a 100644 --- a/spec/lib/gitlab/git/tag_spec.rb +++ b/spec/lib/gitlab/git/tag_spec.rb @@ -68,7 +68,7 @@ describe Gitlab::Git::Tag, :seed_helper do context 'message_size less than threshold' do let(:message_size) { 123 } - it 'fetches tag message seperately' do + it 'fetches tag message separately' do expect(described_class).to receive(:get_message).with(repository, gitaly_tag.id) tag.message diff --git a/spec/lib/gitlab/gitaly_client/remote_service_spec.rb b/spec/lib/gitlab/gitaly_client/remote_service_spec.rb index 9030a49983d..aff47599ad6 100644 --- a/spec/lib/gitlab/gitaly_client/remote_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/remote_service_spec.rb @@ -68,6 +68,8 @@ describe Gitlab::GitalyClient::RemoteService do describe '#update_remote_mirror' do let(:ref_name) { 'remote_mirror_1' } let(:only_branches_matching) { ['my-branch', 'master'] } + let(:ssh_key) { 'KEY' } + let(:known_hosts) { 'KNOWN HOSTS' } it 'sends an update_remote_mirror message' do expect_any_instance_of(Gitaly::RemoteService::Stub) @@ -75,7 +77,7 @@ describe Gitlab::GitalyClient::RemoteService do .with(kind_of(Enumerator), kind_of(Hash)) .and_return(double(:update_remote_mirror_response)) - client.update_remote_mirror(ref_name, only_branches_matching) + client.update_remote_mirror(ref_name, only_branches_matching, ssh_key: ssh_key, known_hosts: known_hosts) end end diff --git a/spec/lib/gitlab/gitaly_client/repository_service_spec.rb b/spec/lib/gitlab/gitaly_client/repository_service_spec.rb index d605fcbafee..46ca2340389 100644 --- a/spec/lib/gitlab/gitaly_client/repository_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/repository_service_spec.rb @@ -130,7 +130,7 @@ describe Gitlab::GitalyClient::RepositoryService do end context 'SSH auth' do - where(:ssh_import, :ssh_key_auth, :ssh_private_key, :ssh_known_hosts, :expected_params) do + where(:ssh_mirror_url, :ssh_key_auth, :ssh_private_key, :ssh_known_hosts, :expected_params) do false | false | 'key' | 'known_hosts' | {} false | true | 'key' | 'known_hosts' | {} true | false | 'key' | 'known_hosts' | { known_hosts: 'known_hosts' } @@ -145,7 +145,7 @@ describe Gitlab::GitalyClient::RepositoryService do let(:ssh_auth) do double( :ssh_auth, - ssh_import?: ssh_import, + ssh_mirror_url?: ssh_mirror_url, ssh_key_auth?: ssh_key_auth, ssh_private_key: ssh_private_key, ssh_known_hosts: ssh_known_hosts diff --git a/spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb b/spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb index 25684ea9e2c..efca8564894 100644 --- a/spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb @@ -240,7 +240,12 @@ describe Gitlab::GithubImport::Importer::PullRequestImporter, :clean_gitlab_redi .and_return(user.id) end - it 'returns the existing merge request' do + # TODO: remove rails5-only after removing rails4 tests + # rails 4 can not handle multiple indexes on the same column set if + # index was added by 't.index' - t.index is used by default in schema.rb in + # rails 5. Let's run this test only in rails 5 env: + # see https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/21492#note_113602758 + it 'returns the existing merge request', :rails5 do mr1, exists1 = importer.create_merge_request mr2, exists2 = importer.create_merge_request diff --git a/spec/lib/gitlab/import_export/project.json b/spec/lib/gitlab/import_export/project.json index 3f2281f213f..58949f76bd6 100644 --- a/spec/lib/gitlab/import_export/project.json +++ b/spec/lib/gitlab/import_export/project.json @@ -2556,7 +2556,7 @@ "merge_request_diff_id": 27, "relative_order": 0, "sha": "bb5206fee213d983da88c47f9cf4cc6caf9c66dc", - "message": "Feature conflcit added\n\nSigned-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>\n", + "message": "Feature conflict added\n\nSigned-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>\n", "authored_date": "2014-08-06T08:35:52.000+02:00", "author_name": "Dmitriy Zaporozhets", "author_email": "dmitriy.zaporozhets@gmail.com", @@ -3605,7 +3605,7 @@ "merge_request_diff_id": 14, "relative_order": 8, "sha": "08f22f255f082689c0d7d39d19205085311542bc", - "message": "remove emtpy file.(beacase git ignore empty file)\nadd whitespace test file.\n", + "message": "remove empty file.(beacase git ignore empty file)\nadd whitespace test file.\n", "authored_date": "2015-11-13T06:00:16.000+01:00", "author_name": "윤민식", "author_email": "minsik.yoon@samsung.com", @@ -4290,7 +4290,7 @@ "merge_request_diff_id": 13, "relative_order": 8, "sha": "08f22f255f082689c0d7d39d19205085311542bc", - "message": "remove emtpy file.(beacase git ignore empty file)\nadd whitespace test file.\n", + "message": "remove empty file.(beacase git ignore empty file)\nadd whitespace test file.\n", "authored_date": "2015-11-13T06:00:16.000+01:00", "author_name": "윤민식", "author_email": "minsik.yoon@samsung.com", @@ -4799,7 +4799,7 @@ "merge_request_diff_id": 12, "relative_order": 8, "sha": "08f22f255f082689c0d7d39d19205085311542bc", - "message": "remove emtpy file.(beacase git ignore empty file)\nadd whitespace test file.\n", + "message": "remove empty file.(beacase git ignore empty file)\nadd whitespace test file.\n", "authored_date": "2015-11-13T06:00:16.000+01:00", "author_name": "윤민식", "author_email": "minsik.yoon@samsung.com", @@ -5507,7 +5507,7 @@ "merge_request_diff_id": 10, "relative_order": 8, "sha": "08f22f255f082689c0d7d39d19205085311542bc", - "message": "remove emtpy file.(beacase git ignore empty file)\nadd whitespace test file.\n", + "message": "remove empty file.(beacase git ignore empty file)\nadd whitespace test file.\n", "authored_date": "2015-11-13T06:00:16.000+01:00", "author_name": "윤민식", "author_email": "minsik.yoon@samsung.com", diff --git a/spec/lib/gitlab/import_export/project.light.json b/spec/lib/gitlab/import_export/project.light.json index ba2248073f5..2971ca0f0f8 100644 --- a/spec/lib/gitlab/import_export/project.light.json +++ b/spec/lib/gitlab/import_export/project.light.json @@ -101,6 +101,28 @@ ] } ], + "services": [ + { + "id": 100, + "title": "JetBrains TeamCity CI", + "project_id": 5, + "created_at": "2016-06-14T15:01:51.315Z", + "updated_at": "2016-06-14T15:01:51.315Z", + "active": false, + "properties": {}, + "template": true, + "push_events": true, + "issues_events": true, + "merge_requests_events": true, + "tag_push_events": true, + "note_events": true, + "job_events": true, + "type": "TeamcityService", + "category": "ci", + "default": false, + "wiki_page_events": true + } + ], "snippets": [], "hooks": [] } diff --git a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb index 365bfae0d88..7171e12a849 100644 --- a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb @@ -297,7 +297,8 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do issues: 1, labels: 1, milestones: 1, - first_issue_labels: 1 + first_issue_labels: 1, + services: 1 context 'project.json file access check' do it 'does not read a symlink' do @@ -382,6 +383,12 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do project_tree_restorer.instance_variable_set(:@path, "spec/lib/gitlab/import_export/project.light.json") end + it 'does not import any templated services' do + restored_project_json + + expect(project.services.where(template: true).count).to eq(0) + end + it 'imports labels' do create(:group_label, name: 'Another label', group: project.group) diff --git a/spec/lib/gitlab/kubernetes/helm/api_spec.rb b/spec/lib/gitlab/kubernetes/helm/api_spec.rb index 9200724ed23..8bce7a4cdf5 100644 --- a/spec/lib/gitlab/kubernetes/helm/api_spec.rb +++ b/spec/lib/gitlab/kubernetes/helm/api_spec.rb @@ -36,6 +36,7 @@ describe Gitlab::Kubernetes::Helm::Api do describe '#install' do before do allow(client).to receive(:create_pod).and_return(nil) + allow(client).to receive(:get_config_map).and_return(nil) allow(client).to receive(:create_config_map).and_return(nil) allow(client).to receive(:create_service_account).and_return(nil) allow(client).to receive(:create_cluster_role_binding).and_return(nil) @@ -57,6 +58,18 @@ describe Gitlab::Kubernetes::Helm::Api do subject.install(command) end + + context 'config map already exists' do + before do + expect(client).to receive(:get_config_map).with("values-content-configuration-#{application_name}", gitlab_namespace).and_return(resource) + end + + it 'updates the config map' do + expect(client).to receive(:update_config_map).with(resource).once + + subject.install(command) + end + end end context 'without a service account' do @@ -88,8 +101,8 @@ describe Gitlab::Kubernetes::Helm::Api do context 'service account and cluster role binding does not exist' do before do - expect(client).to receive('get_service_account').with('tiller', 'gitlab-managed-apps').and_raise(Kubeclient::HttpError.new(404, 'Not found', nil)) - expect(client).to receive('get_cluster_role_binding').with('tiller-admin').and_raise(Kubeclient::HttpError.new(404, 'Not found', nil)) + expect(client).to receive(:get_service_account).with('tiller', 'gitlab-managed-apps').and_raise(Kubeclient::ResourceNotFoundError.new(404, 'Not found', nil)) + expect(client).to receive(:get_cluster_role_binding).with('tiller-admin').and_raise(Kubeclient::ResourceNotFoundError.new(404, 'Not found', nil)) end it 'creates a service account, followed the cluster role binding on kubeclient' do @@ -102,8 +115,8 @@ describe Gitlab::Kubernetes::Helm::Api do context 'service account already exists' do before do - expect(client).to receive('get_service_account').with('tiller', 'gitlab-managed-apps').and_return(service_account_resource) - expect(client).to receive('get_cluster_role_binding').with('tiller-admin').and_raise(Kubeclient::HttpError.new(404, 'Not found', nil)) + expect(client).to receive(:get_service_account).with('tiller', 'gitlab-managed-apps').and_return(service_account_resource) + expect(client).to receive(:get_cluster_role_binding).with('tiller-admin').and_raise(Kubeclient::ResourceNotFoundError.new(404, 'Not found', nil)) end it 'updates the service account, followed by creating the cluster role binding' do @@ -116,8 +129,8 @@ describe Gitlab::Kubernetes::Helm::Api do context 'service account and cluster role binding already exists' do before do - expect(client).to receive('get_service_account').with('tiller', 'gitlab-managed-apps').and_return(service_account_resource) - expect(client).to receive('get_cluster_role_binding').with('tiller-admin').and_return(cluster_role_binding_resource) + expect(client).to receive(:get_service_account).with('tiller', 'gitlab-managed-apps').and_return(service_account_resource) + expect(client).to receive(:get_cluster_role_binding).with('tiller-admin').and_return(cluster_role_binding_resource) end it 'updates the service account, followed by creating the cluster role binding' do @@ -130,7 +143,7 @@ describe Gitlab::Kubernetes::Helm::Api do context 'a non-404 error is thrown' do before do - expect(client).to receive('get_service_account').with('tiller', 'gitlab-managed-apps').and_raise(Kubeclient::HttpError.new(401, 'Unauthorized', nil)) + expect(client).to receive(:get_service_account).with('tiller', 'gitlab-managed-apps').and_raise(Kubeclient::HttpError.new(401, 'Unauthorized', nil)) end it 'raises an error' do diff --git a/spec/lib/gitlab/kubernetes/helm/install_command_spec.rb b/spec/lib/gitlab/kubernetes/helm/install_command_spec.rb index 2b7e3ea6def..39852b7fe29 100644 --- a/spec/lib/gitlab/kubernetes/helm/install_command_spec.rb +++ b/spec/lib/gitlab/kubernetes/helm/install_command_spec.rb @@ -26,7 +26,8 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do it_behaves_like 'helm commands' do let(:commands) do <<~EOS - helm init --client-only + helm init --upgrade + for i in $(seq 1 30); do helm version && break; sleep 1s; echo "Retrying ($i)..."; done helm repo add app-name https://repository.example.com helm repo update #{helm_install_comand} @@ -54,7 +55,8 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do it_behaves_like 'helm commands' do let(:commands) do <<~EOS - helm init --client-only + helm init --upgrade + for i in $(seq 1 30); do helm version && break; sleep 1s; echo "Retrying ($i)..."; done helm repo add app-name https://repository.example.com helm repo update #{helm_install_command} @@ -84,7 +86,8 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do it_behaves_like 'helm commands' do let(:commands) do <<~EOS - helm init --client-only + helm init --upgrade + for i in $(seq 1 30); do helm version && break; sleep 1s; echo "Retrying ($i)..."; done #{helm_install_command} EOS end @@ -111,7 +114,8 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do it_behaves_like 'helm commands' do let(:commands) do <<~EOS - helm init --client-only + helm init --upgrade + for i in $(seq 1 30); do helm version && break; sleep 1s; echo "Retrying ($i)..."; done helm repo add app-name https://repository.example.com helm repo update #{helm_install_command} @@ -134,7 +138,8 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do it_behaves_like 'helm commands' do let(:commands) do <<~EOS - helm init --client-only + helm init --upgrade + for i in $(seq 1 30); do helm version && break; sleep 1s; echo "Retrying ($i)..."; done helm repo add app-name https://repository.example.com helm repo update #{helm_install_command} @@ -157,7 +162,8 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do it_behaves_like 'helm commands' do let(:commands) do <<~EOS - helm init --client-only + helm init --upgrade + for i in $(seq 1 30); do helm version && break; sleep 1s; echo "Retrying ($i)..."; done helm repo add app-name https://repository.example.com helm repo update #{helm_install_command} @@ -182,7 +188,8 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do it_behaves_like 'helm commands' do let(:commands) do <<~EOS - helm init --client-only + helm init --upgrade + for i in $(seq 1 30); do helm version && break; sleep 1s; echo "Retrying ($i)..."; done helm repo add app-name https://repository.example.com helm repo update #{helm_install_command} diff --git a/spec/lib/gitlab/kubernetes/helm/pod_spec.rb b/spec/lib/gitlab/kubernetes/helm/pod_spec.rb index c92bc92c42d..2dd3a570a1d 100644 --- a/spec/lib/gitlab/kubernetes/helm/pod_spec.rb +++ b/spec/lib/gitlab/kubernetes/helm/pod_spec.rb @@ -30,7 +30,7 @@ describe Gitlab::Kubernetes::Helm::Pod do it 'should generate the appropriate specifications for the container' do container = subject.generate.spec.containers.first expect(container.name).to eq('helm') - expect(container.image).to eq('registry.gitlab.com/gitlab-org/cluster-integration/helm-install-image/releases/2.7.2-kube-1.11.0') + expect(container.image).to eq('registry.gitlab.com/gitlab-org/cluster-integration/helm-install-image/releases/2.11.0-kube-1.11.0') expect(container.env.count).to eq(3) expect(container.env.map(&:name)).to match_array([:HELM_VERSION, :TILLER_NAMESPACE, :COMMAND_SCRIPT]) expect(container.command).to match_array(["/bin/sh"]) diff --git a/spec/lib/gitlab/kubernetes/helm/upgrade_command_spec.rb b/spec/lib/gitlab/kubernetes/helm/upgrade_command_spec.rb index 9c9fc91ef3c..9b201dae417 100644 --- a/spec/lib/gitlab/kubernetes/helm/upgrade_command_spec.rb +++ b/spec/lib/gitlab/kubernetes/helm/upgrade_command_spec.rb @@ -21,7 +21,8 @@ describe Gitlab::Kubernetes::Helm::UpgradeCommand do it_behaves_like 'helm commands' do let(:commands) do <<~EOS - helm init --client-only + helm init --upgrade + for i in $(seq 1 30); do helm version && break; sleep 1s; echo "Retrying ($i)..."; done helm upgrade #{application.name} #{application.chart} --tls --tls-ca-cert /data/helm/#{application.name}/config/ca.pem --tls-cert /data/helm/#{application.name}/config/cert.pem --tls-key /data/helm/#{application.name}/config/key.pem --reset-values --install --namespace #{namespace} -f /data/helm/#{application.name}/config/values.yaml EOS end @@ -33,7 +34,8 @@ describe Gitlab::Kubernetes::Helm::UpgradeCommand do it_behaves_like 'helm commands' do let(:commands) do <<~EOS - helm init --client-only + helm init --upgrade + for i in $(seq 1 30); do helm version && break; sleep 1s; echo "Retrying ($i)..."; done helm upgrade #{application.name} #{application.chart} --tls --tls-ca-cert /data/helm/#{application.name}/config/ca.pem --tls-cert /data/helm/#{application.name}/config/cert.pem --tls-key /data/helm/#{application.name}/config/key.pem --reset-values --install --namespace #{namespace} -f /data/helm/#{application.name}/config/values.yaml EOS end @@ -56,7 +58,8 @@ describe Gitlab::Kubernetes::Helm::UpgradeCommand do it_behaves_like 'helm commands' do let(:commands) do <<~EOS - helm init --client-only + helm init --upgrade + for i in $(seq 1 30); do helm version && break; sleep 1s; echo "Retrying ($i)..."; done helm repo add #{application.name} #{application.repository} helm upgrade #{application.name} #{application.chart} --tls --tls-ca-cert /data/helm/#{application.name}/config/ca.pem --tls-cert /data/helm/#{application.name}/config/cert.pem --tls-key /data/helm/#{application.name}/config/key.pem --reset-values --install --namespace #{namespace} -f /data/helm/#{application.name}/config/values.yaml EOS @@ -70,7 +73,8 @@ describe Gitlab::Kubernetes::Helm::UpgradeCommand do it_behaves_like 'helm commands' do let(:commands) do <<~EOS - helm init --client-only + helm init --upgrade + for i in $(seq 1 30); do helm version && break; sleep 1s; echo "Retrying ($i)..."; done helm upgrade #{application.name} #{application.chart} --reset-values --install --namespace #{namespace} -f /data/helm/#{application.name}/config/values.yaml EOS end diff --git a/spec/lib/gitlab/kubernetes/kube_client_spec.rb b/spec/lib/gitlab/kubernetes/kube_client_spec.rb index eed4135d8a2..3979a43216c 100644 --- a/spec/lib/gitlab/kubernetes/kube_client_spec.rb +++ b/spec/lib/gitlab/kubernetes/kube_client_spec.rb @@ -66,6 +66,20 @@ describe Gitlab::Kubernetes::KubeClient do end end + describe '#knative_client' do + subject { client.knative_client } + + it_behaves_like 'a Kubeclient' + + it 'has the extensions API group endpoint' do + expect(subject.api_endpoint.to_s).to match(%r{\/apis\/serving.knative.dev\Z}) + end + + it 'has the api_version' do + expect(subject.instance_variable_get(:@api_version)).to eq('v1alpha1') + end + end + describe 'core API' do let(:core_client) { client.core_client } diff --git a/spec/lib/gitlab/kubernetes/namespace_spec.rb b/spec/lib/gitlab/kubernetes/namespace_spec.rb index e098612f6fb..e1c35c355f4 100644 --- a/spec/lib/gitlab/kubernetes/namespace_spec.rb +++ b/spec/lib/gitlab/kubernetes/namespace_spec.rb @@ -9,7 +9,7 @@ describe Gitlab::Kubernetes::Namespace do describe '#exists?' do context 'when namespace do not exits' do - let(:exception) { ::Kubeclient::HttpError.new(404, "namespace #{name} not found", nil) } + let(:exception) { ::Kubeclient::ResourceNotFoundError.new(404, "namespace #{name} not found", nil) } it 'returns false' do expect(client).to receive(:get_namespace).with(name).once.and_raise(exception) diff --git a/spec/lib/gitlab/private_commit_email_spec.rb b/spec/lib/gitlab/private_commit_email_spec.rb index bc86cd3842a..10bf624bbdd 100644 --- a/spec/lib/gitlab/private_commit_email_spec.rb +++ b/spec/lib/gitlab/private_commit_email_spec.rb @@ -4,6 +4,9 @@ require 'spec_helper' describe Gitlab::PrivateCommitEmail do let(:hostname) { Gitlab::CurrentSettings.current_application_settings.commit_email_hostname } + let(:id) { 1 } + let(:valid_email) { "#{id}-foo@#{hostname}" } + let(:invalid_email) { "#{id}-foo@users.noreply.bar.com" } context '.regex' do subject { described_class.regex } @@ -16,18 +19,25 @@ describe Gitlab::PrivateCommitEmail do end context '.user_id_for_email' do - let(:id) { 1 } - it 'parses user id from email' do - email = "#{id}-foo@#{hostname}" - - expect(described_class.user_id_for_email(email)).to eq(id) + expect(described_class.user_id_for_email(valid_email)).to eq(id) end it 'returns nil on invalid commit email' do - email = "#{id}-foo@users.noreply.bar.com" + expect(described_class.user_id_for_email(invalid_email)).to be_nil + end + end + + context '.user_ids_for_email' do + it 'returns deduplicated user IDs for each valid email' do + result = described_class.user_ids_for_emails([valid_email, valid_email, invalid_email]) + + expect(result).to eq([id]) + end - expect(described_class.user_id_for_email(email)).to be_nil + it 'returns an empty array with no valid emails' do + result = described_class.user_ids_for_emails([invalid_email]) + expect(result).to eq([]) end end diff --git a/spec/lib/gitlab/repository_cache_spec.rb b/spec/lib/gitlab/repository_cache_spec.rb index 741ee12633f..1b9a8b4ab0d 100644 --- a/spec/lib/gitlab/repository_cache_spec.rb +++ b/spec/lib/gitlab/repository_cache_spec.rb @@ -4,14 +4,14 @@ describe Gitlab::RepositoryCache do let(:backend) { double('backend').as_null_object } let(:project) { create(:project) } let(:repository) { project.repository } - let(:namespace) { "#{repository.full_path}:#{project.id}" } + let(:namespace) { "project:#{project.id}" } let(:cache) { described_class.new(repository, backend: backend) } describe '#cache_key' do subject { cache.cache_key(:foo) } it 'includes the namespace' do - expect(subject).to eq "foo:#{namespace}" + expect(subject).to eq "#{namespace}:foo" end context 'with a given namespace' do @@ -22,7 +22,7 @@ describe Gitlab::RepositoryCache do end it 'includes the full namespace' do - expect(subject).to eq "foo:#{namespace}:#{extra_namespace}" + expect(subject).to eq "#{namespace}:#{extra_namespace}:foo" end end end @@ -30,21 +30,21 @@ describe Gitlab::RepositoryCache do describe '#expire' do it 'expires the given key from the cache' do cache.expire(:foo) - expect(backend).to have_received(:delete).with("foo:#{namespace}") + expect(backend).to have_received(:delete).with("#{namespace}:foo") end end describe '#fetch' do it 'fetches the given key from the cache' do cache.fetch(:bar) - expect(backend).to have_received(:fetch).with("bar:#{namespace}") + expect(backend).to have_received(:fetch).with("#{namespace}:bar") end it 'accepts a block' do p = -> {} cache.fetch(:baz, &p) - expect(backend).to have_received(:fetch).with("baz:#{namespace}", &p) + expect(backend).to have_received(:fetch).with("#{namespace}:baz", &p) end end @@ -67,7 +67,7 @@ describe Gitlab::RepositoryCache do end it 'caches the value' do - expect(backend).to receive(:write).with("#{key}:#{namespace}", true) + expect(backend).to receive(:write).with("#{namespace}:#{key}", true) cache.fetch_without_caching_false(key) { true } end @@ -83,7 +83,7 @@ describe Gitlab::RepositoryCache do end it 'does not cache the value' do - expect(backend).not_to receive(:write).with("#{key}:#{namespace}", true) + expect(backend).not_to receive(:write).with("#{namespace}:#{key}", true) cache.fetch_without_caching_false(key, &p) end @@ -92,7 +92,7 @@ describe Gitlab::RepositoryCache do context 'when the cached value is truthy' do before do - backend.write("#{key}:#{namespace}", true) + backend.write("#{namespace}:#{key}", true) end it 'returns the cached value' do @@ -116,7 +116,7 @@ describe Gitlab::RepositoryCache do context 'when the cached value is falsey' do before do - backend.write("#{key}:#{namespace}", false) + backend.write("#{namespace}:#{key}", false) end it 'returns the result of the block' do @@ -126,7 +126,7 @@ describe Gitlab::RepositoryCache do end it 'writes the truthy value to the cache' do - expect(backend).to receive(:write).with("#{key}:#{namespace}", 'block result') + expect(backend).to receive(:write).with("#{namespace}:#{key}", 'block result') cache.fetch_without_caching_false(key) { 'block result' } end diff --git a/spec/lib/gitlab/sentry_spec.rb b/spec/lib/gitlab/sentry_spec.rb index 499757da061..d3b41b27b80 100644 --- a/spec/lib/gitlab/sentry_spec.rb +++ b/spec/lib/gitlab/sentry_spec.rb @@ -52,4 +52,28 @@ describe Gitlab::Sentry do end end end + + context '.track_acceptable_exception' do + let(:exception) { RuntimeError.new('boom') } + + before do + allow(described_class).to receive(:enabled?).and_return(true) + end + + it 'calls Raven.capture_exception' do + expected_extras = { + some_other_info: 'info', + issue_url: 'http://gitlab.com/gitlab-org/gitlab-ce/issues/1' + } + + expect(Raven).to receive(:capture_exception) + .with(exception, extra: a_hash_including(expected_extras)) + + described_class.track_acceptable_exception( + exception, + issue_url: 'http://gitlab.com/gitlab-org/gitlab-ce/issues/1', + extra: { some_other_info: 'info' } + ) + end + end end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index d32f3a0cead..49b3d6ad959 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -1910,12 +1910,26 @@ describe Ci::Build do describe '#repo_url' do subject { build.repo_url } - it { is_expected.to be_a(String) } - it { is_expected.to end_with(".git") } - it { is_expected.to start_with(project.web_url[0..6]) } - it { is_expected.to include(build.token) } - it { is_expected.to include('gitlab-ci-token') } - it { is_expected.to include(project.web_url[7..-1]) } + context 'when token is set' do + before do + build.ensure_token + end + + it { is_expected.to be_a(String) } + it { is_expected.to end_with(".git") } + it { is_expected.to start_with(project.web_url[0..6]) } + it { is_expected.to include(build.token) } + it { is_expected.to include('gitlab-ci-token') } + it { is_expected.to include(project.web_url[7..-1]) } + end + + context 'when token is empty' do + before do + build.token = nil + end + + it { is_expected.to be_nil} + end end describe '#stuck?' do @@ -2025,7 +2039,8 @@ describe Ci::Build do end context 'use from gitlab-ci.yml' do - let(:pipeline) { create(:ci_pipeline) } + let(:project) { create(:project, :repository) } + let(:pipeline) { create(:ci_pipeline, project: project) } before do stub_ci_pipeline_yaml_file(config) @@ -2067,56 +2082,6 @@ describe Ci::Build do describe '#variables' do let(:container_registry_enabled) { false } - let(:gitlab_version_info) { Gitlab::VersionInfo.parse(Gitlab::VERSION) } - let(:predefined_variables) do - [ - { key: 'CI_PIPELINE_ID', value: pipeline.id.to_s, public: true }, - { key: 'CI_PIPELINE_URL', value: project.web_url + "/pipelines/#{pipeline.id}", public: true }, - { key: 'CI_JOB_ID', value: build.id.to_s, public: true }, - { key: 'CI_JOB_URL', value: project.web_url + "/-/jobs/#{build.id}", public: true }, - { key: 'CI_JOB_TOKEN', value: build.token, public: false }, - { key: 'CI_BUILD_ID', value: build.id.to_s, public: true }, - { key: 'CI_BUILD_TOKEN', value: build.token, public: false }, - { key: 'CI_REGISTRY_USER', value: 'gitlab-ci-token', public: true }, - { key: 'CI_REGISTRY_PASSWORD', value: build.token, public: false }, - { key: 'CI_REPOSITORY_URL', value: build.repo_url, public: false }, - { key: 'CI', value: 'true', public: true }, - { key: 'GITLAB_CI', value: 'true', public: true }, - { key: 'GITLAB_FEATURES', value: project.licensed_features.join(','), public: true }, - { key: 'CI_SERVER_NAME', value: 'GitLab', public: true }, - { key: 'CI_SERVER_VERSION', value: Gitlab::VERSION, public: true }, - { key: 'CI_SERVER_VERSION_MAJOR', value: gitlab_version_info.major.to_s, public: true }, - { key: 'CI_SERVER_VERSION_MINOR', value: gitlab_version_info.minor.to_s, public: true }, - { key: 'CI_SERVER_VERSION_PATCH', value: gitlab_version_info.patch.to_s, public: true }, - { key: 'CI_SERVER_REVISION', value: Gitlab.revision, public: true }, - { key: 'CI_JOB_NAME', value: 'test', public: true }, - { key: 'CI_JOB_STAGE', value: 'test', public: true }, - { key: 'CI_COMMIT_SHA', value: build.sha, public: true }, - { key: 'CI_COMMIT_BEFORE_SHA', value: build.before_sha, public: true }, - { key: 'CI_COMMIT_REF_NAME', value: build.ref, public: true }, - { key: 'CI_COMMIT_REF_SLUG', value: build.ref_slug, public: true }, - { key: 'CI_NODE_TOTAL', value: '1', public: true }, - { key: 'CI_BUILD_REF', value: build.sha, public: true }, - { key: 'CI_BUILD_BEFORE_SHA', value: build.before_sha, public: true }, - { key: 'CI_BUILD_REF_NAME', value: build.ref, public: true }, - { key: 'CI_BUILD_REF_SLUG', value: build.ref_slug, public: true }, - { key: 'CI_BUILD_NAME', value: 'test', public: true }, - { key: 'CI_BUILD_STAGE', value: 'test', public: true }, - { key: 'CI_PROJECT_ID', value: project.id.to_s, public: true }, - { key: 'CI_PROJECT_NAME', value: project.path, public: true }, - { key: 'CI_PROJECT_PATH', value: project.full_path, public: true }, - { key: 'CI_PROJECT_PATH_SLUG', value: project.full_path_slug, public: true }, - { key: 'CI_PROJECT_NAMESPACE', value: project.namespace.full_path, public: true }, - { key: 'CI_PROJECT_URL', value: project.web_url, public: true }, - { key: 'CI_PROJECT_VISIBILITY', value: 'private', public: true }, - { key: 'CI_PIPELINE_IID', value: pipeline.iid.to_s, public: true }, - { key: 'CI_CONFIG_PATH', value: pipeline.ci_yaml_file_path, public: true }, - { key: 'CI_PIPELINE_SOURCE', value: pipeline.source, public: true }, - { key: 'CI_COMMIT_MESSAGE', value: pipeline.git_commit_message, public: true }, - { key: 'CI_COMMIT_TITLE', value: pipeline.git_commit_title, public: true }, - { key: 'CI_COMMIT_DESCRIPTION', value: pipeline.git_commit_description, public: true } - ] - end before do stub_container_registry_config(enabled: container_registry_enabled, host_port: 'registry.example.com') @@ -2125,11 +2090,174 @@ describe Ci::Build do subject { build.variables } context 'returns variables' do + let(:predefined_variables) do + [ + { key: 'CI_PIPELINE_ID', value: pipeline.id.to_s, public: true }, + { key: 'CI_PIPELINE_URL', value: project.web_url + "/pipelines/#{pipeline.id}", public: true }, + { key: 'CI_JOB_ID', value: build.id.to_s, public: true }, + { key: 'CI_JOB_URL', value: project.web_url + "/-/jobs/#{build.id}", public: true }, + { key: 'CI_JOB_TOKEN', value: 'my-token', public: false }, + { key: 'CI_BUILD_ID', value: build.id.to_s, public: true }, + { key: 'CI_BUILD_TOKEN', value: 'my-token', public: false }, + { key: 'CI_REGISTRY_USER', value: 'gitlab-ci-token', public: true }, + { key: 'CI_REGISTRY_PASSWORD', value: 'my-token', public: false }, + { key: 'CI_REPOSITORY_URL', value: build.repo_url, public: false }, + { key: 'CI', value: 'true', public: true }, + { key: 'GITLAB_CI', value: 'true', public: true }, + { key: 'GITLAB_FEATURES', value: project.licensed_features.join(','), public: true }, + { key: 'CI_SERVER_NAME', value: 'GitLab', public: true }, + { key: 'CI_SERVER_VERSION', value: Gitlab::VERSION, public: true }, + { key: 'CI_SERVER_VERSION_MAJOR', value: Gitlab.version_info.major.to_s, public: true }, + { key: 'CI_SERVER_VERSION_MINOR', value: Gitlab.version_info.minor.to_s, public: true }, + { key: 'CI_SERVER_VERSION_PATCH', value: Gitlab.version_info.patch.to_s, public: true }, + { key: 'CI_SERVER_REVISION', value: Gitlab.revision, public: true }, + { key: 'CI_JOB_NAME', value: 'test', public: true }, + { key: 'CI_JOB_STAGE', value: 'test', public: true }, + { key: 'CI_COMMIT_SHA', value: build.sha, public: true }, + { key: 'CI_COMMIT_BEFORE_SHA', value: build.before_sha, public: true }, + { key: 'CI_COMMIT_REF_NAME', value: build.ref, public: true }, + { key: 'CI_COMMIT_REF_SLUG', value: build.ref_slug, public: true }, + { key: 'CI_NODE_TOTAL', value: '1', public: true }, + { key: 'CI_BUILD_REF', value: build.sha, public: true }, + { key: 'CI_BUILD_BEFORE_SHA', value: build.before_sha, public: true }, + { key: 'CI_BUILD_REF_NAME', value: build.ref, public: true }, + { key: 'CI_BUILD_REF_SLUG', value: build.ref_slug, public: true }, + { key: 'CI_BUILD_NAME', value: 'test', public: true }, + { key: 'CI_BUILD_STAGE', value: 'test', public: true }, + { key: 'CI_PROJECT_ID', value: project.id.to_s, public: true }, + { key: 'CI_PROJECT_NAME', value: project.path, public: true }, + { key: 'CI_PROJECT_PATH', value: project.full_path, public: true }, + { key: 'CI_PROJECT_PATH_SLUG', value: project.full_path_slug, public: true }, + { key: 'CI_PROJECT_NAMESPACE', value: project.namespace.full_path, public: true }, + { key: 'CI_PROJECT_URL', value: project.web_url, public: true }, + { key: 'CI_PROJECT_VISIBILITY', value: 'private', public: true }, + { key: 'CI_PIPELINE_IID', value: pipeline.iid.to_s, public: true }, + { key: 'CI_CONFIG_PATH', value: pipeline.ci_yaml_file_path, public: true }, + { key: 'CI_PIPELINE_SOURCE', value: pipeline.source, public: true }, + { key: 'CI_COMMIT_MESSAGE', value: pipeline.git_commit_message, public: true }, + { key: 'CI_COMMIT_TITLE', value: pipeline.git_commit_title, public: true }, + { key: 'CI_COMMIT_DESCRIPTION', value: pipeline.git_commit_description, public: true } + ] + end + before do + build.token = 'my-token' build.yaml_variables = [] end it { is_expected.to include(*predefined_variables) } + + context 'when yaml variables are undefined' do + let(:pipeline) do + create(:ci_pipeline, project: project, + sha: project.commit.id, + ref: project.default_branch) + end + + before do + build.yaml_variables = nil + end + + context 'use from gitlab-ci.yml' do + before do + stub_ci_pipeline_yaml_file(config) + end + + context 'when config is not found' do + let(:config) { nil } + + it { is_expected.to include(*predefined_variables) } + end + + context 'when config does not have a questioned job' do + let(:config) do + YAML.dump({ + test_other: { + script: 'Hello World' + } + }) + end + + it { is_expected.to include(*predefined_variables) } + end + + context 'when config has variables' do + let(:config) do + YAML.dump({ + test: { + script: 'Hello World', + variables: { + KEY: 'value' + } + } + }) + end + + let(:variables) do + [{ key: 'KEY', value: 'value', public: true }] + end + + it { is_expected.to include(*predefined_variables) } + it { is_expected.to include(*variables) } + end + end + end + + describe 'variables ordering' do + context 'when variables hierarchy is stubbed' do + let(:build_pre_var) { { key: 'build', value: 'value', public: true } } + let(:project_pre_var) { { key: 'project', value: 'value', public: true } } + let(:pipeline_pre_var) { { key: 'pipeline', value: 'value', public: true } } + let(:build_yaml_var) { { key: 'yaml', value: 'value', public: true } } + + before do + allow(build).to receive(:predefined_variables) { [build_pre_var] } + allow(build).to receive(:yaml_variables) { [build_yaml_var] } + allow(build).to receive(:persisted_variables) { [] } + + allow_any_instance_of(Project) + .to receive(:predefined_variables) { [project_pre_var] } + + project.variables.create!(key: 'secret', value: 'value') + + allow_any_instance_of(Ci::Pipeline) + .to receive(:predefined_variables) { [pipeline_pre_var] } + end + + it 'returns variables in order depending on resource hierarchy' do + is_expected.to eq( + [build_pre_var, + project_pre_var, + pipeline_pre_var, + build_yaml_var, + { key: 'secret', value: 'value', public: false }]) + end + end + + context 'when build has environment and user-provided variables' do + let(:expected_variables) do + predefined_variables.map { |variable| variable.fetch(:key) } + + %w[YAML_VARIABLE CI_ENVIRONMENT_NAME CI_ENVIRONMENT_SLUG + CI_ENVIRONMENT_URL] + end + + before do + create(:environment, project: build.project, + name: 'staging') + + build.yaml_variables = [{ key: 'YAML_VARIABLE', + value: 'var', + public: true }] + build.environment = 'staging' + end + + it 'matches explicit variables ordering' do + received_variables = subject.map { |variable| variable.fetch(:key) } + + expect(received_variables).to eq expected_variables + end + end + end end context 'when build has user' do @@ -2391,75 +2519,20 @@ describe Ci::Build do end before do - pipeline_schedule.pipelines << pipeline + pipeline_schedule.pipelines << pipeline.reload pipeline_schedule.reload end it { is_expected.to include(pipeline_schedule_variable.to_runner_variable) } end - context 'when yaml_variables are undefined' do - let(:pipeline) do - create(:ci_pipeline, project: project, - sha: project.commit.id, - ref: project.default_branch) - end - - before do - build.yaml_variables = nil - end - - context 'use from gitlab-ci.yml' do - before do - stub_ci_pipeline_yaml_file(config) - end - - context 'when config is not found' do - let(:config) { nil } - - it { is_expected.to include(*predefined_variables) } - end - - context 'when config does not have a questioned job' do - let(:config) do - YAML.dump({ - test_other: { - script: 'Hello World' - } - }) - end - - it { is_expected.to include(*predefined_variables) } - end - - context 'when config has variables' do - let(:config) do - YAML.dump({ - test: { - script: 'Hello World', - variables: { - KEY: 'value' - } - } - }) - end - let(:variables) do - [{ key: 'KEY', value: 'value', public: true }] - end - - it { is_expected.to include(*predefined_variables) } - it { is_expected.to include(*variables) } - end - end - end - context 'when container registry is enabled' do let(:container_registry_enabled) { true } let(:ci_registry) do - { key: 'CI_REGISTRY', value: 'registry.example.com', public: true } + { key: 'CI_REGISTRY', value: 'registry.example.com', public: true } end let(:ci_registry_image) do - { key: 'CI_REGISTRY_IMAGE', value: project.container_registry_url, public: true } + { key: 'CI_REGISTRY_IMAGE', value: project.container_registry_url, public: true } end context 'and is disabled for project' do @@ -2580,66 +2653,6 @@ describe Ci::Build do end end - describe 'variables ordering' do - context 'when variables hierarchy is stubbed' do - let(:build_pre_var) { { key: 'build', value: 'value', public: true } } - let(:project_pre_var) { { key: 'project', value: 'value', public: true } } - let(:pipeline_pre_var) { { key: 'pipeline', value: 'value', public: true } } - let(:build_yaml_var) { { key: 'yaml', value: 'value', public: true } } - - before do - allow(build).to receive(:predefined_variables) { [build_pre_var] } - allow(build).to receive(:yaml_variables) { [build_yaml_var] } - allow(build).to receive(:persisted_variables) { [] } - - allow_any_instance_of(Project) - .to receive(:predefined_variables) { [project_pre_var] } - - allow_any_instance_of(Project) - .to receive(:ci_variables_for) - .with(ref: 'master', environment: nil) do - [create(:ci_variable, key: 'secret', value: 'value')] - end - - allow_any_instance_of(Ci::Pipeline) - .to receive(:predefined_variables) { [pipeline_pre_var] } - end - - it 'returns variables in order depending on resource hierarchy' do - is_expected.to eq( - [build_pre_var, - project_pre_var, - pipeline_pre_var, - build_yaml_var, - { key: 'secret', value: 'value', public: false }]) - end - end - - context 'when build has environment and user-provided variables' do - let(:expected_variables) do - predefined_variables.map { |variable| variable.fetch(:key) } + - %w[YAML_VARIABLE CI_ENVIRONMENT_NAME CI_ENVIRONMENT_SLUG - CI_ENVIRONMENT_URL] - end - - before do - create(:environment, project: build.project, - name: 'staging') - - build.yaml_variables = [{ key: 'YAML_VARIABLE', - value: 'var', - public: true }] - build.environment = 'staging' - end - - it 'matches explicit variables ordering' do - received_variables = subject.map { |variable| variable.fetch(:key) } - - expect(received_variables).to eq expected_variables - end - end - end - context 'when build has not been persisted yet' do let(:build) do described_class.new( diff --git a/spec/models/clusters/applications/prometheus_spec.rb b/spec/models/clusters/applications/prometheus_spec.rb index 86de9dc60f2..b5aa1dcece5 100644 --- a/spec/models/clusters/applications/prometheus_spec.rb +++ b/spec/models/clusters/applications/prometheus_spec.rb @@ -35,7 +35,7 @@ describe Clusters::Applications::Prometheus do describe 'transition to installed' do let(:project) { create(:project) } - let(:cluster) { create(:cluster, projects: [project]) } + let(:cluster) { create(:cluster, :with_installed_helm, projects: [project]) } let(:prometheus_service) { double('prometheus_service') } subject { create(:clusters_applications_prometheus, :installing, cluster: cluster) } diff --git a/spec/models/clusters/kubernetes_namespace_spec.rb b/spec/models/clusters/kubernetes_namespace_spec.rb index 0dfeea5cd2f..56c98d016c9 100644 --- a/spec/models/clusters/kubernetes_namespace_spec.rb +++ b/spec/models/clusters/kubernetes_namespace_spec.rb @@ -8,6 +8,22 @@ RSpec.describe Clusters::KubernetesNamespace, type: :model do it { is_expected.to belong_to(:cluster) } it { is_expected.to have_one(:platform_kubernetes) } + describe 'has_service_account_token' do + subject { described_class.has_service_account_token } + + context 'namespace has service_account_token' do + let!(:namespace) { create(:cluster_kubernetes_namespace, :with_token) } + + it { is_expected.to include(namespace) } + end + + context 'namespace has no service_account_token' do + let!(:namespace) { create(:cluster_kubernetes_namespace) } + + it { is_expected.not_to include(namespace) } + end + end + describe 'namespace uniqueness validation' do let(:cluster_project) { create(:cluster_project) } let(:kubernetes_namespace) { build(:cluster_kubernetes_namespace, namespace: 'my-namespace') } @@ -29,14 +45,14 @@ RSpec.describe Clusters::KubernetesNamespace, type: :model do end end - describe '#configure_predefined_variables' do + describe '#set_defaults' do let(:kubernetes_namespace) { build(:cluster_kubernetes_namespace) } let(:cluster) { kubernetes_namespace.cluster } let(:platform) { kubernetes_namespace.platform_kubernetes } - subject { kubernetes_namespace.configure_predefined_credentials } + subject { kubernetes_namespace.set_defaults } - describe 'namespace' do + describe '#namespace' do before do platform.update_column(:namespace, namespace) end @@ -64,7 +80,7 @@ RSpec.describe Clusters::KubernetesNamespace, type: :model do end end - describe 'service_account_name' do + describe '#service_account_name' do let(:service_account_name) { "#{kubernetes_namespace.namespace}-service-account" } it 'should set a service account name based on namespace' do diff --git a/spec/models/clusters/platforms/kubernetes_spec.rb b/spec/models/clusters/platforms/kubernetes_spec.rb index f5d261c4e9d..99fd6ccc4d8 100644 --- a/spec/models/clusters/platforms/kubernetes_spec.rb +++ b/spec/models/clusters/platforms/kubernetes_spec.rb @@ -210,9 +210,11 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching let(:api_url) { 'https://kube.domain.com' } let(:ca_pem) { 'CA PEM DATA' } + subject { kubernetes.predefined_variables(project: cluster.project) } + shared_examples 'setting variables' do it 'sets the variables' do - expect(kubernetes.predefined_variables(project: cluster.project)).to include( + expect(subject).to include( { key: 'KUBE_URL', value: api_url, public: true }, { key: 'KUBE_CA_PEM', value: ca_pem, public: true }, { key: 'KUBE_CA_PEM_FILE', value: ca_pem, public: true, file: true } @@ -220,6 +222,30 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching end end + context 'kubernetes namespace is created with no service account token' do + let!(:kubernetes_namespace) { create(:cluster_kubernetes_namespace, cluster: cluster) } + + it_behaves_like 'setting variables' + + it 'sets KUBE_TOKEN' do + expect(subject).to include( + { key: 'KUBE_TOKEN', value: kubernetes.token, public: false } + ) + end + end + + context 'kubernetes namespace is created with no service account token' do + let!(:kubernetes_namespace) { create(:cluster_kubernetes_namespace, :with_token, cluster: cluster) } + + it_behaves_like 'setting variables' + + it 'sets KUBE_TOKEN' do + expect(subject).to include( + { key: 'KUBE_TOKEN', value: kubernetes_namespace.service_account_token, public: false } + ) + end + end + context 'namespace is provided' do let(:namespace) { 'my-project' } @@ -228,12 +254,24 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching end it_behaves_like 'setting variables' + + it 'sets KUBE_TOKEN' do + expect(subject).to include( + { key: 'KUBE_TOKEN', value: kubernetes.token, public: false } + ) + end end context 'no namespace provided' do let(:namespace) { kubernetes.actual_namespace } it_behaves_like 'setting variables' + + it 'sets KUBE_TOKEN' do + expect(subject).to include( + { key: 'KUBE_TOKEN', value: kubernetes.token, public: false } + ) + end end end diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index ed41ff7a0fa..2a0039a0635 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -72,6 +72,7 @@ describe Commit do context 'using eager loading' do let!(:alice) { create(:user, email: 'alice@example.com') } let!(:bob) { create(:user, email: 'hunter2@example.com') } + let!(:jeff) { create(:user) } let(:alice_commit) do described_class.new(RepoHelpers.sample_commit, project).tap do |c| @@ -93,7 +94,14 @@ describe Commit do end end - let!(:commits) { [alice_commit, bob_commit, eve_commit] } + let(:jeff_commit) do + # The commit for Jeff uses his private commit email + described_class.new(RepoHelpers.sample_commit, project).tap do |c| + c.author_email = jeff.private_commit_email + end + end + + let!(:commits) { [alice_commit, bob_commit, eve_commit, jeff_commit] } before do create(:email, user: bob, email: 'bob@example.com') @@ -125,6 +133,20 @@ describe Commit do expect(bob_commit.author).to eq(bob) end + it "preloads the authors for Commits using a User's private commit Email" do + commits.each(&:lazy_author) + + expect(jeff_commit.author).to eq(jeff) + end + + it "preloads the authors for Commits using a User's outdated private commit Email" do + jeff.update!(username: 'new-username') + + commits.each(&:lazy_author) + + expect(jeff_commit.author).to eq(jeff) + end + it 'sets the author to Nil if an author could not be found for a Commit' do commits.each(&:lazy_author) diff --git a/spec/models/concerns/awardable_spec.rb b/spec/models/concerns/awardable_spec.rb index debc02fa51f..5713106418d 100644 --- a/spec/models/concerns/awardable_spec.rb +++ b/spec/models/concerns/awardable_spec.rb @@ -37,8 +37,8 @@ describe Awardable do create(:award_emoji, awardable: issue3, name: "star", user: award_emoji.user) create(:award_emoji, awardable: issue3, name: "star", user: award_emoji2.user) - expect(Issue.awarded(award_emoji.user)).to eq [issue, issue3] - expect(Issue.awarded(award_emoji2.user)).to eq [issue2, issue3] + expect(Issue.awarded(award_emoji.user)).to contain_exactly(issue, issue3) + expect(Issue.awarded(award_emoji2.user)).to contain_exactly(issue2, issue3) end end diff --git a/spec/models/concerns/deployable_spec.rb b/spec/models/concerns/deployable_spec.rb index ac79c75a55e..6951be903fe 100644 --- a/spec/models/concerns/deployable_spec.rb +++ b/spec/models/concerns/deployable_spec.rb @@ -49,5 +49,26 @@ describe Deployable do expect(environment).to be_nil end end + + context 'when environment scope contains invalid character' do + let(:job) do + create( + :ci_build, + name: 'job:deploy-to-test-site', + environment: '$CI_JOB_NAME', + options: { + environment: { + name: '$CI_JOB_NAME', + url: 'http://staging.example.com/$CI_JOB_NAME', + on_stop: 'stop_review_app' + } + }) + end + + it 'does not create a deployment and environment record' do + expect(deployment).to be_nil + expect(environment).to be_nil + end + end end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 131db6a5ff9..a58dc8e25e8 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -1782,7 +1782,7 @@ describe MergeRequest do allow(subject).to receive(:head_pipeline) { nil } end - it { expect(subject.mergeable_ci_state?).to be_truthy } + it { expect(subject.mergeable_ci_state?).to be_falsey } 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 19c2862264f..973d6bdb2a0 100644 --- a/spec/models/project_services/chat_message/push_message_spec.rb +++ b/spec/models/project_services/chat_message/push_message_spec.rb @@ -48,12 +48,12 @@ describe ChatMessage::PushMessage do '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', + expect(subject.activity).to eq( + title: 'test.user pushed to branch [master](http://url.com/commits/master)', subtitle: 'in [project_name](http://url.com)', text: '[Compare changes](http://url.com/compare/before...after)', image: 'http://someavatar.com' - }) + ) end end end @@ -89,12 +89,53 @@ describe ChatMessage::PushMessage 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', + expect(subject.activity).to eq( + title: 'test.user pushed new tag [new_tag](http://url.com/commits/new_tag)', subtitle: 'in [project_name](http://url.com)', text: '[Compare changes](http://url.com/compare/0000000000000000000000000000000000000000...after)', image: 'http://someavatar.com' - }) + ) + end + end + end + + context 'removed tag' do + let(:args) do + { + after: Gitlab::Git::BLANK_SHA, + before: 'before', + project_name: 'project_name', + ref: 'refs/tags/new_tag', + user_name: 'test.user', + user_avatar: 'http://someavatar.com', + project_url: 'http://url.com' + } + end + + context 'without markdown' do + it 'returns a message regarding removal of tags' do + expect(subject.pretext).to eq('test.user removed tag ' \ + 'new_tag 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 removal of tags' do + expect(subject.pretext).to eq( + 'test.user removed tag new_tag from [project_name](http://url.com)') + expect(subject.attachments).to be_empty + expect(subject.activity).to eq( + title: 'test.user removed tag new_tag', + subtitle: 'in [project_name](http://url.com)', + text: '[Compare changes](http://url.com/compare/before...0000000000000000000000000000000000000000)', + image: 'http://someavatar.com' + ) end end end @@ -122,12 +163,12 @@ describe ChatMessage::PushMessage 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', + expect(subject.activity).to eq( + title: 'test.user pushed new branch [master](http://url.com/commits/master)', subtitle: 'in [project_name](http://url.com)', text: '[Compare changes](http://url.com/compare/0000000000000000000000000000000000000000...after)', image: 'http://someavatar.com' - }) + ) end end end @@ -154,12 +195,12 @@ describe ChatMessage::PushMessage 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', + expect(subject.activity).to eq( + title: 'test.user removed branch master', subtitle: 'in [project_name](http://url.com)', text: '[Compare changes](http://url.com/compare/before...0000000000000000000000000000000000000000)', image: 'http://someavatar.com' - }) + ) end end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index b2ca6e98068..51278836604 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2415,7 +2415,7 @@ describe Project do end context 'when user configured kubernetes from CI/CD > Clusters and KubernetesNamespace migration has been executed' do - let!(:kubernetes_namespace) { create(:cluster_kubernetes_namespace) } + let!(:kubernetes_namespace) { create(:cluster_kubernetes_namespace, :with_token) } let!(:cluster) { kubernetes_namespace.cluster } let(:project) { kubernetes_namespace.project } @@ -3087,6 +3087,14 @@ describe Project do it 'does not flag as read-only' do expect { project.migrate_to_hashed_storage! }.not_to change { project.repository_read_only } end + + context 'when partially migrated' do + it 'returns true' do + project = create(:project, storage_version: 1, skip_disk_validation: true) + + expect(project.migrate_to_hashed_storage!).to be_truthy + end + end end end diff --git a/spec/models/remote_mirror_spec.rb b/spec/models/remote_mirror_spec.rb index 3d316fb3c5b..da61a5f2771 100644 --- a/spec/models/remote_mirror_spec.rb +++ b/spec/models/remote_mirror_spec.rb @@ -222,14 +222,26 @@ describe RemoteMirror do context '#ensure_remote!' do let(:remote_mirror) { create(:project, :repository, :remote_mirror).remote_mirrors.first } + let(:project) { remote_mirror.project } + let(:repository) { project.repository } it 'adds a remote multiple times with no errors' do - expect(remote_mirror.project.repository).to receive(:add_remote).with(remote_mirror.remote_name, remote_mirror.url).twice.and_call_original + expect(repository).to receive(:add_remote).with(remote_mirror.remote_name, remote_mirror.url).twice.and_call_original 2.times do remote_mirror.ensure_remote! end end + + context 'SSH public-key authentication' do + it 'omits the password from the URL' do + remote_mirror.update!(auth_method: 'ssh_public_key', url: 'ssh://git:pass@example.com') + + expect(repository).to receive(:add_remote).with(remote_mirror.remote_name, 'ssh://git@example.com') + + remote_mirror.ensure_remote! + end + end end context '#updated_since?' do diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 799a60ac62f..56edb0fd6da 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -2403,4 +2403,22 @@ describe Repository do repository.merge_base('master', 'fix') end end + + describe '#cache' do + subject(:cache) { repository.send(:cache) } + + it 'returns a RepositoryCache' do + expect(subject).to be_kind_of Gitlab::RepositoryCache + end + + it 'when is_wiki it includes wiki as part of key' do + allow(repository).to receive(:is_wiki) { true } + + expect(subject.namespace).to include('wiki') + end + + it 'when is_wiki is false extra_namespace is nil' do + expect(subject.namespace).not_to include('wiki') + end + end end diff --git a/spec/models/todo_spec.rb b/spec/models/todo_spec.rb index 2c01578aaca..82ff2a002e0 100644 --- a/spec/models/todo_spec.rb +++ b/spec/models/todo_spec.rb @@ -226,7 +226,7 @@ describe Todo do create(:todo, target: create(:merge_request)) - expect(described_class.for_type(Issue)).to eq([todo]) + expect(described_class.for_type(Issue.name)).to eq([todo]) end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 0ac5bd666ae..733c1c49f08 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1174,6 +1174,22 @@ describe User do expect(described_class.by_any_email(user.email, confirmed: true)).to eq([user]) end + + it 'finds user through a private commit email' do + user = create(:user) + private_email = user.private_commit_email + + expect(described_class.by_any_email(private_email)).to eq([user]) + expect(described_class.by_any_email(private_email, confirmed: true)).to eq([user]) + end + + it 'finds user through a private commit email in an array' do + user = create(:user) + private_email = user.private_commit_email + + expect(described_class.by_any_email([private_email])).to eq([user]) + expect(described_class.by_any_email([private_email], confirmed: true)).to eq([user]) + end end describe '.search' do @@ -1501,7 +1517,12 @@ describe User do email_unconfirmed = create :email, user: user user.reload - expect(user.all_emails).to match_array([user.email, email_unconfirmed.email, email_confirmed.email]) + expect(user.all_emails).to contain_exactly( + user.email, + user.private_commit_email, + email_unconfirmed.email, + email_confirmed.email + ) end end @@ -1512,7 +1533,11 @@ describe User do email_confirmed = create :email, user: user, confirmed_at: Time.now create :email, user: user - expect(user.verified_emails).to match_array([user.email, user.private_commit_email, email_confirmed.email]) + expect(user.verified_emails).to contain_exactly( + user.email, + user.private_commit_email, + email_confirmed.email + ) end end @@ -1532,6 +1557,14 @@ describe User do expect(user.verified_email?(user.private_commit_email)).to be_truthy end + it 'returns true for an outdated private commit email' do + old_email = user.private_commit_email + + user.update!(username: 'changed-username') + + expect(user.verified_email?(old_email)).to be_truthy + end + it 'returns false when the email is not verified/confirmed' do email_unconfirmed = create :email, user: user user.reload diff --git a/spec/policies/ci/pipeline_policy_spec.rb b/spec/policies/ci/pipeline_policy_spec.rb index bd32faf06ef..8022f61e67d 100644 --- a/spec/policies/ci/pipeline_policy_spec.rb +++ b/spec/policies/ci/pipeline_policy_spec.rb @@ -74,5 +74,23 @@ describe Ci::PipelinePolicy, :models do expect(policy).to be_allowed :update_pipeline end end + + describe 'destroy_pipeline' do + let(:project) { create(:project, :public) } + + context 'when user has owner access' do + let(:user) { project.owner } + + it 'is enabled' do + expect(policy).to be_allowed :destroy_pipeline + end + end + + context 'when user is not owner' do + it 'is disabled' do + expect(policy).not_to be_allowed :destroy_pipeline + end + end + end end end diff --git a/spec/requests/api/pipelines_spec.rb b/spec/requests/api/pipelines_spec.rb index f0e1992bccd..638cc9767d4 100644 --- a/spec/requests/api/pipelines_spec.rb +++ b/spec/requests/api/pipelines_spec.rb @@ -438,6 +438,67 @@ describe API::Pipelines do end end + describe 'DELETE /projects/:id/pipelines/:pipeline_id' do + context 'authorized user' do + let(:owner) { project.owner } + + it 'destroys the pipeline' do + delete api("/projects/#{project.id}/pipelines/#{pipeline.id}", owner) + + expect(response).to have_gitlab_http_status(204) + expect { pipeline.reload }.to raise_error(ActiveRecord::RecordNotFound) + end + + it 'returns 404 when it does not exist' do + delete api("/projects/#{project.id}/pipelines/123456", owner) + + expect(response).to have_gitlab_http_status(404) + expect(json_response['message']).to eq '404 Not found' + end + + it 'logs an audit event' do + expect { delete api("/projects/#{project.id}/pipelines/#{pipeline.id}", owner) }.to change { SecurityEvent.count }.by(1) + end + + context 'when the pipeline has jobs' do + let!(:build) { create(:ci_build, project: project, pipeline: pipeline) } + + it 'destroys associated jobs' do + delete api("/projects/#{project.id}/pipelines/#{pipeline.id}", owner) + + expect(response).to have_gitlab_http_status(204) + expect { build.reload }.to raise_error(ActiveRecord::RecordNotFound) + end + end + end + + context 'unauthorized user' do + context 'when user is not member' do + it 'should return a 404' do + delete api("/projects/#{project.id}/pipelines/#{pipeline.id}", non_member) + + expect(response).to have_gitlab_http_status(404) + expect(json_response['message']).to eq '404 Project Not Found' + end + end + + context 'when user is developer' do + let(:developer) { create(:user) } + + before do + project.add_developer(developer) + end + + it 'should return a 403' do + delete api("/projects/#{project.id}/pipelines/#{pipeline.id}", developer) + + expect(response).to have_gitlab_http_status(403) + expect(json_response['message']).to eq '403 Forbidden' + end + end + end + end + describe 'POST /projects/:id/pipelines/:pipeline_id/retry' do context 'authorized user' do let!(:pipeline) do diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index e6d01c9689f..bb913ae0e79 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -2018,11 +2018,11 @@ describe API::Users do expect(json_response['message']).to eq('403 Forbidden') end - it 'returns a personal access token' do + it 'returns an impersonation token' do get api("/users/#{user.id}/impersonation_tokens/#{impersonation_token.id}", admin) expect(response).to have_gitlab_http_status(200) - expect(json_response['token']).to be_present + expect(json_response['token']).not_to be_present expect(json_response['impersonation']).to be_truthy end end diff --git a/spec/serializers/environment_status_entity_spec.rb b/spec/serializers/environment_status_entity_spec.rb index 58e2e627410..8a6a38fe5f8 100644 --- a/spec/serializers/environment_status_entity_spec.rb +++ b/spec/serializers/environment_status_entity_spec.rb @@ -40,4 +40,40 @@ describe EnvironmentStatusEntity do it { is_expected.to include(:stop_url) } end + + context 'when deployment has metrics' do + let(:prometheus_adapter) { double('prometheus_adapter', can_query?: true) } + + let(:simple_metrics) do + { + success: true, + metrics: {}, + last_update: 42 + } + end + + before do + project.add_maintainer(user) + allow(deployment).to receive(:prometheus_adapter).and_return(prometheus_adapter) + allow(prometheus_adapter).to receive(:query).with(:deployment, deployment).and_return(simple_metrics) + allow(entity).to receive(:deployment).and_return(deployment) + end + + context 'when deployment succeeded' do + let(:deployment) { create(:deployment, :succeed, :review_app) } + + it 'returns metrics url' do + expect(subject[:metrics_url]) + .to eq("/#{project.namespace.name}/#{project.name}/environments/#{environment.id}/deployments/#{deployment.iid}/metrics") + end + end + + context 'when deployment is running' do + let(:deployment) { create(:deployment, :running, :review_app) } + + it 'does not return metrics url' do + expect(subject[:metrics_url]).to be_nil + end + end + end end diff --git a/spec/serializers/project_mirror_entity_spec.rb b/spec/serializers/project_mirror_entity_spec.rb new file mode 100644 index 00000000000..ad0a8bbdff0 --- /dev/null +++ b/spec/serializers/project_mirror_entity_spec.rb @@ -0,0 +1,12 @@ +require 'spec_helper' + +describe ProjectMirrorEntity do + let(:project) { create(:project, :repository, :remote_mirror) } + let(:entity) { described_class.new(project) } + + subject { entity.as_json } + + it 'exposes project-specific elements' do + is_expected.to include(:id, :remote_mirrors_attributes) + end +end diff --git a/spec/serializers/remote_mirror_entity_spec.rb b/spec/serializers/remote_mirror_entity_spec.rb new file mode 100644 index 00000000000..885b0b9b423 --- /dev/null +++ b/spec/serializers/remote_mirror_entity_spec.rb @@ -0,0 +1,16 @@ +require 'spec_helper' + +describe RemoteMirrorEntity do + let(:project) { create(:project, :repository, :remote_mirror) } + let(:remote_mirror) { project.remote_mirrors.first } + let(:entity) { described_class.new(remote_mirror) } + + subject { entity.as_json } + + it 'exposes remote-mirror-specific elements' do + is_expected.to include( + :id, :url, :enabled, :auth_method, + :ssh_known_hosts, :ssh_public_key, :ssh_known_hosts_fingerprints + ) + end +end diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index 193148d403a..4d9c5aabbda 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -608,5 +608,53 @@ describe Ci::CreatePipelineService do .to eq variables_attributes.map(&:with_indifferent_access) end end + + context 'when pipeline has a job with environment' do + let(:pipeline) { execute_service } + + before do + stub_ci_pipeline_yaml_file(YAML.dump(config)) + end + + context 'when environment name is valid' do + let(:config) do + { + review_app: { + script: 'deploy', + environment: { + name: 'review/${CI_COMMIT_REF_NAME}', + url: 'http://${CI_COMMIT_REF_SLUG}-staging.example.com' + } + } + } + end + + it 'has a job with environment' do + expect(pipeline.builds.count).to eq(1) + expect(pipeline.builds.first.persisted_environment.name).to eq('review/master') + expect(pipeline.builds.first.deployment).to be_created + end + end + + context 'when environment name is invalid' do + let(:config) do + { + 'job:deploy-to-test-site': { + script: 'deploy', + environment: { + name: '${CI_JOB_NAME}', + url: 'https://$APP_URL' + } + } + } + end + + it 'has a job without environment' do + expect(pipeline.builds.count).to eq(1) + expect(pipeline.builds.first.persisted_environment).to be_nil + expect(pipeline.builds.first.deployment).to be_nil + end + end + end end end diff --git a/spec/services/ci/destroy_pipeline_service_spec.rb b/spec/services/ci/destroy_pipeline_service_spec.rb new file mode 100644 index 00000000000..097daf67feb --- /dev/null +++ b/spec/services/ci/destroy_pipeline_service_spec.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe ::Ci::DestroyPipelineService do + let(:project) { create(:project) } + let!(:pipeline) { create(:ci_pipeline, project: project) } + + subject { described_class.new(project, user).execute(pipeline) } + + context 'user is owner' do + let(:user) { project.owner } + + it 'destroys the pipeline' do + subject + + expect { pipeline.reload }.to raise_error(ActiveRecord::RecordNotFound) + end + + it 'logs an audit event' do + expect { subject }.to change { SecurityEvent.count }.by(1) + end + + context 'when the pipeline has jobs' do + let!(:build) { create(:ci_build, project: project, pipeline: pipeline) } + + it 'destroys associated jobs' do + subject + + expect { build.reload }.to raise_error(ActiveRecord::RecordNotFound) + end + + it 'destroys associated stages' do + stages = pipeline.stages + + subject + + expect(stages).to all(raise_error(ActiveRecord::RecordNotFound)) + end + + context 'when job has artifacts' do + let!(:artifact) { create(:ci_job_artifact, :archive, job: build) } + + it 'destroys associated artifacts' do + subject + + expect { artifact.reload }.to raise_error(ActiveRecord::RecordNotFound) + end + end + end + end + + context 'user is not owner' do + let(:user) { create(:user) } + + it 'raises an exception' do + expect { subject }.to raise_error(Gitlab::Access::AccessDeniedError) + end + end +end diff --git a/spec/services/clusters/applications/check_installation_progress_service_spec.rb b/spec/services/clusters/applications/check_installation_progress_service_spec.rb index 1a565bb734d..ea17f2bb423 100644 --- a/spec/services/clusters/applications/check_installation_progress_service_spec.rb +++ b/spec/services/clusters/applications/check_installation_progress_service_spec.rb @@ -19,6 +19,10 @@ describe Clusters::Applications::CheckInstallationProgressService do shared_examples 'a not yet terminated installation' do |a_phase| let(:phase) { a_phase } + before do + expect(service).to receive(:installation_phase).once.and_return(phase) + end + context "when phase is #{a_phase}" do context 'when not timeouted' do it 'reschedule a new check' do @@ -50,8 +54,6 @@ describe Clusters::Applications::CheckInstallationProgressService do end before do - expect(service).to receive(:installation_phase).once.and_return(phase) - allow(service).to receive(:installation_errors).and_return(errors) allow(service).to receive(:remove_installation_pod).and_return(nil) end @@ -60,6 +62,10 @@ describe Clusters::Applications::CheckInstallationProgressService do context 'when installation POD succeeded' do let(:phase) { Gitlab::Kubernetes::Pod::SUCCEEDED } + before do + expect(service).to receive(:installation_phase).once.and_return(phase) + end + it_behaves_like 'a terminated installation' it 'make the application installed' do @@ -76,6 +82,10 @@ describe Clusters::Applications::CheckInstallationProgressService do let(:phase) { Gitlab::Kubernetes::Pod::FAILED } let(:errors) { 'test installation failed' } + before do + expect(service).to receive(:installation_phase).once.and_return(phase) + end + it_behaves_like 'a terminated installation' it 'make the application errored' do @@ -87,5 +97,22 @@ describe Clusters::Applications::CheckInstallationProgressService do end RESCHEDULE_PHASES.each { |phase| it_behaves_like 'a not yet terminated installation', phase } + + context 'when installation raises a Kubeclient::HttpError' do + let(:cluster) { create(:cluster, :provided_by_user, :project) } + + before do + application.update!(cluster: cluster) + + expect(service).to receive(:installation_phase).and_raise(Kubeclient::HttpError.new(401, 'Unauthorized', nil)) + end + + it 'shows the response code from the error' do + service.execute + + expect(application).to be_errored + expect(application.status_reason).to eq('Kubernetes error: 401') + end + end end end diff --git a/spec/services/clusters/applications/install_service_spec.rb b/spec/services/clusters/applications/install_service_spec.rb index 4bd19f5bd79..2f801d019fe 100644 --- a/spec/services/clusters/applications/install_service_spec.rb +++ b/spec/services/clusters/applications/install_service_spec.rb @@ -42,7 +42,7 @@ describe Clusters::Applications::InstallService do service.execute expect(application).to be_errored - expect(application.status_reason).to match('Kubernetes error.') + expect(application.status_reason).to match('Kubernetes error: 500') end end diff --git a/spec/services/clusters/gcp/kubernetes/create_or_update_namespace_service_spec.rb b/spec/services/clusters/gcp/kubernetes/create_or_update_namespace_service_spec.rb index fc922218ad0..661364ac765 100644 --- a/spec/services/clusters/gcp/kubernetes/create_or_update_namespace_service_spec.rb +++ b/spec/services/clusters/gcp/kubernetes/create_or_update_namespace_service_spec.rb @@ -44,7 +44,7 @@ describe Clusters::Gcp::Kubernetes::CreateOrUpdateNamespaceService, '#execute' d let(:namespace) { "#{project.path}-#{project.id}" } let(:kubernetes_namespace) do - build(:cluster_kubernetes_namespace, + create(:cluster_kubernetes_namespace, cluster: cluster, project: cluster_project.project, cluster_project: cluster_project) diff --git a/spec/services/clusters/gcp/kubernetes/fetch_kubernetes_token_service_spec.rb b/spec/services/clusters/gcp/kubernetes/fetch_kubernetes_token_service_spec.rb index 4d1a6bb7b3a..a5806559b14 100644 --- a/spec/services/clusters/gcp/kubernetes/fetch_kubernetes_token_service_spec.rb +++ b/spec/services/clusters/gcp/kubernetes/fetch_kubernetes_token_service_spec.rb @@ -19,13 +19,16 @@ describe Clusters::Gcp::Kubernetes::FetchKubernetesTokenService do subject { described_class.new(kubeclient, service_account_token_name, namespace).execute } + before do + stub_kubeclient_discover(api_url) + end + context 'when params correct' do let(:decoded_token) { 'xxx.token.xxx' } let(:token) { Base64.encode64(decoded_token) } context 'when gitlab-token exists' do before do - stub_kubeclient_discover(api_url) stub_kubeclient_get_secret( api_url, { @@ -39,9 +42,17 @@ describe Clusters::Gcp::Kubernetes::FetchKubernetesTokenService do it { is_expected.to eq(decoded_token) } end + context 'when there is a 500 error' do + before do + stub_kubeclient_get_secret_error(api_url, service_account_token_name, namespace: namespace, status: 500) + end + + it { expect { subject }.to raise_error(Kubeclient::HttpError) } + end + context 'when gitlab-token does not exist' do before do - allow(kubeclient).to receive(:get_secret).and_raise(Kubeclient::HttpError.new(404, 'Not found', nil)) + stub_kubeclient_get_secret_error(api_url, service_account_token_name, namespace: namespace, status: 404) end it { is_expected.to be_nil } diff --git a/spec/services/merge_requests/create_from_issue_service_spec.rb b/spec/services/merge_requests/create_from_issue_service_spec.rb index b1882df732d..393299cce00 100644 --- a/spec/services/merge_requests/create_from_issue_service_spec.rb +++ b/spec/services/merge_requests/create_from_issue_service_spec.rb @@ -61,7 +61,15 @@ describe MergeRequests::CreateFromIssueService do expect(project.repository.branch_exists?(custom_source_branch)).to be_truthy end - it 'creates a system note' do + it 'creates the new_merge_request system note' do + expect(SystemNoteService).to receive(:new_merge_request).with(issue, project, user, instance_of(MergeRequest)) + + service.execute + end + + it 'creates the new_issue_branch system note when the branch could be created but the merge_request cannot be created' do + project.project_feature.update!(merge_requests_access_level: ProjectFeature::DISABLED) + expect(SystemNoteService).to receive(:new_issue_branch).with(issue, project, user, issue.to_branch_name) service.execute diff --git a/spec/services/projects/update_remote_mirror_service_spec.rb b/spec/services/projects/update_remote_mirror_service_spec.rb index cd903bfe8a5..c1e5f788146 100644 --- a/spec/services/projects/update_remote_mirror_service_spec.rb +++ b/spec/services/projects/update_remote_mirror_service_spec.rb @@ -16,7 +16,7 @@ describe Projects::UpdateRemoteMirrorService do end it "ensures the remote exists" do - stub_fetch_remote(project, remote_name: remote_name) + stub_fetch_remote(project, remote_name: remote_name, ssh_auth: remote_mirror) expect(remote_mirror).to receive(:ensure_remote!) @@ -26,13 +26,13 @@ describe Projects::UpdateRemoteMirrorService do it "fetches the remote repository" do expect(project.repository) .to receive(:fetch_remote) - .with(remote_mirror.remote_name, no_tags: true) + .with(remote_mirror.remote_name, no_tags: true, ssh_auth: remote_mirror) service.execute(remote_mirror) end it "returns success when updated succeeds" do - stub_fetch_remote(project, remote_name: remote_name) + stub_fetch_remote(project, remote_name: remote_name, ssh_auth: remote_mirror) result = service.execute(remote_mirror) @@ -41,7 +41,7 @@ describe Projects::UpdateRemoteMirrorService do context 'when syncing all branches' do it "push all the branches the first time" do - stub_fetch_remote(project, remote_name: remote_name) + stub_fetch_remote(project, remote_name: remote_name, ssh_auth: remote_mirror) expect(remote_mirror).to receive(:update_repository).with({}) @@ -51,7 +51,7 @@ describe Projects::UpdateRemoteMirrorService do context 'when only syncing protected branches' do it "sync updated protected branches" do - stub_fetch_remote(project, remote_name: remote_name) + stub_fetch_remote(project, remote_name: remote_name, ssh_auth: remote_mirror) protected_branch = create_protected_branch(project) remote_mirror.only_protected_branches = true @@ -69,10 +69,10 @@ describe Projects::UpdateRemoteMirrorService do end end - def stub_fetch_remote(project, remote_name:) + def stub_fetch_remote(project, remote_name:, ssh_auth:) allow(project.repository) .to receive(:fetch_remote) - .with(remote_name, no_tags: true) { fetch_remote(project.repository, remote_name) } + .with(remote_name, no_tags: true, ssh_auth: ssh_auth) { fetch_remote(project.repository, remote_name) } end def fetch_remote(repository, remote_name) diff --git a/spec/services/quick_actions/interpret_service_spec.rb b/spec/services/quick_actions/interpret_service_spec.rb index 5a7cafcb60f..938764f40b0 100644 --- a/spec/services/quick_actions/interpret_service_spec.rb +++ b/spec/services/quick_actions/interpret_service_spec.rb @@ -1222,6 +1222,37 @@ describe QuickActions::InterpretService do expect(commands).to be_empty expect(text).to eq("#{described_class::SHRUG}\n/close") end + + context '/create_merge_request command' do + let(:branch_name) { '1-feature' } + let(:content) { "/create_merge_request #{branch_name}" } + let(:issuable) { issue } + + context 'if issuable is not an Issue' do + let(:issuable) { merge_request } + + it_behaves_like 'empty command' + end + + context "when logged user cannot create_merge_requests in the project" do + let(:project) { create(:project, :archived) } + + it_behaves_like 'empty command' + end + + context 'when logged user cannot push code to the project' do + let(:project) { create(:project, :private) } + let(:service) { described_class.new(project, create(:user)) } + + it_behaves_like 'empty command' + end + + it 'populates create_merge_request with branch_name and issue iid' do + _, updates = service.execute(content, issuable) + + expect(updates).to eq(create_merge_request: { branch_name: branch_name, issue_iid: issuable.iid }) + end + end end describe '#explain' do @@ -1473,5 +1504,27 @@ describe QuickActions::InterpretService do end end end + + describe 'create a merge request' do + context 'with no branch name' do + let(:content) { '/create_merge_request' } + + it 'uses the default branch name' do + _, explanations = service.explain(content, issue) + + expect(explanations).to eq(['Creates a branch and a merge request to resolve this issue']) + end + end + + context 'with a branch name' do + let(:content) { '/create_merge_request foo' } + + it 'uses the given branch name' do + _, explanations = service.explain(content, issue) + + expect(explanations).to eq(["Creates branch 'foo' and a merge request to resolve this issue"]) + end + end + end end end diff --git a/spec/services/system_hooks_service_spec.rb b/spec/services/system_hooks_service_spec.rb index e0335880e8e..81b2c17fdb5 100644 --- a/spec/services/system_hooks_service_spec.rb +++ b/spec/services/system_hooks_service_spec.rb @@ -32,7 +32,7 @@ describe SystemHooksService do end it do - project.old_path_with_namespace = 'transfered_from_path' + project.old_path_with_namespace = 'transferred_from_path' expect(event_data(project, :transfer)).to include( :event_name, :name, :created_at, :updated_at, :path, :project_id, :owner_name, :owner_email, :project_visibility, diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index a18126ee339..0fbfcb34e50 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -432,6 +432,20 @@ describe SystemNoteService do end end + describe '.new_merge_request' do + subject { described_class.new_merge_request(noteable, project, author, merge_request) } + + let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } + + it_behaves_like 'a system note' do + let(:action) { 'merge' } + end + + it 'sets the new merge request note text' do + expect(subject.note).to eq("created merge request #{merge_request.to_reference} to address this issue") + end + end + describe '.cross_reference' do subject { described_class.cross_reference(noteable, mentioner, author) } diff --git a/spec/services/users/build_service_spec.rb b/spec/services/users/build_service_spec.rb index 051e8c87f39..b7b9817efdb 100644 --- a/spec/services/users/build_service_spec.rb +++ b/spec/services/users/build_service_spec.rb @@ -8,7 +8,7 @@ describe Users::BuildService do context 'with an admin user' do let(:admin_user) { create(:admin) } - let(:service) { described_class.new(admin_user, params) } + let(:service) { described_class.new(admin_user, ActionController::Parameters.new(params).permit!) } it 'returns a valid user' do expect(service.execute).to be_valid @@ -159,9 +159,9 @@ describe Users::BuildService do true | true | 'fl@example.com' | '' | true true | false | 'fl@example.com' | '' | true - true | nil | 'fl@example.com' | '^(?:(?!\.ext@).)*$\r?' | true - true | true | 'fl@example.com' | '^(?:(?!\.ext@).)*$\r?' | true - true | false | 'fl@example.com' | '^(?:(?!\.ext@).)*$\r?' | true + true | nil | 'fl@example.com' | '^(?:(?!\.ext@).)*$\r?' | false + true | true | 'fl@example.com' | '^(?:(?!\.ext@).)*$\r?' | false + true | false | 'fl@example.com' | '^(?:(?!\.ext@).)*$\r?' | false true | nil | 'tester.ext@domain.com' | '^(?:(?!\.ext@).)*$\r?' | true true | true | 'tester.ext@domain.com' | '^(?:(?!\.ext@).)*$\r?' | true diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index cd69160be10..3fedb9ed48c 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -235,6 +235,10 @@ RSpec.configure do |config| example.run if Gitlab::Database.mysql? end + config.around(:each, :rails5) do |example| + example.run if Gitlab.rails5? + end + # This makes sure the `ApplicationController#can?` method is stubbed with the # original implementation for all view specs. config.before(:each, type: :view) do diff --git a/spec/support/helpers/devise_helpers.rb b/spec/support/helpers/devise_helpers.rb index 66874e10f38..d32bc2424c0 100644 --- a/spec/support/helpers/devise_helpers.rb +++ b/spec/support/helpers/devise_helpers.rb @@ -8,8 +8,15 @@ module DeviseHelpers end def env_from_context(context) + # When we modify env_config, that is on the global + # Rails.application, and we need to stub it and allow it to be + # modified in-place, without polluting later tests. if context.respond_to?(:env_config) - context.env_config + context.env_config.deep_dup.tap do |env| + allow(context).to receive(:env_config).and_return(env) + end + # When we modify env, then the context is a request, or something + # else that only lives for a single spec. elsif context.respond_to?(:env) context.env end diff --git a/spec/support/helpers/features/branches_helpers.rb b/spec/support/helpers/features/branches_helpers.rb index 3525d9a70a7..df88fd425c9 100644 --- a/spec/support/helpers/features/branches_helpers.rb +++ b/spec/support/helpers/features/branches_helpers.rb @@ -20,7 +20,7 @@ module Spec end def select_branch(branch_name) - find(".git-revision-dropdown-toggle").click + find(".js-branch-select").click page.within("#new-branch-form .dropdown-menu") do click_link(branch_name) diff --git a/spec/support/helpers/filter_item_select_helper.rb b/spec/support/helpers/filter_item_select_helper.rb deleted file mode 100644 index 519e84d359e..00000000000 --- a/spec/support/helpers/filter_item_select_helper.rb +++ /dev/null @@ -1,19 +0,0 @@ -# Helper allows you to select value from filter-items -# -# Params -# value - value for select -# selector - css selector of item -# -# Usage: -# -# filter_item_select('Any Author', '.js-author-search') -# -module FilterItemSelectHelper - def filter_item_select(value, selector) - find(selector).click - wait_for_requests - page.within('.dropdown-content') do - click_link value - end - end -end diff --git a/spec/support/helpers/kubernetes_helpers.rb b/spec/support/helpers/kubernetes_helpers.rb index a03d9c4045f..ccaf86aa3a6 100644 --- a/spec/support/helpers/kubernetes_helpers.rb +++ b/spec/support/helpers/kubernetes_helpers.rb @@ -17,6 +17,7 @@ module KubernetesHelpers WebMock.stub_request(:get, api_url + '/api/v1').to_return(kube_response(kube_v1_discovery_body)) WebMock.stub_request(:get, api_url + '/apis/extensions/v1beta1').to_return(kube_response(kube_v1beta1_discovery_body)) WebMock.stub_request(:get, api_url + '/apis/rbac.authorization.k8s.io/v1').to_return(kube_response(kube_v1_rbac_authorization_discovery_body)) + WebMock.stub_request(:get, api_url + '/apis/serving.knative.dev/v1alpha1').to_return(kube_response(kube_v1alpha1_serving_knative_discovery_body)) end def stub_kubeclient_pods(response = nil) @@ -41,9 +42,9 @@ module KubernetesHelpers .to_return(kube_response(kube_v1_secret_body(options))) end - def stub_kubeclient_get_secret_error(api_url, name, namespace: 'default') + def stub_kubeclient_get_secret_error(api_url, name, namespace: 'default', status: 404) WebMock.stub_request(:get, api_url + "/api/v1/namespaces/#{namespace}/secrets/#{name}") - .to_return(status: [404, "Internal Server Error"]) + .to_return(status: [status, "Internal Server Error"]) end def stub_kubeclient_create_service_account(api_url, namespace: 'default') @@ -134,6 +135,18 @@ module KubernetesHelpers } end + def kube_v1alpha1_serving_knative_discovery_body + { + "kind" => "APIResourceList", + "resources" => [ + { "name" => "revisions", "namespaced" => true, "kind" => "Revision" }, + { "name" => "services", "namespaced" => true, "kind" => "Service" }, + { "name" => "configurations", "namespaced" => true, "kind" => "Configuration" }, + { "name" => "routes", "namespaced" => true, "kind" => "Route" } + ] + } + end + def kube_pods_body { "kind" => "PodList", diff --git a/spec/support/helpers/user_login_helper.rb b/spec/support/helpers/user_login_helper.rb new file mode 100644 index 00000000000..36c002f53af --- /dev/null +++ b/spec/support/helpers/user_login_helper.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +module UserLoginHelper + def ensure_tab_pane_correctness(visit_path = true) + if visit_path + visit new_user_session_path + end + + ensure_tab_pane_counts + ensure_one_active_tab + ensure_one_active_pane + end + + def ensure_tab_pane_counts + tabs_count = page.all('[role="tab"]').size + expect(page).to have_selector('[role="tabpanel"]', count: tabs_count) + end + + def ensure_one_active_tab + expect(page).to have_selector('ul.new-session-tabs > li > a.active', count: 1) + end + + def ensure_one_active_pane + expect(page).to have_selector('.tab-pane.active', count: 1) + end +end diff --git a/spec/support/shared_examples/models/cluster_application_status_shared_examples.rb b/spec/support/shared_examples/models/cluster_application_status_shared_examples.rb index 82f0dd5d00f..c391cc48f4e 100644 --- a/spec/support/shared_examples/models/cluster_application_status_shared_examples.rb +++ b/spec/support/shared_examples/models/cluster_application_status_shared_examples.rb @@ -44,10 +44,40 @@ shared_examples 'cluster application status specs' do |application_name| subject { create(application_name, :installing) } it 'is installed' do - subject.make_installed + subject.make_installed! expect(subject).to be_installed end + + it 'updates helm version' do + subject.cluster.application_helm.update!(version: '1.2.3') + + subject.make_installed! + + subject.cluster.application_helm.reload + + expect(subject.cluster.application_helm.version).to eq(Gitlab::Kubernetes::Helm::HELM_VERSION) + end + end + + describe '#make_updated' do + subject { create(application_name, :updating) } + + it 'is updated' do + subject.make_updated! + + expect(subject).to be_updated + end + + it 'updates helm version' do + subject.cluster.application_helm.update!(version: '1.2.3') + + subject.make_updated! + + subject.cluster.application_helm.reload + + expect(subject.cluster.application_helm.version).to eq(Gitlab::Kubernetes::Helm::HELM_VERSION) + end end describe '#make_errored' do diff --git a/spec/workers/emails_on_push_worker_spec.rb b/spec/workers/emails_on_push_worker_spec.rb index f17c5ac6aac..05b4fb49ea3 100644 --- a/spec/workers/emails_on_push_worker_spec.rb +++ b/spec/workers/emails_on_push_worker_spec.rb @@ -101,7 +101,7 @@ describe EmailsOnPushWorker, :mailer do context "when there are multiple recipients" do before do - # This is a hack because we modify the mail object before sending, for efficency, + # This is a hack because we modify the mail object before sending, for efficiency, # but the TestMailer adapter just appends the objects to an array. To clone a mail # object, create a new one! # https://github.com/mikel/mail/issues/314#issuecomment-12750108 |