diff options
Diffstat (limited to 'spec/requests')
41 files changed, 1675 insertions, 456 deletions
diff --git a/spec/requests/api/award_emoji_spec.rb b/spec/requests/api/award_emoji_spec.rb index 6c67d84b59b..342fcfa1041 100644 --- a/spec/requests/api/award_emoji_spec.rb +++ b/spec/requests/api/award_emoji_spec.rb @@ -155,6 +155,14 @@ describe API::AwardEmoji do expect(json_response['user']['username']).to eq(user.username) end + it 'marks Todos on the Issue as done' do + todo = create(:todo, target: issue, project: project, user: user) + + post api("/projects/#{project.id}/issues/#{issue.iid}/award_emoji", user), params: { name: '8ball' } + + expect(todo.reload).to be_done + end + it "returns a 400 bad request error if the name is not given" do post api("/projects/#{project.id}/issues/#{issue.iid}/award_emoji", user) @@ -209,6 +217,14 @@ describe API::AwardEmoji do expect(json_response['user']['username']).to eq(user.username) end + it 'marks Todos on the Noteable as done' do + todo = create(:todo, target: note2.noteable, project: project, user: user) + + post api("/projects/#{project.id}/issues/#{issue.iid}/notes/#{note.id}/award_emoji", user), params: { name: 'rocket' } + + expect(todo.reload).to be_done + end + it "normalizes +1 as thumbsup award" do post api("/projects/#{project.id}/issues/#{issue.iid}/notes/#{note.id}/award_emoji", user), params: { name: '+1' } diff --git a/spec/requests/api/boards_spec.rb b/spec/requests/api/boards_spec.rb index de79e8c4c5c..0b9c0c2ebe9 100644 --- a/spec/requests/api/boards_spec.rb +++ b/spec/requests/api/boards_spec.rb @@ -63,7 +63,7 @@ describe API::Boards do end end - describe "POST /groups/:id/boards/lists", :nested_groups do + describe "POST /groups/:id/boards/lists" do set(:group) { create(:group) } set(:board_parent) { create(:group, parent: group ) } let(:url) { "/groups/#{board_parent.id}/boards/#{board.id}/lists" } diff --git a/spec/requests/api/commit_statuses_spec.rb b/spec/requests/api/commit_statuses_spec.rb index b5e45f99109..1be8883bd3c 100644 --- a/spec/requests/api/commit_statuses_spec.rb +++ b/spec/requests/api/commit_statuses_spec.rb @@ -8,10 +8,6 @@ describe API::CommitStatuses do let(:developer) { create_user(:developer) } let(:sha) { commit.id } - let(:commit_status) do - create(:commit_status, status: :pending, pipeline: pipeline) - end - describe "GET /projects/:id/repository/commits/:sha/statuses" do let(:get_url) { "/projects/#{project.id}/repository/commits/#{sha}/statuses" } @@ -239,6 +235,26 @@ describe API::CommitStatuses do expect(CommitStatus.count).to eq 1 end end + + context 'when a pipeline id is specified' do + let!(:first_pipeline) { project.ci_pipelines.create(source: :push, sha: commit.id, ref: 'master', status: 'created') } + let!(:other_pipeline) { project.ci_pipelines.create(source: :push, sha: commit.id, ref: 'master', status: 'created') } + + subject do + post api(post_url, developer), params: { + pipeline_id: other_pipeline.id, + state: 'success', + ref: 'master' + } + end + + it 'update the correct pipeline' do + subject + + expect(first_pipeline.reload.status).to eq('created') + expect(other_pipeline.reload.status).to eq('success') + end + end end context 'when retrying a commit status' do diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb index 3df5d9412f8..5e6ff40e8cf 100644 --- a/spec/requests/api/commits_spec.rb +++ b/spec/requests/api/commits_spec.rb @@ -126,6 +126,12 @@ describe API::Commits do end end + context "with empty ref_name parameter" do + let(:route) { "/projects/#{project_id}/repository/commits?ref_name=" } + + it_behaves_like 'project commits' + end + context "path optional parameter" do it "returns project commits matching provided path parameter" do path = 'files/ruby/popen.rb' @@ -281,7 +287,7 @@ describe API::Commits do end it 'does not increment the usage counters using access token authentication' do - expect(::Gitlab::WebIdeCommitsCounter).not_to receive(:increment) + expect(::Gitlab::UsageDataCounters::WebIdeCounter).not_to receive(:increment_commits_count) post api(url, user), params: valid_c_params end @@ -320,67 +326,132 @@ describe API::Commits do end end - context 'when the API user is a guest' do + context 'when committing to a new branch' do def last_commit_id(project, branch_name) project.repository.find_branch(branch_name)&.dereferenced_target&.id end - let(:public_project) { create(:project, :public, :repository) } - let!(:url) { "/projects/#{public_project.id}/repository/commits" } - let(:guest) { create(:user).tap { |u| public_project.add_guest(u) } } + before do + valid_c_params[:start_branch] = 'master' + valid_c_params[:branch] = 'patch' + end - it 'returns a 403' do - post api(url, guest), params: valid_c_params + context 'when the API user is a guest' do + let(:public_project) { create(:project, :public, :repository) } + let(:url) { "/projects/#{public_project.id}/repository/commits" } + let(:guest) { create(:user).tap { |u| public_project.add_guest(u) } } - expect(response).to have_gitlab_http_status(403) - end + it 'returns a 403' do + post api(url, guest), params: valid_c_params - context 'when start_project is provided' do - context 'when posting to a forked project the user owns' do - let!(:forked_project) { fork_project(public_project, guest, namespace: guest.namespace, repository: true) } - let!(:url) { "/projects/#{forked_project.id}/repository/commits" } + expect(response).to have_gitlab_http_status(403) + end - before do - valid_c_params[:start_branch] = "master" - valid_c_params[:branch] = "patch" - end + context 'when start_project is provided' do + context 'when posting to a forked project the user owns' do + let(:forked_project) { fork_project(public_project, guest, namespace: guest.namespace, repository: true) } + let(:url) { "/projects/#{forked_project.id}/repository/commits" } + + context 'identified by Integer (id)' do + before do + valid_c_params[:start_project] = public_project.id + end + + it 'adds a new commit to forked_project and returns a 201' do + expect_request_with_status(201) { post api(url, guest), params: valid_c_params } + .to change { last_commit_id(forked_project, valid_c_params[:branch]) } + .and not_change { last_commit_id(public_project, valid_c_params[:start_branch]) } + end + end - context 'identified by Integer (id)' do - before do - valid_c_params[:start_project] = public_project.id + context 'identified by String (full_path)' do + before do + valid_c_params[:start_project] = public_project.full_path + end + + it 'adds a new commit to forked_project and returns a 201' do + expect_request_with_status(201) { post api(url, guest), params: valid_c_params } + .to change { last_commit_id(forked_project, valid_c_params[:branch]) } + .and not_change { last_commit_id(public_project, valid_c_params[:start_branch]) } + end + end + + context 'when branch already exists' do + before do + valid_c_params.delete(:start_branch) + valid_c_params[:branch] = 'master' + valid_c_params[:start_project] = public_project.id + end + + it 'returns a 400' do + post api(url, guest), params: valid_c_params + + expect(response).to have_gitlab_http_status(400) + expect(json_response['message']).to eq("A branch called 'master' already exists. Switch to that branch in order to make changes") + end + + context 'when force is set to true' do + before do + valid_c_params[:force] = true + end + + it 'adds a new commit to forked_project and returns a 201' do + expect_request_with_status(201) { post api(url, guest), params: valid_c_params } + .to change { last_commit_id(forked_project, valid_c_params[:branch]) } + .and not_change { last_commit_id(public_project, valid_c_params[:branch]) } + end + end end - it 'adds a new commit to forked_project and returns a 201' do - expect { post api(url, guest), params: valid_c_params } - .to change { last_commit_id(forked_project, valid_c_params[:branch]) } - .and not_change { last_commit_id(public_project, valid_c_params[:start_branch]) } + context 'when start_sha is also provided' do + let(:forked_project) { fork_project(public_project, guest, namespace: guest.namespace, repository: false) } + let(:start_sha) { public_project.repository.commit.parent.sha } - expect(response).to have_gitlab_http_status(201) + before do + # initialize an empty repository to force fetching from the original project + forked_project.repository.create_if_not_exists + + valid_c_params[:start_project] = public_project.id + valid_c_params[:start_sha] = start_sha + valid_c_params.delete(:start_branch) + end + + it 'fetches the start_sha from the original project to use as parent commit and returns a 201' do + expect_request_with_status(201) { post api(url, guest), params: valid_c_params } + .to change { last_commit_id(forked_project, valid_c_params[:branch]) } + .and not_change { last_commit_id(forked_project, 'master') } + + last_commit = forked_project.repository.find_branch(valid_c_params[:branch]).dereferenced_target + expect(last_commit.parent_id).to eq(start_sha) + end end end - context 'identified by String (full_path)' do + context 'when the target project is not part of the fork network of start_project' do + let(:unrelated_project) { create(:project, :public, :repository, creator: guest) } + let(:url) { "/projects/#{unrelated_project.id}/repository/commits" } + before do - valid_c_params[:start_project] = public_project.full_path + valid_c_params[:start_branch] = 'master' + valid_c_params[:branch] = 'patch' + valid_c_params[:start_project] = public_project.id end - it 'adds a new commit to forked_project and returns a 201' do - expect { post api(url, guest), params: valid_c_params } - .to change { last_commit_id(forked_project, valid_c_params[:branch]) } - .and not_change { last_commit_id(public_project, valid_c_params[:start_branch]) } + it 'returns a 403' do + post api(url, guest), params: valid_c_params - expect(response).to have_gitlab_http_status(201) + expect(response).to have_gitlab_http_status(403) end end end - context 'when the target project is not part of the fork network of start_project' do - let(:unrelated_project) { create(:project, :public, :repository, creator: guest) } - let!(:url) { "/projects/#{unrelated_project.id}/repository/commits" } + context 'when posting to a forked project the user does not have write access' do + let(:forked_project) { fork_project(public_project, user, namespace: user.namespace, repository: true) } + let(:url) { "/projects/#{forked_project.id}/repository/commits" } before do - valid_c_params[:start_branch] = "master" - valid_c_params[:branch] = "patch" + valid_c_params[:start_branch] = 'master' + valid_c_params[:branch] = 'patch' valid_c_params[:start_project] = public_project.id end @@ -392,20 +463,68 @@ describe API::Commits do end end - context 'when posting to a forked project the user does not have write access' do - let!(:forked_project) { fork_project(public_project, user, namespace: user.namespace, repository: true) } - let!(:url) { "/projects/#{forked_project.id}/repository/commits" } + context 'when start_sha is provided' do + let(:start_sha) { project.repository.commit.parent.sha } before do - valid_c_params[:start_branch] = "master" - valid_c_params[:branch] = "patch" - valid_c_params[:start_project] = public_project.id + valid_c_params[:start_sha] = start_sha + valid_c_params.delete(:start_branch) end - it 'returns a 403' do - post api(url, guest), params: valid_c_params + it 'returns a 400 if start_branch is also provided' do + valid_c_params[:start_branch] = 'master' + post api(url, user), params: valid_c_params - expect(response).to have_gitlab_http_status(403) + expect(response).to have_gitlab_http_status(400) + expect(json_response['error']).to eq('start_branch, start_sha are mutually exclusive') + end + + it 'returns a 400 if branch already exists' do + valid_c_params[:branch] = 'master' + post api(url, user), params: valid_c_params + + expect(response).to have_gitlab_http_status(400) + expect(json_response['message']).to eq("A branch called 'master' already exists. Switch to that branch in order to make changes") + end + + it 'returns a 400 if start_sha does not exist' do + valid_c_params[:start_sha] = '1' * 40 + post api(url, user), params: valid_c_params + + expect(response).to have_gitlab_http_status(400) + expect(json_response['message']).to eq("Cannot find start_sha '#{valid_c_params[:start_sha]}'") + end + + it 'returns a 400 if start_sha is not a full SHA' do + valid_c_params[:start_sha] = start_sha.slice(0, 7) + post api(url, user), params: valid_c_params + + expect(response).to have_gitlab_http_status(400) + expect(json_response['message']).to eq("Invalid start_sha '#{valid_c_params[:start_sha]}'") + end + + it 'uses the start_sha as parent commit and returns a 201' do + expect_request_with_status(201) { post api(url, user), params: valid_c_params } + .to change { last_commit_id(project, valid_c_params[:branch]) } + .and not_change { last_commit_id(project, 'master') } + + last_commit = project.repository.find_branch(valid_c_params[:branch]).dereferenced_target + expect(last_commit.parent_id).to eq(start_sha) + end + + context 'when force is set to true and branch already exists' do + before do + valid_c_params[:force] = true + valid_c_params[:branch] = 'master' + end + + it 'uses the start_sha as parent commit and returns a 201' do + expect_request_with_status(201) { post api(url, user), params: valid_c_params } + .to change { last_commit_id(project, valid_c_params[:branch]) } + + last_commit = project.repository.find_branch(valid_c_params[:branch]).dereferenced_target + expect(last_commit.parent_id).to eq(start_sha) + end end end end diff --git a/spec/requests/api/discussions_spec.rb b/spec/requests/api/discussions_spec.rb index ca1ffe3c524..0420201efe3 100644 --- a/spec/requests/api/discussions_spec.rb +++ b/spec/requests/api/discussions_spec.rb @@ -9,6 +9,13 @@ describe API::Discussions do project.add_developer(user) end + context 'when discussions have cross-reference system notes' do + let(:url) { "/projects/#{project.id}/merge_requests/#{merge_request.iid}/discussions" } + let(:notes_in_response) { json_response.first['notes'] } + + it_behaves_like 'with cross-reference system notes' + end + context 'when noteable is an Issue' do let!(:issue) { create(:issue, project: project, author: user) } let!(:issue_note) { create(:discussion_note_on_issue, noteable: issue, project: project, author: user) } diff --git a/spec/requests/api/files_spec.rb b/spec/requests/api/files_spec.rb index 1ad536258ba..21b67357543 100644 --- a/spec/requests/api/files_spec.rb +++ b/spec/requests/api/files_spec.rb @@ -186,6 +186,14 @@ describe API::Files do expect(headers[Gitlab::Workhorse::DETECT_HEADER]).to eq "true" end + it 'returns blame file info' do + url = route(file_path) + '/blame' + + get api(url, current_user), params: params + + expect(response).to have_gitlab_http_status(200) + end + it 'sets inline content disposition by default' do url = route(file_path) + "/raw" @@ -252,6 +260,160 @@ describe API::Files do end end + describe 'GET /projects/:id/repository/files/:file_path/blame' do + shared_examples_for 'repository blame files' do + let(:expected_blame_range_sizes) do + [3, 2, 1, 2, 1, 1, 1, 1, 8, 1, 3, 1, 2, 1, 4, 1, 2, 2] + end + + let(:expected_blame_range_commit_ids) do + %w[ + 913c66a37b4a45b9769037c55c2d238bd0942d2e + 874797c3a73b60d2187ed6e2fcabd289ff75171e + 913c66a37b4a45b9769037c55c2d238bd0942d2e + 874797c3a73b60d2187ed6e2fcabd289ff75171e + 570e7b2abdd848b95f2f578043fc23bd6f6fd24d + 874797c3a73b60d2187ed6e2fcabd289ff75171e + 913c66a37b4a45b9769037c55c2d238bd0942d2e + 874797c3a73b60d2187ed6e2fcabd289ff75171e + 570e7b2abdd848b95f2f578043fc23bd6f6fd24d + 913c66a37b4a45b9769037c55c2d238bd0942d2e + 874797c3a73b60d2187ed6e2fcabd289ff75171e + 913c66a37b4a45b9769037c55c2d238bd0942d2e + 874797c3a73b60d2187ed6e2fcabd289ff75171e + 570e7b2abdd848b95f2f578043fc23bd6f6fd24d + 874797c3a73b60d2187ed6e2fcabd289ff75171e + 913c66a37b4a45b9769037c55c2d238bd0942d2e + 874797c3a73b60d2187ed6e2fcabd289ff75171e + 913c66a37b4a45b9769037c55c2d238bd0942d2e + ] + end + + it 'returns file attributes in headers' do + head api(route(file_path) + '/blame', current_user), params: params + + expect(response).to have_gitlab_http_status(200) + expect(response.headers['X-Gitlab-File-Path']).to eq(CGI.unescape(file_path)) + expect(response.headers['X-Gitlab-File-Name']).to eq('popen.rb') + expect(response.headers['X-Gitlab-Last-Commit-Id']).to eq('570e7b2abdd848b95f2f578043fc23bd6f6fd24d') + expect(response.headers['X-Gitlab-Content-Sha256']) + .to eq('c440cd09bae50c4632cc58638ad33c6aa375b6109d811e76a9cc3a613c1e8887') + end + + it 'returns blame file attributes as json' do + get api(route(file_path) + '/blame', current_user), params: params + + expect(response).to have_gitlab_http_status(200) + expect(json_response.map { |x| x['lines'].size }).to eq(expected_blame_range_sizes) + expect(json_response.map { |x| x['commit']['id'] }).to eq(expected_blame_range_commit_ids) + range = json_response[0] + expect(range['lines']).to eq(["require 'fileutils'", "require 'open3'", '']) + expect(range['commit']['id']).to eq('913c66a37b4a45b9769037c55c2d238bd0942d2e') + expect(range['commit']['parent_ids']).to eq(['cfe32cf61b73a0d5e9f13e774abde7ff789b1660']) + expect(range['commit']['message']) + .to eq("Files, encoding and much more\n\nSigned-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>\n") + + expect(range['commit']['authored_date']).to eq('2014-02-27T08:14:56.000Z') + expect(range['commit']['author_name']).to eq('Dmitriy Zaporozhets') + expect(range['commit']['author_email']).to eq('dmitriy.zaporozhets@gmail.com') + + expect(range['commit']['committed_date']).to eq('2014-02-27T08:14:56.000Z') + expect(range['commit']['committer_name']).to eq('Dmitriy Zaporozhets') + expect(range['commit']['committer_email']).to eq('dmitriy.zaporozhets@gmail.com') + end + + it 'returns blame file info for files with dots' do + url = route('.gitignore') + '/blame' + + get api(url, current_user), params: params + + expect(response).to have_gitlab_http_status(200) + end + + it 'returns file by commit sha' do + # This file is deleted on HEAD + file_path = 'files%2Fjs%2Fcommit%2Ejs%2Ecoffee' + params[:ref] = '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9' + + get api(route(file_path) + '/blame', current_user), params: params + + expect(response).to have_gitlab_http_status(200) + end + + context 'when mandatory params are not given' do + it_behaves_like '400 response' do + let(:request) { get api(route('any%2Ffile/blame'), current_user) } + end + end + + context 'when file_path does not exist' do + let(:params) { { ref: 'master' } } + + it_behaves_like '404 response' do + let(:request) { get api(route('app%2Fmodels%2Fapplication%2Erb/blame'), current_user), params: params } + let(:message) { '404 File Not Found' } + end + end + + context 'when commit does not exist' do + let(:params) { { ref: '1111111111111111111111111111111111111111' } } + + it_behaves_like '404 response' do + let(:request) { get api(route(file_path + '/blame'), current_user), params: params } + let(:message) { '404 Commit Not Found' } + end + end + + context 'when repository is disabled' do + include_context 'disabled repository' + + it_behaves_like '403 response' do + let(:request) { get api(route(file_path + '/blame'), current_user), params: params } + end + end + end + + context 'when unauthenticated', 'and project is public' do + it_behaves_like 'repository blame files' do + let(:project) { create(:project, :public, :repository) } + let(:current_user) { nil } + end + end + + context 'when unauthenticated', 'and project is private' do + it_behaves_like '404 response' do + let(:request) { get api(route(file_path)), params: params } + let(:message) { '404 Project Not Found' } + end + end + + context 'when authenticated', 'as a developer' do + it_behaves_like 'repository blame files' do + let(:current_user) { user } + end + end + + context 'when authenticated', 'as a guest' do + it_behaves_like '403 response' do + let(:request) { get api(route(file_path) + '/blame', guest), params: params } + end + end + + context 'when PATs are used' do + it 'returns blame file by commit sha' do + token = create(:personal_access_token, scopes: ['read_repository'], user: user) + + # This file is deleted on HEAD + file_path = 'files%2Fjs%2Fcommit%2Ejs%2Ecoffee' + params[:ref] = '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9' + + get api(route(file_path) + '/blame', personal_access_token: token), params: params + + expect(response).to have_gitlab_http_status(200) + end + end + end + describe "GET /projects/:id/repository/files/:file_path/raw" do shared_examples_for 'repository raw files' do it 'returns raw file info' do diff --git a/spec/requests/api/graphql/multiplexed_queries_spec.rb b/spec/requests/api/graphql/multiplexed_queries_spec.rb index 844fd979285..9ebb57f6b9c 100644 --- a/spec/requests/api/graphql/multiplexed_queries_spec.rb +++ b/spec/requests/api/graphql/multiplexed_queries_spec.rb @@ -6,9 +6,9 @@ describe 'Multiplexed queries' do it 'returns responses for multiple queries' do queries = [ - { query: 'query($text: String) { echo(text: $text) }', + { query: 'query($text: String!) { echo(text: $text) }', variables: { 'text' => 'Hello' } }, - { query: 'query($text: String) { echo(text: $text) }', + { query: 'query($text: String!) { echo(text: $text) }', variables: { 'text' => 'World' } } ] @@ -23,8 +23,8 @@ describe 'Multiplexed queries' do it 'returns error and data combinations' do queries = [ - { query: 'query($text: String) { broken query }' }, - { query: 'query working($text: String) { echo(text: $text) }', + { query: 'query($text: String!) { broken query }' }, + { query: 'query working($text: String!) { echo(text: $text) }', variables: { 'text' => 'World' } } ] diff --git a/spec/requests/api/graphql/mutations/award_emojis/add_spec.rb b/spec/requests/api/graphql/mutations/award_emojis/add_spec.rb index 3982125a38a..5b910d5bfe0 100644 --- a/spec/requests/api/graphql/mutations/award_emojis/add_spec.rb +++ b/spec/requests/api/graphql/mutations/award_emojis/add_spec.rb @@ -5,9 +5,9 @@ require 'spec_helper' describe 'Adding an AwardEmoji' do include GraphqlHelpers - let(:current_user) { create(:user) } - let(:awardable) { create(:note) } - let(:project) { awardable.project } + set(:current_user) { create(:user) } + set(:project) { create(:project) } + set(:awardable) { create(:note, project: project) } let(:emoji_name) { 'thumbsup' } let(:mutation) do variables = { @@ -43,7 +43,7 @@ describe 'Adding an AwardEmoji' do end context 'when the given awardable is not an Awardable' do - let(:awardable) { create(:label) } + let(:awardable) { create(:label, project: project) } it_behaves_like 'a mutation that does not create an AwardEmoji' @@ -52,7 +52,7 @@ describe 'Adding an AwardEmoji' do end context 'when the given awardable is an Awardable but still cannot be awarded an emoji' do - let(:awardable) { create(:system_note) } + let(:awardable) { create(:system_note, project: project) } it_behaves_like 'a mutation that does not create an AwardEmoji' @@ -73,6 +73,13 @@ describe 'Adding an AwardEmoji' do expect(mutation_response['awardEmoji']['name']).to eq(emoji_name) end + describe 'marking Todos as done' do + let(:user) { current_user} + subject { post_graphql_mutation(mutation, current_user: user) } + + include_examples 'creating award emojis marks Todos as done' + end + context 'when there were active record validation errors' do before do expect_next_instance_of(AwardEmoji) do |award| diff --git a/spec/requests/api/graphql/mutations/award_emojis/toggle_spec.rb b/spec/requests/api/graphql/mutations/award_emojis/toggle_spec.rb index 31145730f10..ae628d3e56c 100644 --- a/spec/requests/api/graphql/mutations/award_emojis/toggle_spec.rb +++ b/spec/requests/api/graphql/mutations/award_emojis/toggle_spec.rb @@ -5,9 +5,9 @@ require 'spec_helper' describe 'Toggling an AwardEmoji' do include GraphqlHelpers - let(:current_user) { create(:user) } - let(:awardable) { create(:note) } - let(:project) { awardable.project } + set(:current_user) { create(:user) } + set(:project) { create(:project) } + set(:awardable) { create(:note, project: project) } let(:emoji_name) { 'thumbsup' } let(:mutation) do variables = { @@ -40,7 +40,7 @@ describe 'Toggling an AwardEmoji' do end context 'when the given awardable is not an Awardable' do - let(:awardable) { create(:label) } + let(:awardable) { create(:label, project: project) } it_behaves_like 'a mutation that does not create or destroy an AwardEmoji' @@ -49,7 +49,7 @@ describe 'Toggling an AwardEmoji' do end context 'when the given awardable is an Awardable but still cannot be awarded an emoji' do - let(:awardable) { create(:system_note) } + let(:awardable) { create(:system_note, project: project) } it_behaves_like 'a mutation that does not create or destroy an AwardEmoji' @@ -81,6 +81,13 @@ describe 'Toggling an AwardEmoji' do expect(mutation_response['toggledOn']).to eq(true) end + describe 'marking Todos as done' do + let(:user) { current_user} + subject { post_graphql_mutation(mutation, current_user: user) } + + include_examples 'creating award emojis marks Todos as done' + end + context 'when there were active record validation errors' do before do expect_next_instance_of(AwardEmoji) do |award| diff --git a/spec/requests/api/graphql/namespace/projects_spec.rb b/spec/requests/api/graphql/namespace/projects_spec.rb index 63fa16c79ca..815e9531ecf 100644 --- a/spec/requests/api/graphql/namespace/projects_spec.rb +++ b/spec/requests/api/graphql/namespace/projects_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'getting projects', :nested_groups do +describe 'getting projects' do include GraphqlHelpers let(:group) { create(:group) } diff --git a/spec/requests/api/graphql/namespace/root_storage_statistics_spec.rb b/spec/requests/api/graphql/namespace/root_storage_statistics_spec.rb new file mode 100644 index 00000000000..ac76d991bd4 --- /dev/null +++ b/spec/requests/api/graphql/namespace/root_storage_statistics_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'rendering namespace statistics' do + include GraphqlHelpers + + let(:namespace) { user.namespace } + let!(:statistics) { create(:namespace_root_storage_statistics, namespace: namespace, packages_size: 5.megabytes) } + let(:user) { create(:user) } + + let(:query) do + graphql_query_for('namespace', + { 'fullPath' => namespace.full_path }, + "rootStorageStatistics { #{all_graphql_fields_for('RootStorageStatistics')} }") + end + + shared_examples 'a working namespace with storage statistics query' do + it_behaves_like 'a working graphql query' do + before do + post_graphql(query, current_user: user) + end + end + + it 'includes the packages size if the user can read the statistics' do + post_graphql(query, current_user: user) + + expect(graphql_data['namespace']['rootStorageStatistics']).not_to be_blank + expect(graphql_data['namespace']['rootStorageStatistics']['packagesSize']).to eq(5.megabytes) + end + end + + it_behaves_like 'a working namespace with storage statistics query' + + context 'when the namespace is a group' do + let(:group) { create(:group) } + let(:namespace) { group } + + before do + group.add_owner(user) + end + + it_behaves_like 'a working namespace with storage statistics query' + + context 'when the namespace is public' do + let(:group) { create(:group, :public)} + + it 'hides statistics for unauthenticated requests' do + post_graphql(query, current_user: nil) + + expect(graphql_data['namespace']).to be_blank + end + end + end +end diff --git a/spec/requests/api/graphql/project/project_statistics_spec.rb b/spec/requests/api/graphql/project/project_statistics_spec.rb index 14a3f37b779..ddee8537454 100644 --- a/spec/requests/api/graphql/project/project_statistics_spec.rb +++ b/spec/requests/api/graphql/project/project_statistics_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe 'rendering namespace statistics' do +describe 'rendering project statistics' do include GraphqlHelpers let(:project) { create(:project) } diff --git a/spec/requests/api/group_clusters_spec.rb b/spec/requests/api/group_clusters_spec.rb new file mode 100644 index 00000000000..46e3dd650cc --- /dev/null +++ b/spec/requests/api/group_clusters_spec.rb @@ -0,0 +1,452 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe API::GroupClusters do + include KubernetesHelpers + + let(:current_user) { create(:user) } + let(:developer_user) { create(:user) } + let(:group) { create(:group, :private) } + + before do + group.add_developer(developer_user) + group.add_maintainer(current_user) + end + + describe 'GET /groups/:id/clusters' do + let!(:extra_cluster) { create(:cluster, :provided_by_gcp, :group) } + + let!(:clusters) do + create_list(:cluster, 5, :provided_by_gcp, :group, :production_environment, + groups: [group]) + end + + context 'non-authorized user' do + it 'responds with 403' do + get api("/groups/#{group.id}/clusters", developer_user) + + expect(response).to have_gitlab_http_status(403) + end + end + + context 'authorized user' do + before do + get api("/groups/#{group.id}/clusters", current_user) + end + + it 'responds with 200' do + expect(response).to have_gitlab_http_status(200) + end + + it 'includes pagination headers' do + expect(response).to include_pagination_headers + end + + it 'only include authorized clusters' do + cluster_ids = json_response.map { |cluster| cluster['id'] } + + expect(cluster_ids).to match_array(clusters.pluck(:id)) + expect(cluster_ids).not_to include(extra_cluster.id) + end + end + end + + describe 'GET /groups/:id/clusters/:cluster_id' do + let(:cluster_id) { cluster.id } + + let(:platform_kubernetes) do + create(:cluster_platform_kubernetes, :configured) + end + + let(:cluster) do + create(:cluster, :group, :provided_by_gcp, :with_domain, + platform_kubernetes: platform_kubernetes, + user: current_user, + groups: [group]) + end + + context 'non-authorized user' do + it 'responds with 403' do + get api("/groups/#{group.id}/clusters/#{cluster_id}", developer_user) + + expect(response).to have_gitlab_http_status(403) + end + end + + context 'authorized user' do + before do + get api("/groups/#{group.id}/clusters/#{cluster_id}", current_user) + end + + it 'returns specific cluster' do + expect(json_response['id']).to eq(cluster.id) + end + + it 'returns cluster information' do + expect(json_response['provider_type']).to eq('gcp') + expect(json_response['platform_type']).to eq('kubernetes') + expect(json_response['environment_scope']).to eq('*') + expect(json_response['cluster_type']).to eq('group_type') + expect(json_response['domain']).to eq('example.com') + end + + it 'returns group information' do + cluster_group = json_response['group'] + + expect(cluster_group['id']).to eq(group.id) + expect(cluster_group['name']).to eq(group.name) + expect(cluster_group['web_url']).to eq(group.web_url) + end + + it 'returns kubernetes platform information' do + platform = json_response['platform_kubernetes'] + + expect(platform['api_url']).to eq('https://kubernetes.example.com') + expect(platform['ca_cert']).to be_present + end + + it 'returns user information' do + user = json_response['user'] + + expect(user['id']).to eq(current_user.id) + expect(user['username']).to eq(current_user.username) + end + + it 'returns GCP provider information' do + gcp_provider = json_response['provider_gcp'] + + expect(gcp_provider['cluster_id']).to eq(cluster.id) + expect(gcp_provider['status_name']).to eq('created') + expect(gcp_provider['gcp_project_id']).to eq('test-gcp-project') + expect(gcp_provider['zone']).to eq('us-central1-a') + expect(gcp_provider['machine_type']).to eq('n1-standard-2') + expect(gcp_provider['num_nodes']).to eq(3) + expect(gcp_provider['endpoint']).to eq('111.111.111.111') + end + + context 'when cluster has no provider' do + let(:cluster) do + create(:cluster, :group, :provided_by_user, + groups: [group]) + end + + it 'does not include GCP provider info' do + expect(json_response['provider_gcp']).not_to be_present + end + end + + context 'with non-existing cluster' do + let(:cluster_id) { 123 } + + it 'returns 404' do + expect(response).to have_gitlab_http_status(404) + end + end + end + end + + shared_context 'kubernetes calls stubbed' do + before do + stub_kubeclient_discover(api_url) + end + end + + describe 'POST /groups/:id/clusters/user' do + include_context 'kubernetes calls stubbed' + + let(:api_url) { 'https://kubernetes.example.com' } + let(:authorization_type) { 'rbac' } + + let(:platform_kubernetes_attributes) do + { + api_url: api_url, + token: 'sample-token', + authorization_type: authorization_type + } + end + + let(:cluster_params) do + { + name: 'test-cluster', + domain: 'domain.example.com', + managed: false, + platform_kubernetes_attributes: platform_kubernetes_attributes + } + end + + context 'non-authorized user' do + it 'responds with 403' do + post api("/groups/#{group.id}/clusters/user", developer_user), params: cluster_params + + expect(response).to have_gitlab_http_status(403) + end + end + + context 'authorized user' do + before do + post api("/groups/#{group.id}/clusters/user", current_user), params: cluster_params + end + + context 'with valid params' do + it 'responds with 201' do + expect(response).to have_gitlab_http_status(201) + end + + it 'creates a new Cluster::Cluster' do + cluster_result = Clusters::Cluster.find(json_response["id"]) + platform_kubernetes = cluster_result.platform + + expect(cluster_result).to be_user + expect(cluster_result).to be_kubernetes + expect(cluster_result.group).to eq(group) + expect(cluster_result.name).to eq('test-cluster') + expect(cluster_result.domain).to eq('domain.example.com') + expect(cluster_result.managed).to be_falsy + expect(platform_kubernetes.rbac?).to be_truthy + expect(platform_kubernetes.api_url).to eq(api_url) + expect(platform_kubernetes.token).to eq('sample-token') + end + end + + context 'when user does not indicate authorization type' do + let(:platform_kubernetes_attributes) do + { + api_url: api_url, + token: 'sample-token' + } + end + + it 'defaults to RBAC' do + cluster_result = Clusters::Cluster.find(json_response['id']) + + expect(cluster_result.platform_kubernetes.rbac?).to be_truthy + end + end + + context 'when user sets authorization type as ABAC' do + let(:authorization_type) { 'abac' } + + it 'creates an ABAC cluster' do + cluster_result = Clusters::Cluster.find(json_response['id']) + + expect(cluster_result.platform.abac?).to be_truthy + end + end + + context 'with invalid params' do + let(:api_url) { 'invalid_api_url' } + + it 'responds with 400' do + expect(response).to have_gitlab_http_status(400) + end + + it 'does not create a new Clusters::Cluster' do + expect(group.reload.clusters).to be_empty + end + + it 'returns validation errors' do + expect(json_response['message']['platform_kubernetes.api_url'].first).to be_present + end + end + end + + context 'when user tries to add multiple clusters' do + before do + create(:cluster, :provided_by_gcp, :group, + groups: [group]) + + post api("/groups/#{group.id}/clusters/user", current_user), params: cluster_params + end + + it 'responds with 400' do + expect(response).to have_gitlab_http_status(400) + expect(json_response['message']['base'].first).to include('Instance does not support multiple Kubernetes clusters') + end + end + + context 'non-authorized user' do + before do + post api("/groups/#{group.id}/clusters/user", developer_user), params: cluster_params + end + + it 'responds with 403' do + expect(response).to have_gitlab_http_status(403) + + expect(json_response['message']).to eq('403 Forbidden') + end + end + end + + describe 'PUT /groups/:id/clusters/:cluster_id' do + include_context 'kubernetes calls stubbed' + + let(:api_url) { 'https://kubernetes.example.com' } + + let(:update_params) do + { + domain: domain, + platform_kubernetes_attributes: platform_kubernetes_attributes + } + end + + let(:domain) { 'new-domain.com' } + let(:platform_kubernetes_attributes) { {} } + + let(:cluster) do + create(:cluster, :group, :provided_by_gcp, + groups: [group], domain: 'old-domain.com') + end + + context 'non-authorized user' do + it 'responds with 403' do + put api("/groups/#{group.id}/clusters/#{cluster.id}", developer_user), params: update_params + + expect(response).to have_gitlab_http_status(403) + end + end + + context 'authorized user' do + before do + put api("/groups/#{group.id}/clusters/#{cluster.id}", current_user), params: update_params + + cluster.reload + end + + context 'with valid params' do + it 'responds with 200' do + expect(response).to have_gitlab_http_status(200) + end + + it 'updates cluster attributes' do + expect(cluster.domain).to eq('new-domain.com') + end + end + + context 'with invalid params' do + let(:domain) { 'invalid domain' } + + it 'responds with 400' do + expect(response).to have_gitlab_http_status(400) + end + + it 'does not update cluster attributes' do + expect(cluster.domain).to eq('old-domain.com') + end + + it 'returns validation errors' do + expect(json_response['message']['domain'].first).to match('contains invalid characters (valid characters: [a-z0-9\\-])') + end + end + + context 'with a GCP cluster' do + context 'when user tries to change GCP specific fields' do + let(:platform_kubernetes_attributes) do + { + api_url: 'https://new-api-url.com', + token: 'new-sample-token' + } + end + + it 'responds with 400' do + expect(response).to have_gitlab_http_status(400) + end + + it 'returns validation error' do + expect(json_response['message']['platform_kubernetes.base'].first).to eq('Cannot modify managed Kubernetes cluster') + end + end + + context 'when user tries to change domain' do + let(:domain) { 'new-domain.com' } + + it 'responds with 200' do + expect(response).to have_gitlab_http_status(200) + end + end + end + + context 'with an user cluster' do + let(:api_url) { 'https://new-api-url.com' } + + let(:cluster) do + create(:cluster, :group, :provided_by_user, + groups: [group]) + end + + let(:platform_kubernetes_attributes) do + { + api_url: api_url, + token: 'new-sample-token' + } + end + + let(:update_params) do + { + name: 'new-name', + platform_kubernetes_attributes: platform_kubernetes_attributes + } + end + + it 'responds with 200' do + expect(response).to have_gitlab_http_status(200) + end + + it 'updates platform kubernetes attributes' do + platform_kubernetes = cluster.platform_kubernetes + + expect(cluster.name).to eq('new-name') + expect(platform_kubernetes.api_url).to eq('https://new-api-url.com') + expect(platform_kubernetes.token).to eq('new-sample-token') + end + end + + context 'with a cluster that does not belong to user' do + let(:cluster) { create(:cluster, :group, :provided_by_user) } + + it 'responds with 404' do + expect(response).to have_gitlab_http_status(404) + end + end + end + end + + describe 'DELETE /groups/:id/clusters/:cluster_id' do + let(:cluster_params) { { cluster_id: cluster.id } } + + let(:cluster) do + create(:cluster, :group, :provided_by_gcp, + groups: [group]) + end + + context 'non-authorized user' do + it 'responds with 403' do + delete api("/groups/#{group.id}/clusters/#{cluster.id}", developer_user), params: cluster_params + + expect(response).to have_gitlab_http_status(403) + end + end + + context 'authorized user' do + before do + delete api("/groups/#{group.id}/clusters/#{cluster.id}", current_user), params: cluster_params + end + + it 'responds with 204' do + expect(response).to have_gitlab_http_status(204) + end + + it 'deletes the cluster' do + expect(Clusters::Cluster.exists?(id: cluster.id)).to be_falsy + end + + context 'with a cluster that does not belong to user' do + let(:cluster) { create(:cluster, :group, :provided_by_user) } + + it 'responds with 404' do + expect(response).to have_gitlab_http_status(404) + end + end + end + end +end diff --git a/spec/requests/api/group_container_repositories_spec.rb b/spec/requests/api/group_container_repositories_spec.rb new file mode 100644 index 00000000000..0a41e455d01 --- /dev/null +++ b/spec/requests/api/group_container_repositories_spec.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe API::GroupContainerRepositories do + set(:group) { create(:group, :private) } + set(:project) { create(:project, :private, group: group) } + let(:reporter) { create(:user) } + let(:guest) { create(:user) } + + let(:root_repository) { create(:container_repository, :root, project: project) } + let(:test_repository) { create(:container_repository, project: project) } + + let(:users) do + { + anonymous: nil, + guest: guest, + reporter: reporter + } + end + + let(:api_user) { reporter } + + before do + group.add_reporter(reporter) + group.add_guest(guest) + + stub_feature_flags(container_registry_api: true) + stub_container_registry_config(enabled: true) + + root_repository + test_repository + end + + describe 'GET /groups/:id/registry/repositories' do + let(:url) { "/groups/#{group.id}/registry/repositories" } + + subject { get api(url, api_user) } + + it_behaves_like 'rejected container repository access', :guest, :forbidden + it_behaves_like 'rejected container repository access', :anonymous, :not_found + + it_behaves_like 'returns repositories for allowed users', :reporter, 'group' do + let(:object) { group } + end + + context 'with invalid group id' do + let(:url) { '/groups/123412341234/registry/repositories' } + + it 'returns not found' do + subject + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end +end diff --git a/spec/requests/api/group_labels_spec.rb b/spec/requests/api/group_labels_spec.rb index 3769f8b78e4..0be4e2e9121 100644 --- a/spec/requests/api/group_labels_spec.rb +++ b/spec/requests/api/group_labels_spec.rb @@ -14,12 +14,25 @@ describe API::GroupLabels do get api("/groups/#{group.id}/labels", user) expect(response).to have_gitlab_http_status(200) - expect(response).to match_response_schema('public_api/v4/group_labels') expect(response).to include_pagination_headers expect(json_response).to be_an Array + expect(json_response).to all(match_schema('public_api/v4/labels/label')) expect(json_response.size).to eq(2) expect(json_response.map {|r| r['name'] }).to contain_exactly('feature', 'bug') end + + context 'when the with_counts parameter is set' do + it 'includes counts in the response' do + get api("/groups/#{group.id}/labels", user), params: { with_counts: true } + + expect(response).to have_gitlab_http_status(200) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response).to all(match_schema('public_api/v4/labels/label_with_counts')) + expect(json_response.size).to eq(2) + expect(json_response.map { |r| r['open_issues_count'] }).to contain_exactly(0, 0) + end + end end describe 'POST /groups/:id/labels' do @@ -94,7 +107,7 @@ describe API::GroupLabels do expect(response).to have_gitlab_http_status(400) end - it "does not delete parent's group labels", :nested_groups do + it "does not delete parent's group labels" do subgroup = create(:group, parent: group) subgroup_label = create(:group_label, title: 'feature', group: subgroup) @@ -127,7 +140,7 @@ describe API::GroupLabels do expect(json_response['description']).to eq('test') end - it "does not update parent's group label", :nested_groups do + it "does not update parent's group label" do subgroup = create(:group, parent: group) subgroup_label = create(:group_label, title: 'feature', group: subgroup) diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index c41408fba65..50f36141aed 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -530,7 +530,7 @@ describe API::Groups do expect(json_response.length).to eq(2) end - it "returns projects including those in subgroups", :nested_groups do + it "returns projects including those in subgroups" do subgroup = create(:group, parent: group1) create(:project, group: subgroup) create(:project, group: subgroup) @@ -642,7 +642,7 @@ describe API::Groups do end end - describe 'GET /groups/:id/subgroups', :nested_groups do + describe 'GET /groups/:id/subgroups' do let!(:subgroup1) { create(:group, parent: group1) } let!(:subgroup2) { create(:group, :private, parent: group1) } let!(:subgroup3) { create(:group, :private, parent: group2) } @@ -786,7 +786,7 @@ describe API::Groups do expect(response).to have_gitlab_http_status(403) end - context 'as owner', :nested_groups do + context 'as owner' do before do group2.add_owner(user1) end @@ -798,15 +798,15 @@ describe API::Groups do end end - context 'as maintainer', :nested_groups do + context 'as maintainer' do before do group2.add_maintainer(user1) end - it 'cannot create subgroups' do + it 'can create subgroups' do post api("/groups", user1), params: { parent_id: group2.id, name: 'foo', path: 'foo' } - expect(response).to have_gitlab_http_status(403) + expect(response).to have_gitlab_http_status(201) end end end @@ -825,7 +825,7 @@ describe API::Groups do expect(json_response["visibility"]).to eq(Gitlab::VisibilityLevel.string_level(Gitlab::CurrentSettings.current_application_settings.default_group_visibility)) end - it "creates a nested group", :nested_groups do + it "creates a nested group" do parent = create(:group) parent.add_owner(user3) group = attributes_for(:group, { parent_id: parent.id }) diff --git a/spec/requests/api/internal_spec.rb b/spec/requests/api/internal_spec.rb index fcbff19bd61..3ab1818bebb 100644 --- a/spec/requests/api/internal_spec.rb +++ b/spec/requests/api/internal_spec.rb @@ -474,7 +474,7 @@ describe API::Internal do 'ssh', { authentication_abilities: [:read_project, :download_code, :push_code], - namespace_path: project.namespace.name, + namespace_path: project.namespace.path, project_path: project.path, redirected_path: nil } @@ -753,7 +753,7 @@ describe API::Internal do end describe 'GET /internal/merge_request_urls' do - let(:repo_name) { "#{project.namespace.name}/#{project.path}" } + let(:repo_name) { "#{project.full_path}" } let(:changes) { URI.escape("#{Gitlab::Git::BLANK_SHA} 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/new_branch") } before do @@ -765,7 +765,7 @@ describe API::Internal do expect(json_response).to match [{ "branch_name" => "new_branch", - "url" => "http://#{Gitlab.config.gitlab.host}/#{project.namespace.name}/#{project.path}/merge_requests/new?merge_request%5Bsource_branch%5D=new_branch", + "url" => "http://#{Gitlab.config.gitlab.host}/#{project.full_path}/merge_requests/new?merge_request%5Bsource_branch%5D=new_branch", "new_merge_request" => true }] end @@ -786,7 +786,7 @@ describe API::Internal do expect(json_response).to match [{ "branch_name" => "new_branch", - "url" => "http://#{Gitlab.config.gitlab.host}/#{project.namespace.name}/#{project.path}/merge_requests/new?merge_request%5Bsource_branch%5D=new_branch", + "url" => "http://#{Gitlab.config.gitlab.host}/#{project.full_path}/merge_requests/new?merge_request%5Bsource_branch%5D=new_branch", "new_merge_request" => true }] end @@ -927,7 +927,7 @@ describe API::Internal do expect(json_response['merge_request_urls']).to match [{ "branch_name" => branch_name, - "url" => "http://#{Gitlab.config.gitlab.host}/#{project.namespace.name}/#{project.path}/merge_requests/new?merge_request%5Bsource_branch%5D=#{branch_name}", + "url" => "http://#{Gitlab.config.gitlab.host}/#{project.full_path}/merge_requests/new?merge_request%5Bsource_branch%5D=#{branch_name}", "new_merge_request" => true }] end @@ -970,7 +970,7 @@ describe API::Internal do expect(json_response['merge_request_urls']).to match [{ 'branch_name' => branch_name, - 'url' => "http://#{Gitlab.config.gitlab.host}/#{project.namespace.name}/#{project.path}/merge_requests/1", + 'url' => "http://#{Gitlab.config.gitlab.host}/#{project.full_path}/merge_requests/1", 'new_merge_request' => false }] end diff --git a/spec/requests/api/issues/get_group_issues_spec.rb b/spec/requests/api/issues/get_group_issues_spec.rb index 9a41d790945..c487471e4a1 100644 --- a/spec/requests/api/issues/get_group_issues_spec.rb +++ b/spec/requests/api/issues/get_group_issues_spec.rb @@ -82,7 +82,7 @@ describe API::Issues do end end - context 'when group has subgroups', :nested_groups do + context 'when group has subgroups' do let(:subgroup_1) { create(:group, parent: group) } let(:subgroup_2) { create(:group, parent: subgroup_1) } @@ -342,6 +342,14 @@ describe API::Issues do group_project.add_reporter(user) end + it 'exposes known attributes' do + get api(base_url, admin) + + expect(response).to have_gitlab_http_status(200) + expect(json_response.last.keys).to include(*%w(id iid project_id title description)) + expect(json_response.last).not_to have_key('subscribed') + end + it 'returns all group issues (including opened and closed)' do get api(base_url, admin) diff --git a/spec/requests/api/issues/get_project_issues_spec.rb b/spec/requests/api/issues/get_project_issues_spec.rb index f7ca6fd1e0a..b7aa3f93451 100644 --- a/spec/requests/api/issues/get_project_issues_spec.rb +++ b/spec/requests/api/issues/get_project_issues_spec.rb @@ -446,6 +446,14 @@ describe API::Issues do expect_paginated_array_response([closed_issue.id, confidential_issue.id, issue.id]) end + it 'exposes known attributes' do + get api("#{base_url}/issues", user) + + expect(response).to have_gitlab_http_status(200) + expect(json_response.last.keys).to include(*%w(id iid project_id title description)) + expect(json_response.last).not_to have_key('subscribed') + end + context 'issues_statistics' do context 'no state is treated as all state' do let(:params) { {} } @@ -575,6 +583,7 @@ describe API::Issues do expect(json_response['assignee']).to be_a Hash expect(json_response['author']).to be_a Hash expect(json_response['confidential']).to be_falsy + expect(json_response['subscribed']).to be_truthy end it 'exposes the closed_at attribute' do diff --git a/spec/requests/api/issues/issues_spec.rb b/spec/requests/api/issues/issues_spec.rb index d195f54be11..f19c2dcc6fe 100644 --- a/spec/requests/api/issues/issues_spec.rb +++ b/spec/requests/api/issues/issues_spec.rb @@ -216,6 +216,10 @@ describe API::Issues do expect_paginated_array_response([issue.id, closed_issue.id]) expect(json_response.first['title']).to eq(issue.title) expect(json_response.last).to have_key('web_url') + # Calculating the value of subscribed field triggers Markdown + # processing. We can't do that for multiple issues / merge + # requests in a single API request. + expect(json_response.last).not_to have_key('subscribed') end it 'returns an array of closed issues' do @@ -603,6 +607,22 @@ describe API::Issues do expect_paginated_array_response([closed_issue.id, issue.id]) end + context 'with issues list sort options' do + it 'accepts only predefined order by params' do + API::Helpers::IssuesHelpers.sort_options.each do |sort_opt| + get api('/issues', user), params: { order_by: sort_opt, sort: 'asc' } + expect(response).to have_gitlab_http_status(200) + end + end + + it 'fails to sort with non predefined options' do + %w(milestone title abracadabra).each do |sort_opt| + get api('/issues', user), params: { order_by: sort_opt, sort: 'asc' } + expect(response).to have_gitlab_http_status(400) + end + end + end + it 'matches V4 response schema' do get api('/issues', user) diff --git a/spec/requests/api/labels_spec.rb b/spec/requests/api/labels_spec.rb index 518181e4d93..9aef67e28a7 100644 --- a/spec/requests/api/labels_spec.rb +++ b/spec/requests/api/labels_spec.rb @@ -6,70 +6,255 @@ describe API::Labels do let!(:label1) { create(:label, title: 'label1', project: project) } let!(:priority_label) { create(:label, title: 'bug', project: project, priority: 3) } + shared_examples 'label update API' do + it 'returns 200 if name is changed' do + request_params = { + new_name: 'New Label' + }.merge(spec_params) + + put api("/projects/#{project.id}/labels", user), + params: request_params + + expect(response).to have_gitlab_http_status(200) + expect(json_response['name']).to eq('New Label') + expect(json_response['color']).to eq(label1.color) + end + + it 'returns 200 if colors is changed' do + request_params = { + color: '#FFFFFF' + }.merge(spec_params) + + put api("/projects/#{project.id}/labels", user), + params: request_params + + expect(response).to have_gitlab_http_status(200) + expect(json_response['name']).to eq(label1.name) + expect(json_response['color']).to eq('#FFFFFF') + end + + it 'returns 200 if a priority is added' do + request_params = { + priority: 3 + }.merge(spec_params) + + put api("/projects/#{project.id}/labels", user), + params: request_params + + expect(response.status).to eq(200) + expect(json_response['name']).to eq(label1.name) + expect(json_response['priority']).to eq(3) + end + + it 'returns 400 if no new parameters given' do + put api("/projects/#{project.id}/labels", user), params: spec_params + + expect(response).to have_gitlab_http_status(400) + expect(json_response['error']).to eq('new_name, color, description, priority are missing, '\ + 'at least one parameter must be provided') + end + + it 'returns 400 when color code is too short' do + request_params = { + color: '#FF' + }.merge(spec_params) + + put api("/projects/#{project.id}/labels", user), + params: request_params + + expect(response).to have_gitlab_http_status(400) + expect(json_response['message']['color']).to eq(['must be a valid color code']) + end + + it 'returns 400 for too long color code' do + request_params = { + color: '#FFAAFFFF' + }.merge(spec_params) + + put api("/projects/#{project.id}/labels", user), + params: request_params + + expect(response).to have_gitlab_http_status(400) + expect(json_response['message']['color']).to eq(['must be a valid color code']) + end + + it 'returns 400 for invalid priority' do + request_params = { + priority: 'foo' + }.merge(spec_params) + + put api("/projects/#{project.id}/labels", user), + params: request_params + + expect(response).to have_gitlab_http_status(400) + end + + it 'returns 200 if name and colors and description are changed' do + request_params = { + new_name: 'New Label', + color: '#FFFFFF', + description: 'test' + }.merge(spec_params) + + put api("/projects/#{project.id}/labels", user), + params: request_params + + expect(response).to have_gitlab_http_status(200) + expect(json_response['name']).to eq('New Label') + expect(json_response['color']).to eq('#FFFFFF') + expect(json_response['description']).to eq('test') + end + + it 'returns 400 for invalid name' do + request_params = { + new_name: ',', + color: '#FFFFFF' + }.merge(spec_params) + + put api("/projects/#{project.id}/labels", user), + params: request_params + + expect(response).to have_gitlab_http_status(400) + expect(json_response['message']['title']).to eq(['is invalid']) + end + + it 'returns 200 if description is changed' do + request_params = { + description: 'test' + }.merge(spec_params) + + put api("/projects/#{project.id}/labels", user), + params: request_params + + expect(response).to have_gitlab_http_status(200) + expect(json_response['id']).to eq(expected_response_label_id) + expect(json_response['description']).to eq('test') + end + + it 'returns 200 if priority is changed' do + request_params = { + priority: 10 + }.merge(spec_params) + + put api("/projects/#{project.id}/labels", user), + params: request_params + + expect(response.status).to eq(200) + expect(json_response['id']).to eq(expected_response_label_id) + expect(json_response['priority']).to eq(10) + end + + it 'returns 200 if a priority is removed' do + label = find_by_spec_params(spec_params) + expect(label).not_to be_nil + + label.priorities.create(project: label.project, priority: 1) + label.save! + + request_params = { + priority: nil + }.merge(spec_params) + + put api("/projects/#{project.id}/labels", user), + params: request_params + + expect(response.status).to eq(200) + expect(json_response['id']).to eq(expected_response_label_id) + expect(json_response['priority']).to be_nil + end + + def find_by_spec_params(params) + if params.key?(:label_id) + Label.find(params[:label_id]) + else + Label.find_by(name: params[:name]) + end + end + end + + shared_examples 'label delete API' do + it 'returns 204 for existing label' do + delete api("/projects/#{project.id}/labels", user), params: spec_params + + expect(response).to have_gitlab_http_status(204) + end + end + before do project.add_maintainer(user) end describe 'GET /projects/:id/labels' do - it 'returns all available labels to the project' do - group = create(:group) - group_label = create(:group_label, title: 'feature', group: group) - project.update(group: group) - create(:labeled_issue, project: project, labels: [group_label], author: user) - create(:labeled_issue, project: project, labels: [label1], author: user, state: :closed) - create(:labeled_merge_request, labels: [priority_label], author: user, source_project: project ) + let(:group) { create(:group) } + let!(:group_label) { create(:group_label, title: 'feature', group: group) } - expected_keys = %w( - id name color text_color description - open_issues_count closed_issues_count open_merge_requests_count - subscribed priority is_project_label - ) + before do + project.update!(group: group) + end + it 'returns all available labels to the project' do get api("/projects/#{project.id}/labels", user) expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers - expect(json_response).to be_an Array + expect(json_response).to all(match_schema('public_api/v4/labels/project_label')) expect(json_response.size).to eq(3) - expect(json_response.first.keys).to match_array expected_keys expect(json_response.map { |l| l['name'] }).to match_array([group_label.name, priority_label.name, label1.name]) + end - label1_response = json_response.find { |l| l['name'] == label1.title } - group_label_response = json_response.find { |l| l['name'] == group_label.title } - priority_label_response = json_response.find { |l| l['name'] == priority_label.title } - - expect(label1_response['open_issues_count']).to eq(0) - expect(label1_response['closed_issues_count']).to eq(1) - expect(label1_response['open_merge_requests_count']).to eq(0) - expect(label1_response['name']).to eq(label1.name) - expect(label1_response['color']).to be_present - expect(label1_response['text_color']).to be_present - expect(label1_response['description']).to be_nil - expect(label1_response['priority']).to be_nil - expect(label1_response['subscribed']).to be_falsey - expect(label1_response['is_project_label']).to be_truthy - - expect(group_label_response['open_issues_count']).to eq(1) - expect(group_label_response['closed_issues_count']).to eq(0) - expect(group_label_response['open_merge_requests_count']).to eq(0) - expect(group_label_response['name']).to eq(group_label.name) - expect(group_label_response['color']).to be_present - expect(group_label_response['text_color']).to be_present - expect(group_label_response['description']).to be_nil - expect(group_label_response['priority']).to be_nil - expect(group_label_response['subscribed']).to be_falsey - expect(group_label_response['is_project_label']).to be_falsey - - expect(priority_label_response['open_issues_count']).to eq(0) - expect(priority_label_response['closed_issues_count']).to eq(0) - expect(priority_label_response['open_merge_requests_count']).to eq(1) - expect(priority_label_response['name']).to eq(priority_label.name) - expect(priority_label_response['color']).to be_present - expect(priority_label_response['text_color']).to be_present - expect(priority_label_response['description']).to be_nil - expect(priority_label_response['priority']).to eq(3) - expect(priority_label_response['subscribed']).to be_falsey - expect(priority_label_response['is_project_label']).to be_truthy + context 'when the with_counts parameter is set' do + before do + create(:labeled_issue, project: project, labels: [group_label], author: user) + create(:labeled_issue, project: project, labels: [label1], author: user, state: :closed) + create(:labeled_merge_request, labels: [priority_label], author: user, source_project: project ) + end + + it 'includes counts in the response' do + get api("/projects/#{project.id}/labels", user), params: { with_counts: true } + + expect(response).to have_gitlab_http_status(200) + expect(response).to include_pagination_headers + expect(json_response).to all(match_schema('public_api/v4/labels/project_label_with_counts')) + expect(json_response.size).to eq(3) + expect(json_response.map { |l| l['name'] }).to match_array([group_label.name, priority_label.name, label1.name]) + + label1_response = json_response.find { |l| l['name'] == label1.title } + group_label_response = json_response.find { |l| l['name'] == group_label.title } + priority_label_response = json_response.find { |l| l['name'] == priority_label.title } + + expect(label1_response).to include('open_issues_count' => 0, + 'closed_issues_count' => 1, + 'open_merge_requests_count' => 0, + 'name' => label1.name, + 'description' => nil, + 'color' => a_string_matching(/^#\h{6}$/), + 'text_color' => a_string_matching(/^#\h{6}$/), + 'priority' => nil, + 'subscribed' => false, + 'is_project_label' => true) + + expect(group_label_response).to include('open_issues_count' => 1, + 'closed_issues_count' => 0, + 'open_merge_requests_count' => 0, + 'name' => group_label.name, + 'description' => nil, + 'color' => a_string_matching(/^#\h{6}$/), + 'text_color' => a_string_matching(/^#\h{6}$/), + 'priority' => nil, + 'subscribed' => false, + 'is_project_label' => false) + + expect(priority_label_response).to include('open_issues_count' => 0, + 'closed_issues_count' => 0, + 'open_merge_requests_count' => 1, + 'name' => priority_label.name, + 'description' => nil, + 'color' => a_string_matching(/^#\h{6}$/), + 'text_color' => a_string_matching(/^#\h{6}$/), + 'priority' => 3, + 'subscribed' => false, + 'is_project_label' => true) + end end end @@ -197,20 +382,34 @@ describe API::Labels do end describe 'DELETE /projects/:id/labels' do - it 'returns 204 for existing label' do - delete api("/projects/#{project.id}/labels", user), params: { name: 'label1' } + it_behaves_like 'label delete API' do + let(:spec_params) { { name: 'label1' } } + end - expect(response).to have_gitlab_http_status(204) + it_behaves_like 'label delete API' do + let(:spec_params) { { label_id: label1.id } } end it 'returns 404 for non existing label' do delete api("/projects/#{project.id}/labels", user), params: { name: 'label2' } + expect(response).to have_gitlab_http_status(404) expect(json_response['message']).to eq('404 Label Not Found') end it 'returns 400 for wrong parameters' do delete api("/projects/#{project.id}/labels", user) + + expect(response).to have_gitlab_http_status(400) + end + + it 'fails if label_id and name are given in params' do + delete api("/projects/#{project.id}/labels", user), + params: { + label_id: label1.id, + name: priority_label.name + } + expect(response).to have_gitlab_http_status(400) end @@ -221,152 +420,105 @@ describe API::Labels do end describe 'PUT /projects/:id/labels' do - it 'returns 200 if name and colors and description are changed' do - put api("/projects/#{project.id}/labels", user), - params: { - name: 'label1', - new_name: 'New Label', - color: '#FFFFFF', - description: 'test' - } - expect(response).to have_gitlab_http_status(200) - expect(json_response['name']).to eq('New Label') - expect(json_response['color']).to eq('#FFFFFF') - expect(json_response['description']).to eq('test') + context 'when using name' do + it_behaves_like 'label update API' do + let(:spec_params) { { name: 'label1' } } + let(:expected_response_label_id) { label1.id } + end end - it 'returns 200 if name is changed' do - put api("/projects/#{project.id}/labels", user), - params: { - name: 'label1', - new_name: 'New Label' - } - expect(response).to have_gitlab_http_status(200) - expect(json_response['name']).to eq('New Label') - expect(json_response['color']).to eq(label1.color) + context 'when using label_id' do + it_behaves_like 'label update API' do + let(:spec_params) { { label_id: label1.id } } + let(:expected_response_label_id) { label1.id } + end end - it 'returns 200 if colors is changed' do + it 'returns 404 if label does not exist' do put api("/projects/#{project.id}/labels", user), params: { - name: 'label1', - color: '#FFFFFF' + name: 'label2', + new_name: 'label3' } - expect(response).to have_gitlab_http_status(200) - expect(json_response['name']).to eq(label1.name) - expect(json_response['color']).to eq('#FFFFFF') + + expect(response).to have_gitlab_http_status(404) end - it 'returns 200 if description is changed' do + it 'returns 404 if label by id does not exist' do put api("/projects/#{project.id}/labels", user), params: { - name: 'bug', - description: 'test' + label_id: 0, + new_name: 'label3' } - expect(response).to have_gitlab_http_status(200) - expect(json_response['name']).to eq(priority_label.name) - expect(json_response['description']).to eq('test') - expect(json_response['priority']).to eq(3) - end - - it 'returns 200 if priority is changed' do - put api("/projects/#{project.id}/labels", user), - params: { - name: 'bug', - priority: 10 - } - - expect(response.status).to eq(200) - expect(json_response['name']).to eq(priority_label.name) - expect(json_response['priority']).to eq(10) + expect(response).to have_gitlab_http_status(404) end - it 'returns 200 if a priority is added' do - put api("/projects/#{project.id}/labels", user), - params: { - name: 'label1', - priority: 3 - } + it 'returns 400 if no label name and id is given' do + put api("/projects/#{project.id}/labels", user), params: { new_name: 'label2' } - expect(response.status).to eq(200) - expect(json_response['name']).to eq(label1.name) - expect(json_response['priority']).to eq(3) + expect(response).to have_gitlab_http_status(400) + expect(json_response['error']).to eq('label_id, name are missing, exactly one parameter must be provided') end - it 'returns 200 if the priority is removed' do + it 'fails if label_id and name are given in params' do put api("/projects/#{project.id}/labels", user), params: { + label_id: label1.id, name: priority_label.name, - priority: nil + new_name: 'New Label' } - expect(response.status).to eq(200) - expect(json_response['name']).to eq(priority_label.name) - expect(json_response['priority']).to be_nil + expect(response).to have_gitlab_http_status(400) end + end - it 'returns 404 if label does not exist' do - put api("/projects/#{project.id}/labels", user), - params: { - name: 'label2', - new_name: 'label3' - } - expect(response).to have_gitlab_http_status(404) - end + describe 'PUT /projects/:id/labels/promote' do + let(:group) { create(:group) } - it 'returns 400 if no label name given' do - put api("/projects/#{project.id}/labels", user), params: { new_name: 'label2' } - expect(response).to have_gitlab_http_status(400) - expect(json_response['error']).to eq('name is missing') + before do + group.add_owner(user) + project.update!(group: group) end - it 'returns 400 if no new parameters given' do - put api("/projects/#{project.id}/labels", user), params: { name: 'label1' } - expect(response).to have_gitlab_http_status(400) - expect(json_response['error']).to eq('new_name, color, description, priority are missing, '\ - 'at least one parameter must be provided') + it 'returns 200 if label is promoted' do + put api("/projects/#{project.id}/labels/promote", user), params: { name: label1.name } + + expect(response).to have_gitlab_http_status(200) + expect(json_response['name']).to eq(label1.name) + expect(json_response['color']).to eq(label1.color) end - it 'returns 400 for invalid name' do - put api("/projects/#{project.id}/labels", user), - params: { - name: 'label1', - new_name: ',', - color: '#FFFFFF' - } - expect(response).to have_gitlab_http_status(400) - expect(json_response['message']['title']).to eq(['is invalid']) + it 'returns 200 if group label already exists' do + create(:group_label, title: label1.name, group: group) + + expect { put api("/projects/#{project.id}/labels/promote", user), params: { name: label1.name } } + .to change(project.labels, :count).by(-1) + .and change(group.labels, :count).by(0) + + expect(response).to have_gitlab_http_status(200) end - it 'returns 400 when color code is too short' do - put api("/projects/#{project.id}/labels", user), - params: { - name: 'label1', - color: '#FF' - } - expect(response).to have_gitlab_http_status(400) - expect(json_response['message']['color']).to eq(['must be a valid color code']) + it 'returns 403 if guest promotes label' do + guest = create(:user) + project.add_guest(guest) + + put api("/projects/#{project.id}/labels/promote", guest), params: { name: label1.name } + + expect(response).to have_gitlab_http_status(403) end - it 'returns 400 for too long color code' do - put api("/projects/#{project.id}/labels", user), - params: { - name: 'label1', - color: '#FFAAFFFF' - } - expect(response).to have_gitlab_http_status(400) - expect(json_response['message']['color']).to eq(['must be a valid color code']) + it 'returns 404 if label does not exist' do + put api("/projects/#{project.id}/labels/promote", user), params: { name: 'unknown' } + + expect(response).to have_gitlab_http_status(404) end - it 'returns 400 for invalid priority' do - put api("/projects/#{project.id}/labels", user), - params: { - name: 'label1', - priority: 'foo' - } + it 'returns 400 if no label name given' do + put api("/projects/#{project.id}/labels/promote", user) expect(response).to have_gitlab_http_status(400) + expect(json_response['error']).to eq('name is missing') end end diff --git a/spec/requests/api/members_spec.rb b/spec/requests/api/members_spec.rb index 55f38079b1f..26f6e705528 100644 --- a/spec/requests/api/members_spec.rb +++ b/spec/requests/api/members_spec.rb @@ -99,7 +99,7 @@ describe API::Members do end end - describe 'GET /:source_type/:id/members/all', :nested_groups do + describe 'GET /:source_type/:id/members/all' do let(:nested_user) { create(:user) } let(:project_user) { create(:user) } let(:linked_group_user) { create(:user) } @@ -238,7 +238,7 @@ describe API::Members do end context 'access levels' do - it 'does not create the member if group level is higher', :nested_groups do + it 'does not create the member if group level is higher' do parent = create(:group) group.update(parent: parent) @@ -252,7 +252,7 @@ describe API::Members do expect(json_response['message']['access_level']).to eq(["should be greater than or equal to Developer inherited membership from group #{parent.name}"]) end - it 'creates the member if group level is lower', :nested_groups do + it 'creates the member if group level is lower' do parent = create(:group) group.update(parent: parent) diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index ced853caab4..15d6db42760 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -723,7 +723,7 @@ describe API::MergeRequests do it_behaves_like 'merge requests list' - context 'when have subgroups', :nested_groups do + context 'when have subgroups' do let!(:group) { create(:group, :public) } let!(:subgroup) { create(:group, parent: group) } let!(:project) { create(:project, :public, :repository, creator: user, namespace: subgroup, only_allow_merge_if_pipeline_succeeds: false) } @@ -1571,7 +1571,7 @@ describe API::MergeRequests do end end - describe "GET /projects/:id/merge_requests/:merge_request_iid/merge_ref" do + describe "GET /projects/:id/merge_requests/:merge_request_iid/merge_ref", :clean_gitlab_redis_shared_state do before do merge_request.mark_as_unchecked! end diff --git a/spec/requests/api/notes_spec.rb b/spec/requests/api/notes_spec.rb index 424f0a82e43..6c1e30791d2 100644 --- a/spec/requests/api/notes_spec.rb +++ b/spec/requests/api/notes_spec.rb @@ -9,6 +9,13 @@ describe API::Notes do project.add_reporter(user) end + context 'when there are cross-reference system notes' do + let(:url) { "/projects/#{project.id}/merge_requests/#{merge_request.iid}/notes" } + let(:notes_in_response) { json_response } + + it_behaves_like 'with cross-reference system notes' + end + context "when noteable is an Issue" do let!(:issue) { create(:issue, project: project, author: user) } let!(:issue_note) { create(:note, noteable: issue, project: project, author: user) } diff --git a/spec/requests/api/pipelines_spec.rb b/spec/requests/api/pipelines_spec.rb index 35b3dd219f7..174b3214d13 100644 --- a/spec/requests/api/pipelines_spec.rb +++ b/spec/requests/api/pipelines_spec.rb @@ -17,6 +17,8 @@ describe API::Pipelines do end describe 'GET /projects/:id/pipelines ' do + it_behaves_like 'pipelines visibility table' + context 'authorized user' do it 'returns project pipelines' do get api("/projects/#{project.id}/pipelines", user) @@ -401,6 +403,15 @@ describe API::Pipelines do end describe 'GET /projects/:id/pipelines/:pipeline_id' do + it_behaves_like 'pipelines visibility table' do + let(:pipelines_api_path) do + "/projects/#{project.id}/pipelines/#{pipeline.id}" + end + + let(:api_response) { response_status == 200 ? response : json_response } + let(:response_200) { match_response_schema('public_api/v4/pipeline/detail') } + end + context 'authorized user' do it 'exposes known attributes' do get api("/projects/#{project.id}/pipelines/#{pipeline.id}", user) diff --git a/spec/requests/api/project_clusters_spec.rb b/spec/requests/api/project_clusters_spec.rb index a6e08ab3ab6..a7b919de2ef 100644 --- a/spec/requests/api/project_clusters_spec.rb +++ b/spec/requests/api/project_clusters_spec.rb @@ -257,12 +257,22 @@ describe API::ProjectClusters do post api("/projects/#{project.id}/clusters/user", current_user), params: cluster_params end + it 'responds with 400' do + expect(response).to have_gitlab_http_status(400) + + expect(json_response['message']['base'].first).to eq('Instance does not support multiple Kubernetes clusters') + end + end + + context 'non-authorized user' do + before do + post api("/projects/#{project.id}/clusters/user", developer_user), params: cluster_params + end + it 'responds with 403' do expect(response).to have_gitlab_http_status(403) - end - it 'returns an appropriate message' do - expect(json_response['message']).to include('Instance does not support multiple Kubernetes clusters') + expect(json_response['message']).to eq('403 Forbidden') end end end @@ -326,7 +336,6 @@ describe API::ProjectClusters do it 'does not update cluster attributes' do expect(cluster.domain).not_to eq('new_domain.com') expect(cluster.platform_kubernetes.namespace).not_to eq('invalid_namespace') - expect(cluster.kubernetes_namespace_for(project)).not_to eq('invalid_namespace') end it 'returns validation errors' do diff --git a/spec/requests/api/container_registry_spec.rb b/spec/requests/api/project_container_repositories_spec.rb index b64f3ea1081..f1dc4e6f0b2 100644 --- a/spec/requests/api/container_registry_spec.rb +++ b/spec/requests/api/project_container_repositories_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe API::ContainerRegistry do +describe API::ProjectContainerRepositories do include ExclusiveLeaseHelpers set(:project) { create(:project, :private) } @@ -12,6 +12,16 @@ describe API::ContainerRegistry do let(:root_repository) { create(:container_repository, :root, project: project) } let(:test_repository) { create(:container_repository, project: project) } + let(:users) do + { + anonymous: nil, + developer: developer, + guest: guest, + maintainer: maintainer, + reporter: reporter + } + end + let(:api_user) { maintainer } before do @@ -27,57 +37,24 @@ describe API::ContainerRegistry do test_repository end - shared_examples 'being disallowed' do |param| - context "for #{param}" do - let(:api_user) { public_send(param) } - - it 'returns access denied' do - subject - - expect(response).to have_gitlab_http_status(:forbidden) - end - end - - context "for anonymous" do - let(:api_user) { nil } - - it 'returns not found' do - subject - - expect(response).to have_gitlab_http_status(:not_found) - end - end - end - describe 'GET /projects/:id/registry/repositories' do - subject { get api("/projects/#{project.id}/registry/repositories", api_user) } - - it_behaves_like 'being disallowed', :guest - - context 'for reporter' do - let(:api_user) { reporter } - - it 'returns a list of repositories' do - subject + let(:url) { "/projects/#{project.id}/registry/repositories" } - expect(json_response.length).to eq(2) - expect(json_response.map { |repository| repository['id'] }).to contain_exactly( - root_repository.id, test_repository.id) - end + subject { get api(url, api_user) } - it 'returns a matching schema' do - subject + it_behaves_like 'rejected container repository access', :guest, :forbidden + it_behaves_like 'rejected container repository access', :anonymous, :not_found - expect(response).to have_gitlab_http_status(:ok) - expect(response).to match_response_schema('registry/repositories') - end + it_behaves_like 'returns repositories for allowed users', :reporter, 'project' do + let(:object) { project } end end describe 'DELETE /projects/:id/registry/repositories/:repository_id' do subject { delete api("/projects/#{project.id}/registry/repositories/#{root_repository.id}", api_user) } - it_behaves_like 'being disallowed', :developer + it_behaves_like 'rejected container repository access', :developer, :forbidden + it_behaves_like 'rejected container repository access', :anonymous, :not_found context 'for maintainer' do let(:api_user) { maintainer } @@ -96,7 +73,8 @@ describe API::ContainerRegistry do describe 'GET /projects/:id/registry/repositories/:repository_id/tags' do subject { get api("/projects/#{project.id}/registry/repositories/#{root_repository.id}/tags", api_user) } - it_behaves_like 'being disallowed', :guest + it_behaves_like 'rejected container repository access', :guest, :forbidden + it_behaves_like 'rejected container repository access', :anonymous, :not_found context 'for reporter' do let(:api_user) { reporter } @@ -124,10 +102,13 @@ describe API::ContainerRegistry do describe 'DELETE /projects/:id/registry/repositories/:repository_id/tags' do subject { delete api("/projects/#{project.id}/registry/repositories/#{root_repository.id}/tags", api_user), params: params } - it_behaves_like 'being disallowed', :developer do + context 'disallowed' do let(:params) do { name_regex: 'v10.*' } end + + it_behaves_like 'rejected container repository access', :developer, :forbidden + it_behaves_like 'rejected container repository access', :anonymous, :not_found end context 'for maintainer' do @@ -191,7 +172,8 @@ describe API::ContainerRegistry do describe 'GET /projects/:id/registry/repositories/:repository_id/tags/:tag_name' do subject { get api("/projects/#{project.id}/registry/repositories/#{root_repository.id}/tags/rootA", api_user) } - it_behaves_like 'being disallowed', :guest + it_behaves_like 'rejected container repository access', :guest, :forbidden + it_behaves_like 'rejected container repository access', :anonymous, :not_found context 'for reporter' do let(:api_user) { reporter } @@ -222,7 +204,8 @@ describe API::ContainerRegistry do describe 'DELETE /projects/:id/registry/repositories/:repository_id/tags/:tag_name' do subject { delete api("/projects/#{project.id}/registry/repositories/#{root_repository.id}/tags/rootA", api_user) } - it_behaves_like 'being disallowed', :reporter + it_behaves_like 'rejected container repository access', :reporter, :forbidden + it_behaves_like 'rejected container repository access', :anonymous, :not_found context 'for developer' do let(:api_user) { developer } diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index a2aae257352..5465fe0c366 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -46,8 +46,6 @@ shared_examples 'languages and percentages JSON response' do end describe API::Projects do - include ExternalAuthorizationServiceHelpers - let(:user) { create(:user) } let(:user2) { create(:user) } let(:user3) { create(:user) } @@ -840,6 +838,28 @@ describe API::Projects do end end + describe 'GET /users/:user_id/starred_projects/' do + before do + user3.update(starred_projects: [project, project2, project3]) + end + + it 'returns error when user not found' do + get api('/users/9999/starred_projects/') + + expect(response).to have_gitlab_http_status(404) + expect(json_response['message']).to eq('404 User Not Found') + end + + it 'returns projects filtered by user' do + get api("/users/#{user3.id}/starred_projects/", user) + + expect(response).to have_gitlab_http_status(200) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.map { |project| project['id'] }).to contain_exactly(project.id, project2.id, project3.id) + end + end + describe 'POST /projects/user/:id' do it 'creates new project without path but with name and return 201' do expect { post api("/projects/user/#{user.id}", admin), params: { name: 'Foo Project' } }.to change { Project.count }.by(1) @@ -1359,7 +1379,7 @@ describe API::Projects do end end - context 'nested group project', :nested_groups do + context 'nested group project' do let(:group) { create(:group) } let(:nested_group) { create(:group, parent: group) } let(:project2) { create(:project, group: nested_group) } @@ -1425,39 +1445,6 @@ describe API::Projects do end end end - - context 'with external authorization' do - let(:project) do - create(:project, - namespace: user.namespace, - external_authorization_classification_label: 'the-label') - end - - context 'when the user has access to the project' do - before do - external_service_allow_access(user, project) - end - - it 'includes the label in the response' do - get api("/projects/#{project.id}", user) - - expect(response).to have_gitlab_http_status(200) - expect(json_response['external_authorization_classification_label']).to eq('the-label') - end - end - - context 'when the external service denies access' do - before do - external_service_deny_access(user, project) - end - - it 'returns a 404' do - get api("/projects/#{project.id}", user) - - expect(response).to have_gitlab_http_status(404) - end - end - end end describe 'GET /projects/:id/users' do @@ -1507,6 +1494,17 @@ describe API::Projects do expect(response).to have_gitlab_http_status(404) end + + it 'filters out users listed in skip_users' do + other_user = create(:user) + project.team.add_developer(other_user) + + get api("/projects/#{project.id}/users?skip_users=#{user.id}", user) + + expect(response).to have_gitlab_http_status(200) + expect(json_response.size).to eq(1) + expect(json_response[0]['id']).to eq(other_user.id) + end end end @@ -2061,20 +2059,6 @@ describe API::Projects do expect(response).to have_gitlab_http_status(403) end end - - context 'when updating external classification' do - before do - enable_external_authorization_service_check - end - - it 'updates the classification label' do - put(api("/projects/#{project.id}", user), params: { external_authorization_classification_label: 'new label' }) - - expect(response).to have_gitlab_http_status(200) - - expect(project.reload.external_authorization_classification_label).to eq('new label') - end - end end describe 'POST /projects/:id/archive' do @@ -2197,6 +2181,85 @@ describe API::Projects do end end + describe 'GET /projects/:id/starrers' do + shared_examples_for 'project starrers response' do + it 'returns an array of starrers' do + get api("/projects/#{public_project.id}/starrers", current_user) + + expect(response).to have_gitlab_http_status(200) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response[0]['starred_since']).to be_present + expect(json_response[0]['user']).to be_present + end + + it 'returns the proper security headers' do + get api('/projects/1/starrers', current_user) + + expect(response).to include_security_headers + end + end + + let(:public_project) { create(:project, :public) } + let(:private_user) { create(:user, private_profile: true) } + + before do + user.update(starred_projects: [public_project]) + private_user.update(starred_projects: [public_project]) + end + + it 'returns not_found(404) for not existing project' do + get api("/projects/9999999999/starrers", user) + + expect(response).to have_gitlab_http_status(:not_found) + end + + context 'public project without user' do + it_behaves_like 'project starrers response' do + let(:current_user) { nil } + end + + it 'returns only starrers with a public profile' do + get api("/projects/#{public_project.id}/starrers", nil) + + user_ids = json_response.map { |s| s['user']['id'] } + expect(user_ids).to include(user.id) + expect(user_ids).not_to include(private_user.id) + end + end + + context 'public project with user with private profile' do + it_behaves_like 'project starrers response' do + let(:current_user) { private_user } + end + + it 'returns current user with a private profile' do + get api("/projects/#{public_project.id}/starrers", private_user) + + user_ids = json_response.map { |s| s['user']['id'] } + expect(user_ids).to include(user.id, private_user.id) + end + end + + context 'private project' do + context 'with unauthorized user' do + it 'returns not_found for existing but unauthorized project' do + get api("/projects/#{project3.id}/starrers", user3) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'without user' do + it 'returns not_found for existing but unauthorized project' do + get api("/projects/#{project3.id}/starrers", nil) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + end + describe 'GET /projects/:id/languages' do context 'with an authorized user' do it_behaves_like 'languages and percentages JSON response' do diff --git a/spec/requests/api/resource_label_events_spec.rb b/spec/requests/api/resource_label_events_spec.rb index 37b46eaeb86..25bea627b0c 100644 --- a/spec/requests/api/resource_label_events_spec.rb +++ b/spec/requests/api/resource_label_events_spec.rb @@ -4,55 +4,12 @@ require 'spec_helper' describe API::ResourceLabelEvents do set(:user) { create(:user) } - set(:project) { create(:project, :public, :repository, namespace: user.namespace) } - set(:private_user) { create(:user) } + set(:project) { create(:project, :public, namespace: user.namespace) } before do project.add_developer(user) end - shared_examples 'resource_label_events API' do |parent_type, eventable_type, id_name| - describe "GET /#{parent_type}/:id/#{eventable_type}/:noteable_id/resource_label_events" do - it "returns an array of resource label events" do - get api("/#{parent_type}/#{parent.id}/#{eventable_type}/#{eventable[id_name]}/resource_label_events", user) - - expect(response).to have_gitlab_http_status(200) - expect(response).to include_pagination_headers - expect(json_response).to be_an Array - expect(json_response.first['id']).to eq(event.id) - end - - it "returns a 404 error when eventable id not found" do - get api("/#{parent_type}/#{parent.id}/#{eventable_type}/12345/resource_label_events", user) - - expect(response).to have_gitlab_http_status(404) - end - - it "returns 404 when not authorized" do - parent.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) - - get api("/#{parent_type}/#{parent.id}/#{eventable_type}/#{eventable[id_name]}/resource_label_events", private_user) - - expect(response).to have_gitlab_http_status(404) - end - end - - describe "GET /#{parent_type}/:id/#{eventable_type}/:noteable_id/resource_label_events/:event_id" do - it "returns a resource label event by id" do - get api("/#{parent_type}/#{parent.id}/#{eventable_type}/#{eventable[id_name]}/resource_label_events/#{event.id}", user) - - expect(response).to have_gitlab_http_status(200) - expect(json_response['id']).to eq(event.id) - end - - it "returns a 404 error if resource label event not found" do - get api("/#{parent_type}/#{parent.id}/#{eventable_type}/#{eventable[id_name]}/resource_label_events/12345", user) - - expect(response).to have_gitlab_http_status(404) - end - end - end - context 'when eventable is an Issue' do let(:issue) { create(:issue, project: project, author: user) } diff --git a/spec/requests/api/search_spec.rb b/spec/requests/api/search_spec.rb index 3e0b478abb3..8abdcaa2e0e 100644 --- a/spec/requests/api/search_spec.rb +++ b/spec/requests/api/search_spec.rb @@ -89,7 +89,7 @@ describe API::Search do it 'returns empty array' do get api('/search', user), params: { scope: 'milestones', search: 'awesome' } - milestones = JSON.parse(response.body) + milestones = json_response expect(milestones).to be_empty end @@ -356,7 +356,7 @@ describe API::Search do it 'returns empty array' do get api("/projects/#{project.id}/search", user), params: { scope: 'milestones', search: 'awesome' } - milestones = JSON.parse(response.body) + milestones = json_response expect(milestones).to be_empty end diff --git a/spec/requests/api/services_spec.rb b/spec/requests/api/services_spec.rb index 3f79e332b90..76a70ab6e9e 100644 --- a/spec/requests/api/services_spec.rb +++ b/spec/requests/api/services_spec.rb @@ -10,10 +10,7 @@ describe API::Services do end Service.available_services_names.each do |service| - # TODO: Remove below `if: (service != "kubernetes")` in the next release - # KubernetesService was deprecated and it can't be updated. Right now it's - # only readable. It should be completely removed in the next iteration. - describe "PUT /projects/:id/services/#{service.dasherize}", if: (service != "kubernetes") do + describe "PUT /projects/:id/services/#{service.dasherize}" do include_context service it "updates #{service} settings" do @@ -62,10 +59,7 @@ describe API::Services do end end - # TODO: Remove below `if: (service != "kubernetes")` in the next release - # KubernetesService was deprecated and it can't be updated. Right now it's - # only readable. It should be completely removed in the next iteration. - describe "DELETE /projects/:id/services/#{service.dasherize}", if: (service != "kubernetes") do + describe "DELETE /projects/:id/services/#{service.dasherize}" do include_context service before do @@ -85,9 +79,7 @@ describe API::Services do include_context service # inject some properties into the service - before do - initialize_service(service) - end + let!(:initialized_service) { initialize_service(service) } it 'returns authentication error when unauthenticated' do get api("/projects/#{project.id}/services/#{dashed_service}") @@ -108,6 +100,15 @@ describe API::Services do expect(json_response['properties'].keys).to match_array(service_instance.api_field_names) end + it "returns empty hash if properties are empty" do + # deprecated services are not valid for update + initialized_service.update_attribute(:properties, {}) + get api("/projects/#{project.id}/services/#{dashed_service}", user) + + expect(response).to have_gitlab_http_status(200) + expect(json_response['properties'].keys).to be_empty + end + it "returns error when authenticated but not a project owner" do project.add_developer(user2) get api("/projects/#{project.id}/services/#{dashed_service}", user2) diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb index 8a60980fe80..590107d5161 100644 --- a/spec/requests/api/settings_spec.rb +++ b/spec/requests/api/settings_spec.rb @@ -25,6 +25,9 @@ describe API::Settings, 'Settings' do expect(json_response['ed25519_key_restriction']).to eq(0) expect(json_response['performance_bar_allowed_group_id']).to be_nil expect(json_response['instance_statistics_visibility_private']).to be(false) + expect(json_response['allow_local_requests_from_hooks_and_services']).to be(false) + expect(json_response['allow_local_requests_from_web_hooks_and_services']).to be(false) + expect(json_response['allow_local_requests_from_system_hooks']).to be(true) expect(json_response).not_to have_key('performance_bar_allowed_group_path') expect(json_response).not_to have_key('performance_bar_enabled') end @@ -67,7 +70,9 @@ describe API::Settings, 'Settings' do instance_statistics_visibility_private: true, diff_max_patch_bytes: 150_000, default_branch_protection: ::Gitlab::Access::PROTECTION_DEV_CAN_MERGE, - local_markdown_version: 3 + local_markdown_version: 3, + allow_local_requests_from_web_hooks_and_services: true, + allow_local_requests_from_system_hooks: false } expect(response).to have_gitlab_http_status(200) @@ -95,6 +100,8 @@ describe API::Settings, 'Settings' do expect(json_response['diff_max_patch_bytes']).to eq(150_000) expect(json_response['default_branch_protection']).to eq(Gitlab::Access::PROTECTION_DEV_CAN_MERGE) expect(json_response['local_markdown_version']).to eq(3) + expect(json_response['allow_local_requests_from_web_hooks_and_services']).to eq(true) + expect(json_response['allow_local_requests_from_system_hooks']).to eq(false) end end @@ -117,6 +124,14 @@ describe API::Settings, 'Settings' do expect(json_response['performance_bar_allowed_group_id']).to be_nil end + it 'supports legacy allow_local_requests_from_hooks_and_services' do + put api("/application/settings", admin), + params: { allow_local_requests_from_hooks_and_services: true } + + expect(response).to have_gitlab_http_status(200) + expect(json_response['allow_local_requests_from_hooks_and_services']).to eq(true) + end + context 'external policy classification settings' do let(:settings) do { @@ -129,6 +144,7 @@ describe API::Settings, 'Settings' do external_auth_client_key_pass: "5iveL!fe" } end + let(:attribute_names) { settings.keys.map(&:to_s) } it 'includes the attributes in the API' do @@ -150,6 +166,56 @@ describe API::Settings, 'Settings' do end end + context "snowplow tracking settings" do + let(:settings) do + { + snowplow_collector_hostname: "snowplow.example.com", + snowplow_cookie_domain: ".example.com", + snowplow_enabled: true, + snowplow_site_id: "site_id" + } + end + + let(:attribute_names) { settings.keys.map(&:to_s) } + + it "includes the attributes in the API" do + get api("/application/settings", admin) + + expect(response).to have_gitlab_http_status(200) + attribute_names.each do |attribute| + expect(json_response.keys).to include(attribute) + end + end + + it "allows updating the settings" do + put api("/application/settings", admin), params: settings + + expect(response).to have_gitlab_http_status(200) + settings.each do |attribute, value| + expect(ApplicationSetting.current.public_send(attribute)).to eq(value) + end + end + + context "missing snowplow_collector_hostname value when snowplow_enabled is true" do + it "returns a blank parameter error message" do + put api("/application/settings", admin), params: { snowplow_enabled: true } + + expect(response).to have_gitlab_http_status(400) + expect(json_response["error"]).to eq("snowplow_collector_hostname is missing") + end + + it "handles validation errors" do + put api("/application/settings", admin), params: settings.merge({ + snowplow_collector_hostname: nil + }) + + expect(response).to have_gitlab_http_status(400) + message = json_response["message"] + expect(message["snowplow_collector_hostname"]).to include("can't be blank") + end + end + end + context "missing plantuml_url value when plantuml_enabled is true" do it "returns a blank parameter error message" do put api("/application/settings", admin), params: { plantuml_enabled: true } diff --git a/spec/requests/api/triggers_spec.rb b/spec/requests/api/triggers_spec.rb index f0f01e97f1d..8ea3d16a41f 100644 --- a/spec/requests/api/triggers_spec.rb +++ b/spec/requests/api/triggers_spec.rb @@ -270,34 +270,6 @@ describe API::Triggers do end end - describe 'POST /projects/:id/triggers/:trigger_id/take_ownership' do - context 'authenticated user with valid permissions' do - it 'updates owner' do - post api("/projects/#{project.id}/triggers/#{trigger.id}/take_ownership", user) - - expect(response).to have_gitlab_http_status(200) - expect(json_response).to include('owner') - expect(trigger.reload.owner).to eq(user) - end - end - - context 'authenticated user with invalid permissions' do - it 'does not update owner' do - post api("/projects/#{project.id}/triggers/#{trigger.id}/take_ownership", user2) - - expect(response).to have_gitlab_http_status(403) - end - end - - context 'unauthenticated user' do - it 'does not update owner' do - post api("/projects/#{project.id}/triggers/#{trigger.id}/take_ownership") - - expect(response).to have_gitlab_http_status(401) - end - end - end - describe 'DELETE /projects/:id/triggers/:trigger_id' do context 'authenticated user with valid permissions' do it 'deletes trigger' do diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 46925daf40a..af2bee4563a 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -448,6 +448,7 @@ describe API::Users do it "returns 201 Created on success" do post api("/users", admin), params: attributes_for(:user, projects_limit: 3) + expect(response).to match_response_schema('public_api/v4/user/admin') expect(response).to have_gitlab_http_status(201) end @@ -643,6 +644,13 @@ describe API::Users do describe "PUT /users/:id" do let!(:admin_user) { create(:admin) } + it "returns 200 OK on success" do + put api("/users/#{user.id}", admin), params: { bio: 'new test bio' } + + expect(response).to match_response_schema('public_api/v4/user/admin') + expect(response).to have_gitlab_http_status(200) + end + it "updates user with new bio" do put api("/users/#{user.id}", admin), params: { bio: 'new test bio' } @@ -745,6 +753,14 @@ describe API::Users do expect(user.reload.private_profile).to eq(true) end + it "updates private profile when nil is given to false" do + admin.update(private_profile: true) + + put api("/users/#{user.id}", admin), params: { private_profile: nil } + + expect(user.reload.private_profile).to eq(false) + end + it "does not update admin status" do put api("/users/#{admin_user.id}", admin), params: { can_create_group: false } diff --git a/spec/requests/api/variables_spec.rb b/spec/requests/api/variables_spec.rb index 55b1419a004..69f105b71a8 100644 --- a/spec/requests/api/variables_spec.rb +++ b/spec/requests/api/variables_spec.rb @@ -106,6 +106,30 @@ describe API::Variables do expect(response).to have_gitlab_http_status(400) end + + it 'creates variable with a specific environment scope' do + expect do + post api("/projects/#{project.id}/variables", user), params: { key: 'TEST_VARIABLE_2', value: 'VALUE_2', environment_scope: 'review/*' } + end.to change { project.variables.reload.count }.by(1) + + expect(response).to have_gitlab_http_status(201) + expect(json_response['key']).to eq('TEST_VARIABLE_2') + expect(json_response['value']).to eq('VALUE_2') + expect(json_response['environment_scope']).to eq('review/*') + end + + it 'allows duplicated variable key given different environment scopes' do + variable = create(:ci_variable, project: project) + + expect do + post api("/projects/#{project.id}/variables", user), params: { key: variable.key, value: 'VALUE_2', environment_scope: 'review/*' } + end.to change { project.variables.reload.count }.by(1) + + expect(response).to have_gitlab_http_status(201) + expect(json_response['key']).to eq(variable.key) + expect(json_response['value']).to eq('VALUE_2') + expect(json_response['environment_scope']).to eq('review/*') + end end context 'authorized user with invalid permissions' do diff --git a/spec/requests/jwt_controller_spec.rb b/spec/requests/jwt_controller_spec.rb index bba473f1c20..8b2c698fee1 100644 --- a/spec/requests/jwt_controller_spec.rb +++ b/spec/requests/jwt_controller_spec.rb @@ -108,6 +108,14 @@ describe JwtController do end end end + + it 'does not cause session based checks to be activated' do + expect(Gitlab::Session).not_to receive(:with_session) + + get '/jwt/auth', params: parameters, headers: headers + + expect(response).to have_gitlab_http_status(200) + end end context 'using invalid login' do diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb index 1781759c54b..dc25e4d808e 100644 --- a/spec/requests/lfs_http_spec.rb +++ b/spec/requests/lfs_http_spec.rb @@ -1439,8 +1439,4 @@ describe 'Git LFS API and storage' do post(url, params: params, headers: headers) end - - def json_response - @json_response ||= JSON.parse(response.body) - end end diff --git a/spec/requests/lfs_locks_api_spec.rb b/spec/requests/lfs_locks_api_spec.rb index 5b7b3d2fdd6..11436e5cd0c 100644 --- a/spec/requests/lfs_locks_api_spec.rb +++ b/spec/requests/lfs_locks_api_spec.rb @@ -163,8 +163,4 @@ describe 'Git LFS File Locking API' do def do_get(url, params = nil, headers = nil) get(url, params: (params || {}), headers: (headers || {}).merge('Content-Type' => LfsRequest::CONTENT_TYPE)) end - - def json_response - @json_response ||= JSON.parse(response.body) - end end diff --git a/spec/requests/openid_connect_spec.rb b/spec/requests/openid_connect_spec.rb index 86e41cbdf00..025568d8bea 100644 --- a/spec/requests/openid_connect_spec.rb +++ b/spec/requests/openid_connect_spec.rb @@ -104,7 +104,7 @@ describe 'OpenID Connect requests' do expect(json_response).to match(id_token_claims.merge(user_info_claims)) expected_groups = [group1.full_path, group3.full_path] - expected_groups << group4.full_path if Group.supports_nested_objects? + expected_groups << group4.full_path expect(json_response['groups']).to match_array(expected_groups) end diff --git a/spec/requests/rack_attack_global_spec.rb b/spec/requests/rack_attack_global_spec.rb index d832963292c..478f09a7881 100644 --- a/spec/requests/rack_attack_global_spec.rb +++ b/spec/requests/rack_attack_global_spec.rb @@ -112,9 +112,9 @@ describe 'Rack Attack global throttles' do arguments = { message: 'Rack_Attack', env: :throttle, - ip: '127.0.0.1', + remote_ip: '127.0.0.1', request_method: 'GET', - fullpath: get_args.first, + path: get_args.first, user_id: user.id, username: user.username } @@ -213,9 +213,9 @@ describe 'Rack Attack global throttles' do arguments = { message: 'Rack_Attack', env: :throttle, - ip: '127.0.0.1', + remote_ip: '127.0.0.1', request_method: 'GET', - fullpath: '/users/sign_in' + path: '/users/sign_in' } expect(Gitlab::AuthLogger).to receive(:error).with(arguments) @@ -377,9 +377,9 @@ describe 'Rack Attack global throttles' do arguments = { message: 'Rack_Attack', env: :throttle, - ip: '127.0.0.1', + remote_ip: '127.0.0.1', request_method: 'GET', - fullpath: '/dashboard/snippets', + path: '/dashboard/snippets', user_id: user.id, username: user.username } diff --git a/spec/requests/request_profiler_spec.rb b/spec/requests/request_profiler_spec.rb index 75b22b1879b..851affbcf88 100644 --- a/spec/requests/request_profiler_spec.rb +++ b/spec/requests/request_profiler_spec.rb @@ -3,13 +3,18 @@ require 'spec_helper' describe 'Request Profiler' do let(:user) { create(:user) } - shared_examples 'profiling a request' do + shared_examples 'profiling a request' do |profile_type, extension| before do allow(Rails).to receive(:cache).and_return(ActiveSupport::Cache::MemoryStore.new) allow(RubyProf::Profile).to receive(:profile) do |&blk| blk.call RubyProf::Profile.new end + allow(MemoryProfiler).to receive(:report) do |&blk| + blk.call + MemoryProfiler.start + MemoryProfiler.stop + end end it 'creates a profile of the request' do @@ -18,10 +23,11 @@ describe 'Request Profiler' do path = "/#{project.full_path}" Timecop.freeze(time) do - get path, params: {}, headers: { 'X-Profile-Token' => Gitlab::RequestProfiler.profile_token } + get path, params: {}, headers: { 'X-Profile-Token' => Gitlab::RequestProfiler.profile_token, 'X-Profile-Mode' => profile_type } end - profile_path = "#{Gitlab.config.shared.path}/tmp/requests_profiles/#{path.tr('/', '|')}_#{time.to_i}.html" + profile_type = 'execution' if profile_type.nil? + profile_path = "#{Gitlab.config.shared.path}/tmp/requests_profiles/#{path.tr('/', '|')}_#{time.to_i}_#{profile_type}.#{extension}" expect(File.exist?(profile_path)).to be true end @@ -35,10 +41,14 @@ describe 'Request Profiler' do login_as(user) end - include_examples 'profiling a request' + include_examples 'profiling a request', 'execution', 'html' + include_examples 'profiling a request', nil, 'html' + include_examples 'profiling a request', 'memory', 'txt' end context "when user is not logged-in" do - include_examples 'profiling a request' + include_examples 'profiling a request', 'execution', 'html' + include_examples 'profiling a request', nil, 'html' + include_examples 'profiling a request', 'memory', 'txt' end end |