From 6a0ea605e8b48deacbb4e93f7bb1d9b9abd2f7f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mehmet=20Emin=20=C4=B0NA=C3=87?= Date: Wed, 16 Mar 2016 03:16:25 +0200 Subject: Change deprecated usage of rendering without response body `render nothing: true` has been deprecated. For more information see [pr](https://github.com/rails/rails/pull/20336) --- spec/controllers/projects/raw_controller_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec') diff --git a/spec/controllers/projects/raw_controller_spec.rb b/spec/controllers/projects/raw_controller_spec.rb index 1caa476d37d..fb29274c687 100644 --- a/spec/controllers/projects/raw_controller_spec.rb +++ b/spec/controllers/projects/raw_controller_spec.rb @@ -42,7 +42,7 @@ describe Projects::RawController do before do public_project.lfs_objects << lfs_object allow_any_instance_of(LfsObjectUploader).to receive(:exists?).and_return(true) - allow(controller).to receive(:send_file) { controller.render nothing: true } + allow(controller).to receive(:send_file) { controller.head :ok } end it 'serves the file' do -- cgit v1.2.1 From 8dc19494c3fdae366daa8849b5e2a3f58f98878c Mon Sep 17 00:00:00 2001 From: Long Nguyen Date: Thu, 5 May 2016 13:26:36 +0700 Subject: Remove unused code, update spec, and update changelog --- spec/routing/routing_spec.rb | 8 -------- 1 file changed, 8 deletions(-) (limited to 'spec') diff --git a/spec/routing/routing_spec.rb b/spec/routing/routing_spec.rb index 1527eddfa48..9deffd0a1e3 100644 --- a/spec/routing/routing_spec.rb +++ b/spec/routing/routing_spec.rb @@ -27,18 +27,10 @@ end # PUT /snippets/:id(.:format) snippets#update # DELETE /snippets/:id(.:format) snippets#destroy describe SnippetsController, "routing" do - it "to #user_index" do - expect(get("/s/User")).to route_to('snippets#index', username: 'User') - end - it "to #raw" do expect(get("/snippets/1/raw")).to route_to('snippets#raw', id: '1') end - it "to #index" do - expect(get("/snippets")).to route_to('snippets#index') - end - it "to #create" do expect(post("/snippets")).to route_to('snippets#create') end -- cgit v1.2.1 From ffda8a1a0eb273e62fcb0197f352400946571778 Mon Sep 17 00:00:00 2001 From: Long Nguyen Date: Sun, 8 May 2016 15:27:33 +0700 Subject: user routings refactor --- spec/routing/routing_spec.rb | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'spec') diff --git a/spec/routing/routing_spec.rb b/spec/routing/routing_spec.rb index 9deffd0a1e3..543088fa088 100644 --- a/spec/routing/routing_spec.rb +++ b/spec/routing/routing_spec.rb @@ -31,6 +31,10 @@ describe SnippetsController, "routing" do expect(get("/snippets/1/raw")).to route_to('snippets#raw', id: '1') end + it "to #index" do + expect(get("/snippets")).to route_to('snippets#index') + end + it "to #create" do expect(post("/snippets")).to route_to('snippets#create') end -- cgit v1.2.1 From 6781c1b4ba8bb1a7da0a4bf637d7a311abf281f0 Mon Sep 17 00:00:00 2001 From: Long Nguyen Date: Sun, 8 May 2016 22:06:19 +0700 Subject: Add specs for user routing and update spec for user controller --- spec/controllers/users_controller_spec.rb | 22 ++++++++++++++++++ spec/routing/routing_spec.rb | 37 +++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+) (limited to 'spec') diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 8045c8b940d..c61ec174665 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -112,4 +112,26 @@ describe UsersController do expect(response).to render_template('calendar_activities') end end + + describe 'GET #snippets' do + before do + sign_in(user) + end + + context 'format html' do + it 'renders snippets page' do + get :snippets, username: user.username + expect(response.status).to eq(200) + expect(response).to render_template('show') + end + end + + context 'format json' do + it 'response with snippets json data' do + get :snippets, username: user.username, format: :json + expect(response.status).to eq(200) + expect(JSON.parse(response.body)).to have_key('html') + end + end + end end diff --git a/spec/routing/routing_spec.rb b/spec/routing/routing_spec.rb index 543088fa088..8530a2f31d5 100644 --- a/spec/routing/routing_spec.rb +++ b/spec/routing/routing_spec.rb @@ -1,5 +1,42 @@ require 'spec_helper' +# user GET /u/:username/ +# user_groups GET /u/:username/groups(.:format) +# user_projects GET /u/:username/projects(.:format) +# user_contributed_projects GET /u/:username/contributed(.:format) +# user_snippets GET /u/:username/snippets(.:format) +# user_calendar GET /u/:username/calendar(.:format) +# user_calendar_activities GET /u/:username/calendar_activities(.:format) +describe UsersController, "routing" do + it "to #show" do + expect(get("/u/User")).to route_to('users#show', username: 'User') + end + + it "to #groups" do + expect(get("/u/User/groups")).to route_to('users#groups', username: 'User') + end + + it "to #projects" do + expect(get("/u/User/projects")).to route_to('users#projects', username: 'User') + end + + it "to #contributed" do + expect(get("/u/User/contributed")).to route_to('users#contributed', username: 'User') + end + + it "to #snippets" do + expect(get("/u/User/snippets")).to route_to('users#snippets', username: 'User') + end + + it "to #calendar" do + expect(get("/u/User/calendar")).to route_to('users#calendar', username: 'User') + end + + it "to #calendar_activities" do + expect(get("/u/User/calendar_activities")).to route_to('users#calendar_activities', username: 'User') + end +end + # search GET /search(.:format) search#show describe SearchController, "routing" do it "to #show" do -- cgit v1.2.1 From 9cc0937b3a41caca89fa6722149248a8f7b0a447 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Sun, 8 May 2016 15:33:34 -0600 Subject: Enable the Rubocop DeprecatedClassMethods cop This reports uses of `File.exists?` and `Dir.exists?`, which were both deprecated in Ruby and will eventually be removed in favor of `.exist?`. Also fixes all existing uses of the deprecated methods. --- spec/config/mail_room_spec.rb | 2 +- spec/services/projects/create_service_spec.rb | 4 ++-- spec/services/projects/destroy_service_spec.rb | 8 ++++---- 3 files changed, 7 insertions(+), 7 deletions(-) (limited to 'spec') diff --git a/spec/config/mail_room_spec.rb b/spec/config/mail_room_spec.rb index 462afb24f08..6fad7e2b9e7 100644 --- a/spec/config/mail_room_spec.rb +++ b/spec/config/mail_room_spec.rb @@ -43,7 +43,7 @@ describe "mail_room.yml" do redis_config_file = Rails.root.join('config', 'resque.yml') redis_url = - if File.exists?(redis_config_file) + if File.exist?(redis_config_file) YAML.load_file(redis_config_file)[Rails.env] else "redis://localhost:6379" diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index e43903dbd3c..fd114359467 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -64,7 +64,7 @@ describe Projects::CreateService, services: true do @path = ProjectWiki.new(@project, @user).send(:path_to_repo) end - it { expect(File.exists?(@path)).to be_truthy } + it { expect(File.exist?(@path)).to be_truthy } end context 'wiki_enabled false does not create wiki repository directory' do @@ -74,7 +74,7 @@ describe Projects::CreateService, services: true do @path = ProjectWiki.new(@project, @user).send(:path_to_repo) end - it { expect(File.exists?(@path)).to be_falsey } + it { expect(File.exist?(@path)).to be_falsey } end end diff --git a/spec/services/projects/destroy_service_spec.rb b/spec/services/projects/destroy_service_spec.rb index 1ec27077717..a5cb6f382e4 100644 --- a/spec/services/projects/destroy_service_spec.rb +++ b/spec/services/projects/destroy_service_spec.rb @@ -13,8 +13,8 @@ describe Projects::DestroyService, services: true do end it { expect(Project.all).not_to include(project) } - it { expect(Dir.exists?(path)).to be_falsey } - it { expect(Dir.exists?(remove_path)).to be_falsey } + it { expect(Dir.exist?(path)).to be_falsey } + it { expect(Dir.exist?(remove_path)).to be_falsey } end context 'Sidekiq fake' do @@ -24,8 +24,8 @@ describe Projects::DestroyService, services: true do end it { expect(Project.all).not_to include(project) } - it { expect(Dir.exists?(path)).to be_falsey } - it { expect(Dir.exists?(remove_path)).to be_truthy } + it { expect(Dir.exist?(path)).to be_falsey } + it { expect(Dir.exist?(remove_path)).to be_truthy } end def destroy_project(project, user, params) -- cgit v1.2.1 From baef6728fa4e8e515ccdeba1ea54da996f322aab Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Mon, 9 May 2016 19:59:45 +0300 Subject: Send trace to a browser incrementally when build is running We send a state of ansi2html to client, client needs to send this state back. The state describes the configuration of generator and position within trace. --- spec/lib/ci/ansi2html_spec.rb | 111 +++++++++++++++++++++++++++++------------- 1 file changed, 78 insertions(+), 33 deletions(-) (limited to 'spec') diff --git a/spec/lib/ci/ansi2html_spec.rb b/spec/lib/ci/ansi2html_spec.rb index 3a2b568f4c7..04afbd06929 100644 --- a/spec/lib/ci/ansi2html_spec.rb +++ b/spec/lib/ci/ansi2html_spec.rb @@ -4,131 +4,176 @@ describe Ci::Ansi2html, lib: true do subject { Ci::Ansi2html } it "prints non-ansi as-is" do - expect(subject.convert("Hello")).to eq('Hello') + expect(subject.convert("Hello")[:html]).to eq('Hello') end it "strips non-color-changing controll sequences" do - expect(subject.convert("Hello \e[2Kworld")).to eq('Hello world') + expect(subject.convert("Hello \e[2Kworld")[:html]).to eq('Hello world') end it "prints simply red" do - expect(subject.convert("\e[31mHello\e[0m")).to eq('Hello') + expect(subject.convert("\e[31mHello\e[0m")[:html]).to eq('Hello') end it "prints simply red without trailing reset" do - expect(subject.convert("\e[31mHello")).to eq('Hello') + expect(subject.convert("\e[31mHello")[:html]).to eq('Hello') end it "prints simply yellow" do - expect(subject.convert("\e[33mHello\e[0m")).to eq('Hello') + expect(subject.convert("\e[33mHello\e[0m")[:html]).to eq('Hello') end it "prints default on blue" do - expect(subject.convert("\e[39;44mHello")).to eq('Hello') + expect(subject.convert("\e[39;44mHello")[:html]).to eq('Hello') end it "prints red on blue" do - expect(subject.convert("\e[31;44mHello")).to eq('Hello') + expect(subject.convert("\e[31;44mHello")[:html]).to eq('Hello') end it "resets colors after red on blue" do - expect(subject.convert("\e[31;44mHello\e[0m world")).to eq('Hello world') + expect(subject.convert("\e[31;44mHello\e[0m world")[:html]).to eq('Hello world') end it "performs color change from red/blue to yellow/blue" do - expect(subject.convert("\e[31;44mHello \e[33mworld")).to eq('Hello world') + expect(subject.convert("\e[31;44mHello \e[33mworld")[:html]).to eq('Hello world') end it "performs color change from red/blue to yellow/green" do - expect(subject.convert("\e[31;44mHello \e[33;42mworld")).to eq('Hello world') + expect(subject.convert("\e[31;44mHello \e[33;42mworld")[:html]).to eq('Hello world') end it "performs color change from red/blue to reset to yellow/green" do - expect(subject.convert("\e[31;44mHello\e[0m \e[33;42mworld")).to eq('Hello world') + expect(subject.convert("\e[31;44mHello\e[0m \e[33;42mworld")[:html]).to eq('Hello world') end it "ignores unsupported codes" do - expect(subject.convert("\e[51mHello\e[0m")).to eq('Hello') + expect(subject.convert("\e[51mHello\e[0m")[:html]).to eq('Hello') end it "prints light red" do - expect(subject.convert("\e[91mHello\e[0m")).to eq('Hello') + expect(subject.convert("\e[91mHello\e[0m")[:html]).to eq('Hello') end it "prints default on light red" do - expect(subject.convert("\e[101mHello\e[0m")).to eq('Hello') + expect(subject.convert("\e[101mHello\e[0m")[:html]).to eq('Hello') end it "performs color change from red/blue to default/blue" do - expect(subject.convert("\e[31;44mHello \e[39mworld")).to eq('Hello world') + expect(subject.convert("\e[31;44mHello \e[39mworld")[:html]).to eq('Hello world') end it "performs color change from light red/blue to default/blue" do - expect(subject.convert("\e[91;44mHello \e[39mworld")).to eq('Hello world') + expect(subject.convert("\e[91;44mHello \e[39mworld")[:html]).to eq('Hello world') end it "prints bold text" do - expect(subject.convert("\e[1mHello")).to eq('Hello') + expect(subject.convert("\e[1mHello")[:html]).to eq('Hello') end it "resets bold text" do - expect(subject.convert("\e[1mHello\e[21m world")).to eq('Hello world') - expect(subject.convert("\e[1mHello\e[22m world")).to eq('Hello world') + expect(subject.convert("\e[1mHello\e[21m world")[:html]).to eq('Hello world') + expect(subject.convert("\e[1mHello\e[22m world")[:html]).to eq('Hello world') end it "prints italic text" do - expect(subject.convert("\e[3mHello")).to eq('Hello') + expect(subject.convert("\e[3mHello")[:html]).to eq('Hello') end it "resets italic text" do - expect(subject.convert("\e[3mHello\e[23m world")).to eq('Hello world') + expect(subject.convert("\e[3mHello\e[23m world")[:html]).to eq('Hello world') end it "prints underlined text" do - expect(subject.convert("\e[4mHello")).to eq('Hello') + expect(subject.convert("\e[4mHello")[:html]).to eq('Hello') end it "resets underlined text" do - expect(subject.convert("\e[4mHello\e[24m world")).to eq('Hello world') + expect(subject.convert("\e[4mHello\e[24m world")[:html]).to eq('Hello world') end it "prints concealed text" do - expect(subject.convert("\e[8mHello")).to eq('Hello') + expect(subject.convert("\e[8mHello")[:html]).to eq('Hello') end it "resets concealed text" do - expect(subject.convert("\e[8mHello\e[28m world")).to eq('Hello world') + expect(subject.convert("\e[8mHello\e[28m world")[:html]).to eq('Hello world') end it "prints crossed-out text" do - expect(subject.convert("\e[9mHello")).to eq('Hello') + expect(subject.convert("\e[9mHello")[:html]).to eq('Hello') end it "resets crossed-out text" do - expect(subject.convert("\e[9mHello\e[29m world")).to eq('Hello world') + expect(subject.convert("\e[9mHello\e[29m world")[:html]).to eq('Hello world') end it "can print 256 xterm fg colors" do - expect(subject.convert("\e[38;5;16mHello")).to eq('Hello') + expect(subject.convert("\e[38;5;16mHello")[:html]).to eq('Hello') end it "can print 256 xterm fg colors on normal magenta background" do - expect(subject.convert("\e[38;5;16;45mHello")).to eq('Hello') + expect(subject.convert("\e[38;5;16;45mHello")[:html]).to eq('Hello') end it "can print 256 xterm bg colors" do - expect(subject.convert("\e[48;5;240mHello")).to eq('Hello') + expect(subject.convert("\e[48;5;240mHello")[:html]).to eq('Hello') end it "can print 256 xterm bg colors on normal magenta foreground" do - expect(subject.convert("\e[48;5;16;35mHello")).to eq('Hello') + expect(subject.convert("\e[48;5;16;35mHello")[:html]).to eq('Hello') end it "prints bold colored text vividly" do - expect(subject.convert("\e[1;31mHello\e[0m")).to eq('Hello') + expect(subject.convert("\e[1;31mHello\e[0m")[:html]).to eq('Hello') end it "prints bold light colored text correctly" do - expect(subject.convert("\e[1;91mHello\e[0m")).to eq('Hello') + expect(subject.convert("\e[1;91mHello\e[0m")[:html]).to eq('Hello') + end + + it "prints <" do + expect(subject.convert("<")[:html]).to eq('<') + end + + describe "incremental update" do + shared_examples 'stateable converter' do + let(:pass1) { subject.convert(pre_text) } + let(:pass2) { subject.convert(pre_text + text, pass1[:state]) } + + it "to returns html to append" do + expect(pass2[:append]).to be_truthy + expect(pass2[:html]).to eq(html) + expect(pass1[:text] + pass2[:text]).to eq(pre_text + text) + expect(pass1[:html] + pass2[:html]).to eq(pre_html + html) + end + end + + context "with split word" do + let(:pre_text) { "\e[1mHello" } + let(:pre_html) { "Hello" } + let(:text) { "\e[1mWorld" } + let(:html) { "World" } + + it_behaves_like 'stateable converter' + end + + context "with split sequence" do + let(:pre_text) { "\e[1m" } + let(:pre_html) { "" } + let(:text) { "Hello" } + let(:html) { "Hello" } + + it_behaves_like 'stateable converter' + end + + context "with partial sequence" do + let(:pre_text) { "Hello\e" } + let(:pre_html) { "Hello" } + let(:text) { "[1m World" } + let(:html) { " World" } + + it_behaves_like 'stateable converter' + end end end -- cgit v1.2.1 From c78b97df0eb275415d6ed5ef297841ee2f61b473 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Mon, 9 May 2016 22:06:32 +0300 Subject: Added rspec for testing container registry authentication service --- spec/lib/jwt/rsa_token_spec.rb | 31 ++++ spec/lib/jwt/token_spec.rb | 18 ++ ...ntainer_registry_authentication_service_spec.rb | 192 +++++++++++++++++++++ 3 files changed, 241 insertions(+) create mode 100644 spec/lib/jwt/rsa_token_spec.rb create mode 100644 spec/lib/jwt/token_spec.rb create mode 100644 spec/services/jwt/container_registry_authentication_service_spec.rb (limited to 'spec') diff --git a/spec/lib/jwt/rsa_token_spec.rb b/spec/lib/jwt/rsa_token_spec.rb new file mode 100644 index 00000000000..710801923e7 --- /dev/null +++ b/spec/lib/jwt/rsa_token_spec.rb @@ -0,0 +1,31 @@ +describe Jwt::RSAToken do + let(:rsa_key) { generate_key } + let(:rsa_token) { described_class.new(nil) } + let(:rsa_encoded) { rsa_token.encoded } + + before { allow_any_instance_of(described_class).to receive(:key).and_return(rsa_key) } + + context 'token' do + context 'for valid key to be validated' do + before { rsa_token['key'] = 'value' } + + subject { JWT.decode(rsa_encoded, rsa_key) } + + it { expect{subject}.to_not raise_error } + it { expect(subject.first).to include('key' => 'value') } + end + + context 'for invalid key to raise an exception' do + let(:new_key) { generate_key } + subject { JWT.decode(rsa_encoded, new_key) } + + it { expect{subject}.to raise_error(JWT::DecodeError) } + end + end + + private + + def generate_key + OpenSSL::PKey::RSA.generate(512) + end +end diff --git a/spec/lib/jwt/token_spec.rb b/spec/lib/jwt/token_spec.rb new file mode 100644 index 00000000000..a56b4cf39b5 --- /dev/null +++ b/spec/lib/jwt/token_spec.rb @@ -0,0 +1,18 @@ +describe Jwt::Token do + let(:token) { described_class.new } + + context 'custom parameters' do + let(:value) { 'value' } + before { token[:key] = value } + + it { expect(token[:key]).to eq(value) } + it { expect(token.payload).to include(key: value) } + end + + context 'embeds default payload' do + subject { token.payload } + let(:default) { token.send(:default_payload) } + + it { is_expected.to include(default) } + end +end diff --git a/spec/services/jwt/container_registry_authentication_service_spec.rb b/spec/services/jwt/container_registry_authentication_service_spec.rb new file mode 100644 index 00000000000..ea91f499d0a --- /dev/null +++ b/spec/services/jwt/container_registry_authentication_service_spec.rb @@ -0,0 +1,192 @@ +require 'spec_helper' + +describe Jwt::ContainerRegistryAuthenticationService, services: true do + let(:current_project) { nil } + let(:current_user) { nil } + let(:current_params) { {} } + let(:rsa_key) { OpenSSL::PKey::RSA.generate(512) } + let(:registry_settings) { + { + issuer: 'rspec', + key: nil + } + } + let(:payload) { JWT.decode(subject[:token], rsa_key).first } + + subject { described_class.new(current_project, current_user, current_params).execute } + + before do + allow(Gitlab.config.registry).to receive_messages(registry_settings) + allow_any_instance_of(Jwt::RSAToken).to receive(:key).and_return(rsa_key) + end + + shared_examples 'an authenticated' do + it { is_expected.to include(:token) } + it { expect(payload).to include('access') } + end + + shared_examples 'a accessible' do + let(:access) { + [{ + 'type' => 'repository', + 'name' => project.path_with_namespace, + 'actions' => actions, + }] + } + + it_behaves_like 'an authenticated' + it { expect(payload).to include('access' => access) } + end + + shared_examples 'a pullable' do + it_behaves_like 'a accessible' do + let(:actions) { ['pull'] } + end + end + + shared_examples 'a pushable' do + it_behaves_like 'a accessible' do + let(:actions) { ['push'] } + end + end + + shared_examples 'a pullable and pushable' do + it_behaves_like 'a accessible' do + let(:actions) { ['pull', 'push'] } + end + end + + shared_examples 'a forbidden' do + it { is_expected.to include(http_status: 401) } + it { is_expected.to_not include(:token) } + end + + context 'user authorization' do + let(:project) { create(:project) } + let(:current_user) { create(:user) } + + context 'allow developer to push images' do + before { project.team << [current_user, :developer] } + + let(:current_params) { + { scope: "repository:#{project.path_with_namespace}:push" } + } + + it_behaves_like 'a pushable' + end + + context 'allow reporter to pull images' do + before { project.team << [current_user, :reporter] } + + let(:current_params) { + { scope: "repository:#{project.path_with_namespace}:pull" } + } + + it_behaves_like 'a pullable' + end + + context 'return a least of privileges' do + before { project.team << [current_user, :reporter] } + + let(:current_params) { + { scope: "repository:#{project.path_with_namespace}:push,pull" } + } + + it_behaves_like 'a pullable' + end + + context 'disallow guest to pull or push images' do + before { project.team << [current_user, :guest] } + + let(:current_params) { + { scope: "repository:#{project.path_with_namespace}:pull,push" } + } + + it_behaves_like 'a forbidden' + end + end + + context 'project authorization' do + let(:current_project) { create(:empty_project) } + + context 'allow to pull and push images' do + let(:current_params) { + { scope: "repository:#{current_project.path_with_namespace}:pull,push" } + } + + it_behaves_like 'a pullable and pushable' do + let(:project) { current_project } + end + end + + context 'for other projects' do + context 'when pulling' do + let(:current_params) { + { scope: "repository:#{project.path_with_namespace}:pull" } + } + + context 'allow for public' do + let(:project) { create(:empty_project, :public) } + it_behaves_like 'a pullable' + end + + context 'disallow for private' do + let(:project) { create(:empty_project, :private) } + it_behaves_like 'a forbidden' + end + end + + context 'when pushing' do + let(:current_params) { + { scope: "repository:#{project.path_with_namespace}:push" } + } + + context 'disallow for all' do + let(:project) { create(:empty_project, :public) } + it_behaves_like 'a forbidden' + end + end + + end + end + + context 'unauthorized' do + context 'for invalid scope' do + let(:current_params) { + { scope: 'invalid:aa:bb' } + } + + it_behaves_like 'a forbidden' + end + + context 'for private project' do + let(:project) { create(:empty_project, :private) } + + let(:current_params) { + { scope: "repository:#{project.path_with_namespace}:pull" } + } + + it_behaves_like 'a forbidden' + end + + context 'for public project' do + let(:project) { create(:empty_project, :public) } + + context 'when pulling and pushing' do + let(:current_params) { + { scope: "repository:#{project.path_with_namespace}:pull,push" } + } + + it_behaves_like 'a pullable' + end + + context 'when pushing' do + let(:current_params) { + { scope: "repository:#{project.path_with_namespace}:push" } + } + + it_behaves_like 'a forbidden' + end + end + end +end -- cgit v1.2.1 From e56e3cdc62f96541b9bd8b7814204e92f1909253 Mon Sep 17 00:00:00 2001 From: Felipe Artur Date: Mon, 9 May 2016 19:35:37 -0300 Subject: Fix api leaking notes when user is not authorized to read noteable --- spec/requests/api/notes_spec.rb | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) (limited to 'spec') diff --git a/spec/requests/api/notes_spec.rb b/spec/requests/api/notes_spec.rb index 49091fc0f49..f5b31be1ba3 100644 --- a/spec/requests/api/notes_spec.rb +++ b/spec/requests/api/notes_spec.rb @@ -57,6 +57,15 @@ describe API::API, api: true do expect(json_response).to be_empty end + context "and issue is confidential" do + before { ext_issue.update_attributes(confidential: true) } + + it "returns 404" do + get api("/projects/#{ext_proj.id}/issues/#{ext_issue.id}/notes", user) + expect(response.status).to eq(404) + end + end + context "and current user can view the note" do it "should return an empty array" do get api("/projects/#{ext_proj.id}/issues/#{ext_issue.id}/notes", private_user) @@ -80,6 +89,11 @@ describe API::API, api: true do get api("/projects/#{project.id}/snippets/42/notes", user) expect(response.status).to eq(404) end + + it "returns 404 when not authorized" do + get api("/projects/#{project.id}/snippets/#{snippet.id}/notes", private_user) + expect(response.status).to eq(404) + end end context "when noteable is a Merge Request" do @@ -94,6 +108,11 @@ describe API::API, api: true do get api("/projects/#{project.id}/merge_requests/4444/notes", user) expect(response.status).to eq(404) end + + it "returns 404 when not authorized" do + get api("/projects/#{project.id}/merge_requests/4444/notes", private_user) + expect(response.status).to eq(404) + end end end -- cgit v1.2.1 From 93ca5c9964a26fbf31fcc794348b30193f4dff9f Mon Sep 17 00:00:00 2001 From: Felipe Artur Date: Tue, 10 May 2016 16:06:02 -0300 Subject: Fix notes API calls symbol convertions --- spec/requests/api/notes_spec.rb | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) (limited to 'spec') diff --git a/spec/requests/api/notes_spec.rb b/spec/requests/api/notes_spec.rb index f5b31be1ba3..f9bfee94424 100644 --- a/spec/requests/api/notes_spec.rb +++ b/spec/requests/api/notes_spec.rb @@ -39,6 +39,7 @@ describe API::API, api: true do context "when noteable is an Issue" do it "should return an array of issue notes" do get api("/projects/#{project.id}/issues/#{issue.id}/notes", user) + expect(response.status).to eq(200) expect(json_response).to be_an Array expect(json_response.first['body']).to eq(issue_note.note) @@ -46,12 +47,14 @@ describe API::API, api: true do it "should return a 404 error when issue id not found" do get api("/projects/#{project.id}/issues/12345/notes", user) + expect(response.status).to eq(404) end context "that references a private issue" do it "should return an empty array" do get api("/projects/#{ext_proj.id}/issues/#{ext_issue.id}/notes", user) + expect(response.status).to eq(200) expect(json_response).to be_an Array expect(json_response).to be_empty @@ -62,6 +65,7 @@ describe API::API, api: true do it "returns 404" do get api("/projects/#{ext_proj.id}/issues/#{ext_issue.id}/notes", user) + expect(response.status).to eq(404) end end @@ -69,6 +73,7 @@ describe API::API, api: true do context "and current user can view the note" do it "should return an empty array" do get api("/projects/#{ext_proj.id}/issues/#{ext_issue.id}/notes", private_user) + expect(response.status).to eq(200) expect(json_response).to be_an Array expect(json_response.first['body']).to eq(cross_reference_note.note) @@ -80,6 +85,7 @@ describe API::API, api: true do context "when noteable is a Snippet" do it "should return an array of snippet notes" do get api("/projects/#{project.id}/snippets/#{snippet.id}/notes", user) + expect(response.status).to eq(200) expect(json_response).to be_an Array expect(json_response.first['body']).to eq(snippet_note.note) @@ -87,11 +93,13 @@ describe API::API, api: true do it "should return a 404 error when snippet id not found" do get api("/projects/#{project.id}/snippets/42/notes", user) + expect(response.status).to eq(404) end it "returns 404 when not authorized" do get api("/projects/#{project.id}/snippets/#{snippet.id}/notes", private_user) + expect(response.status).to eq(404) end end @@ -99,6 +107,7 @@ describe API::API, api: true do context "when noteable is a Merge Request" do it "should return an array of merge_requests notes" do get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/notes", user) + expect(response.status).to eq(200) expect(json_response).to be_an Array expect(json_response.first['body']).to eq(merge_request_note.note) @@ -106,11 +115,13 @@ describe API::API, api: true do it "should return a 404 error if merge request id not found" do get api("/projects/#{project.id}/merge_requests/4444/notes", user) + expect(response.status).to eq(404) end it "returns 404 when not authorized" do get api("/projects/#{project.id}/merge_requests/4444/notes", private_user) + expect(response.status).to eq(404) end end @@ -120,24 +131,28 @@ describe API::API, api: true do context "when noteable is an Issue" do it "should return an issue note by id" do get api("/projects/#{project.id}/issues/#{issue.id}/notes/#{issue_note.id}", user) + expect(response.status).to eq(200) expect(json_response['body']).to eq(issue_note.note) end it "should return a 404 error if issue note not found" do get api("/projects/#{project.id}/issues/#{issue.id}/notes/12345", user) + expect(response.status).to eq(404) end context "that references a private issue" do it "should return a 404 error" do get api("/projects/#{ext_proj.id}/issues/#{ext_issue.id}/notes/#{cross_reference_note.id}", user) + expect(response.status).to eq(404) end context "and current user can view the note" do it "should return an issue note by id" do get api("/projects/#{ext_proj.id}/issues/#{ext_issue.id}/notes/#{cross_reference_note.id}", private_user) + expect(response.status).to eq(200) expect(json_response['body']).to eq(cross_reference_note.note) end @@ -148,12 +163,14 @@ describe API::API, api: true do context "when noteable is a Snippet" do it "should return a snippet note by id" do get api("/projects/#{project.id}/snippets/#{snippet.id}/notes/#{snippet_note.id}", user) + expect(response.status).to eq(200) expect(json_response['body']).to eq(snippet_note.note) end it "should return a 404 error if snippet note not found" do get api("/projects/#{project.id}/snippets/#{snippet.id}/notes/12345", user) + expect(response.status).to eq(404) end end @@ -163,6 +180,7 @@ describe API::API, api: true do context "when noteable is an Issue" do it "should create a new issue note" do post api("/projects/#{project.id}/issues/#{issue.id}/notes", user), body: 'hi!' + expect(response.status).to eq(201) expect(json_response['body']).to eq('hi!') expect(json_response['author']['username']).to eq(user.username) @@ -170,11 +188,13 @@ describe API::API, api: true do it "should return a 400 bad request error if body not given" do post api("/projects/#{project.id}/issues/#{issue.id}/notes", user) + expect(response.status).to eq(400) end it "should return a 401 unauthorized error if user not authenticated" do post api("/projects/#{project.id}/issues/#{issue.id}/notes"), body: 'hi!' + expect(response.status).to eq(401) end @@ -183,6 +203,7 @@ describe API::API, api: true do creation_time = 2.weeks.ago post api("/projects/#{project.id}/issues/#{issue.id}/notes", user), body: 'hi!', created_at: creation_time + expect(response.status).to eq(201) expect(json_response['body']).to eq('hi!') expect(json_response['author']['username']).to eq(user.username) @@ -195,6 +216,7 @@ describe API::API, api: true do context "when noteable is a Snippet" do it "should create a new snippet note" do post api("/projects/#{project.id}/snippets/#{snippet.id}/notes", user), body: 'hi!' + expect(response.status).to eq(201) expect(json_response['body']).to eq('hi!') expect(json_response['author']['username']).to eq(user.username) @@ -202,11 +224,13 @@ describe API::API, api: true do it "should return a 400 bad request error if body not given" do post api("/projects/#{project.id}/snippets/#{snippet.id}/notes", user) + expect(response.status).to eq(400) end it "should return a 401 unauthorized error if user not authenticated" do post api("/projects/#{project.id}/snippets/#{snippet.id}/notes"), body: 'hi!' + expect(response.status).to eq(401) end end @@ -246,6 +270,7 @@ describe API::API, api: true do it 'should return modified note' do put api("/projects/#{project.id}/issues/#{issue.id}/"\ "notes/#{issue_note.id}", user), body: 'Hello!' + expect(response.status).to eq(200) expect(json_response['body']).to eq('Hello!') end @@ -253,12 +278,14 @@ describe API::API, api: true do it 'should return a 404 error when note id not found' do put api("/projects/#{project.id}/issues/#{issue.id}/notes/12345", user), body: 'Hello!' + expect(response.status).to eq(404) end it 'should return a 400 bad request error if body not given' do put api("/projects/#{project.id}/issues/#{issue.id}/"\ "notes/#{issue_note.id}", user) + expect(response.status).to eq(400) end end @@ -267,6 +294,7 @@ describe API::API, api: true do it 'should return modified note' do put api("/projects/#{project.id}/snippets/#{snippet.id}/"\ "notes/#{snippet_note.id}", user), body: 'Hello!' + expect(response.status).to eq(200) expect(json_response['body']).to eq('Hello!') end @@ -274,6 +302,7 @@ describe API::API, api: true do it 'should return a 404 error when note id not found' do put api("/projects/#{project.id}/snippets/#{snippet.id}/"\ "notes/12345", user), body: "Hello!" + expect(response.status).to eq(404) end end @@ -282,6 +311,7 @@ describe API::API, api: true do it 'should return modified note' do put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/"\ "notes/#{merge_request_note.id}", user), body: 'Hello!' + expect(response.status).to eq(200) expect(json_response['body']).to eq('Hello!') end @@ -289,6 +319,7 @@ describe API::API, api: true do it 'should return a 404 error when note id not found' do put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/"\ "notes/12345", user), body: "Hello!" + expect(response.status).to eq(404) end end -- cgit v1.2.1 From 0e0caf4d17c28b6b0f3488b25efa265ce2804cc4 Mon Sep 17 00:00:00 2001 From: DJ Mountney Date: Tue, 10 May 2016 16:19:16 -0700 Subject: Add tests for the health check feature --- spec/controllers/health_check_controller_spec.rb | 90 ++++++++++++++++++++++++ spec/features/admin/admin_health_check_spec.rb | 55 +++++++++++++++ spec/routing/admin_routing_spec.rb | 7 ++ spec/routing/routing_spec.rb | 10 +++ 4 files changed, 162 insertions(+) create mode 100644 spec/controllers/health_check_controller_spec.rb create mode 100644 spec/features/admin/admin_health_check_spec.rb (limited to 'spec') diff --git a/spec/controllers/health_check_controller_spec.rb b/spec/controllers/health_check_controller_spec.rb new file mode 100644 index 00000000000..3b9cc5c98f5 --- /dev/null +++ b/spec/controllers/health_check_controller_spec.rb @@ -0,0 +1,90 @@ +require 'spec_helper' + +describe HealthCheckController do + let(:token) { current_application_settings.health_check_access_token } + let(:json_response) { JSON.parse(response.body) } + let(:xml_response) { Hash.from_xml(response.body)['hash'] } + + describe 'GET #index' do + context 'when services are up but NO access token' do + it 'returns a not found page' do + get :index + expect(response).to be_not_found + end + end + + context 'when services are up and an access token is provided' do + it 'supports successful plaintest response' do + get :index, token: token + expect(response).to be_success + expect(response.content_type).to eq 'text/plain' + end + + it 'supports successful json response' do + get :index, token: token, format: :json + expect(response).to be_success + expect(response.content_type).to eq 'application/json' + expect(json_response['healthy']).to be true + end + + it 'supports successful xml response' do + get :index, token: token, format: :xml + expect(response).to be_success + expect(response.content_type).to eq 'application/xml' + expect(xml_response['healthy']).to be true + end + + it 'supports successful responses for specific checks' do + get :index, token: token, checks: 'email', format: :json + expect(response).to be_success + expect(response.content_type).to eq 'application/json' + expect(json_response['healthy']).to be true + end + end + + context 'when a service is down but NO access token' do + it 'returns a not found page' do + get :index + expect(response).to be_not_found + end + end + + context 'when a service is down and an access token is provided' do + before do + allow(HealthCheck::Utils).to receive(:process_checks).with('standard').and_return('The server is on fire') + allow(HealthCheck::Utils).to receive(:process_checks).with('email').and_return('Email is on fire') + end + + it 'supports failure plaintest response' do + get :index, token: token + expect(response.status).to eq(500) + expect(response.content_type).to eq 'text/plain' + expect(response.body).to include('The server is on fire') + end + + it 'supports failure json response' do + get :index, token: token, format: :json + expect(response.status).to eq(500) + expect(response.content_type).to eq 'application/json' + expect(json_response['healthy']).to be false + expect(json_response['message']).to include('The server is on fire') + end + + it 'supports failure xml response' do + get :index, token: token, format: :xml + expect(response.status).to eq(500) + expect(response.content_type).to eq 'application/xml' + expect(xml_response['healthy']).to be false + expect(xml_response['message']).to include('The server is on fire') + end + + it 'supports failure responses for specific checks' do + get :index, token: token, checks: 'email', format: :json + expect(response.status).to eq(500) + expect(response.content_type).to eq 'application/json' + expect(json_response['healthy']).to be false + expect(json_response['message']).to include('Email is on fire') + end + end + end +end diff --git a/spec/features/admin/admin_health_check_spec.rb b/spec/features/admin/admin_health_check_spec.rb new file mode 100644 index 00000000000..4fde04b609b --- /dev/null +++ b/spec/features/admin/admin_health_check_spec.rb @@ -0,0 +1,55 @@ +require 'spec_helper' + +feature "Admin Health Check", feature: true do + include WaitForAjax + + before do + login_as :admin + end + + describe '#show' do + before do + visit admin_health_check_path + end + + it { page.has_text? 'Health Check' } + it { page.has_text? 'Health information can be reteived' } + + it 'has a health check access token' do + token = current_application_settings.health_check_access_token + expect(page).to have_content("Access token is #{token}") + expect(page).to have_selector('#health-check-token', text: token) + end + + describe 'reload access token', js: true do + it 'changes the access token' do + orig_token = current_application_settings.health_check_access_token + click_button 'Reset health check access token' + wait_for_ajax + expect(find('#health-check-token').text).not_to eq orig_token + end + end + end + + context 'when services are up' do + before do + visit admin_health_check_path + end + + it 'shows healthy status' do + expect(page).to have_content('Current Status: Healthy') + end + end + + context 'when a service is down' do + before do + allow(HealthCheck::Utils).to receive(:process_checks).and_return('The server is on fire') + visit admin_health_check_path + end + + it 'shows unhealthy status' do + expect(page).to have_content('Current Status: Unhealthy') + expect(page).to have_content('The server is on fire') + end + end +end diff --git a/spec/routing/admin_routing_spec.rb b/spec/routing/admin_routing_spec.rb index cd16a8e6322..b5ed8584c8a 100644 --- a/spec/routing/admin_routing_spec.rb +++ b/spec/routing/admin_routing_spec.rb @@ -118,3 +118,10 @@ describe Admin::DashboardController, "routing" do expect(get("/admin")).to route_to('admin/dashboard#index') end end + +# admin_health_check GET /admin/health_check(.:format) admin/health_check#show +describe Admin::HealthCheckController, "routing" do + it "to #show" do + expect(get("/admin/health_check")).to route_to('admin/health_check#show') + end +end diff --git a/spec/routing/routing_spec.rb b/spec/routing/routing_spec.rb index 1527eddfa48..e4dfd4bca35 100644 --- a/spec/routing/routing_spec.rb +++ b/spec/routing/routing_spec.rb @@ -243,3 +243,13 @@ describe "Groups", "routing" do expect(get('/1')).to route_to('namespaces#show', id: '1') end end + +describe HealthCheckController, 'routing' do + it 'to #index' do + expect(get('/health_check')).to route_to('health_check#index') + end + + it 'also supports passing checks in the url' do + expect(get('/health_check/email')).to route_to('health_check#index', checks: 'email') + end +end -- cgit v1.2.1 From c8f23bd2edc19f968446b149120df1f7798eb4b1 Mon Sep 17 00:00:00 2001 From: DJ Mountney Date: Wed, 11 May 2016 17:27:08 -0700 Subject: Support token header for health check token, and general cleanup of the health_check feature. --- spec/controllers/health_check_controller_spec.rb | 15 +++++++++++++++ 1 file changed, 15 insertions(+) (limited to 'spec') diff --git a/spec/controllers/health_check_controller_spec.rb b/spec/controllers/health_check_controller_spec.rb index 3b9cc5c98f5..0d8a68bb51a 100644 --- a/spec/controllers/health_check_controller_spec.rb +++ b/spec/controllers/health_check_controller_spec.rb @@ -14,6 +14,13 @@ describe HealthCheckController do end context 'when services are up and an access token is provided' do + it 'supports passing the token in the header' do + request.headers['TOKEN'] = token + get :index + expect(response).to be_success + expect(response.content_type).to eq 'text/plain' + end + it 'supports successful plaintest response' do get :index, token: token expect(response).to be_success @@ -55,6 +62,14 @@ describe HealthCheckController do allow(HealthCheck::Utils).to receive(:process_checks).with('email').and_return('Email is on fire') end + it 'supports passing the token in the header' do + request.headers['TOKEN'] = token + get :index + expect(response.status).to eq(500) + expect(response.content_type).to eq 'text/plain' + expect(response.body).to include('The server is on fire') + end + it 'supports failure plaintest response' do get :index, token: token expect(response.status).to eq(500) -- cgit v1.2.1 From a59ad3936a0bdbfd64d9c54af631a272317fe680 Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Thu, 5 May 2016 15:38:01 +0530 Subject: Add a spec for `WikiLinkFilter` - And fix behavior for non-file hierarchical links. --- spec/lib/banzai/filter/wiki_link_filter_spec.rb | 77 +++++++++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 spec/lib/banzai/filter/wiki_link_filter_spec.rb (limited to 'spec') diff --git a/spec/lib/banzai/filter/wiki_link_filter_spec.rb b/spec/lib/banzai/filter/wiki_link_filter_spec.rb new file mode 100644 index 00000000000..56b1a267390 --- /dev/null +++ b/spec/lib/banzai/filter/wiki_link_filter_spec.rb @@ -0,0 +1,77 @@ +require 'spec_helper' + +describe Banzai::Filter::WikiLinkFilter, lib: true do + include FilterSpecHelper + + let(:namespace) { build(:namespace, name: "wiki_link_ns") } + let(:project) { build(:empty_project, :public, name: "wiki_link_project", namespace: namespace) } + let(:user) { double } + let(:project_wiki) { ProjectWiki.new(project, user) } + + describe "links within the wiki (relative)" do + describe "hierarchical links to the current directory" do + it "doesn't rewrite non-file links" do + link = "Link to Page" + filtered_link = filter(link, project_wiki: project_wiki).children[0] + expect(filtered_link.attribute('href').value).to eq('./page') + end + + it "doesn't rewrite file links" do + link = "Link to Page" + filtered_link = filter(link, project_wiki: project_wiki).children[0] + expect(filtered_link.attribute('href').value).to eq('./page.md') + end + end + + describe "hierarchical links to the parent directory" do + it "doesn't rewrite non-file links" do + link = "Link to Page" + filtered_link = filter(link, project_wiki: project_wiki).children[0] + expect(filtered_link.attribute('href').value).to eq('../page') + end + + it "doesn't rewrite file links" do + link = "Link to Page" + filtered_link = filter(link, project_wiki: project_wiki).children[0] + expect(filtered_link.attribute('href').value).to eq('../page.md') + end + end + + describe "hierarchical links to a sub-directory" do + it "doesn't rewrite non-file links" do + link = "Link to Page" + filtered_link = filter(link, project_wiki: project_wiki).children[0] + expect(filtered_link.attribute('href').value).to eq('./subdirectory/page') + end + + it "doesn't rewrite file links" do + link = "Link to Page" + filtered_link = filter(link, project_wiki: project_wiki).children[0] + expect(filtered_link.attribute('href').value).to eq('./subdirectory/page.md') + end + end + + describe "non-hierarchical links" do + it 'rewrites non-file links to be at the scope of the wiki root' do + link = "Link to Page" + filtered_link = filter(link, project_wiki: project_wiki).children[0] + + expect(filtered_link.attribute('href').value).to match('/wiki_link_ns/wiki_link_project/wikis/page') + end + + it "doesn't rewrite file links" do + link = "Link to Page" + filtered_link = filter(link, project_wiki: project_wiki).children[0] + expect(filtered_link.attribute('href').value).to eq('page.md') + end + end + end + + describe "links outside the wiki (absolute)" do + it "doesn't rewrite links" do + link = "Link to Page" + filtered_link = filter(link, project_wiki: project_wiki).children[0] + expect(filtered_link.attribute('href').value).to eq('http://example.com/page') + end + end +end -- cgit v1.2.1 From a4ee7d25e398f9d2e2311703fbbcf5f6b9bdf728 Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Mon, 9 May 2016 16:04:05 +0530 Subject: Implement @rymai's feedback after review. - Separate 'exercise' and 'verify' steps of tests. - Use `build_stubbed` instead of `build` --- spec/lib/banzai/filter/wiki_link_filter_spec.rb | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) (limited to 'spec') diff --git a/spec/lib/banzai/filter/wiki_link_filter_spec.rb b/spec/lib/banzai/filter/wiki_link_filter_spec.rb index 56b1a267390..185abbb2108 100644 --- a/spec/lib/banzai/filter/wiki_link_filter_spec.rb +++ b/spec/lib/banzai/filter/wiki_link_filter_spec.rb @@ -3,8 +3,8 @@ require 'spec_helper' describe Banzai::Filter::WikiLinkFilter, lib: true do include FilterSpecHelper - let(:namespace) { build(:namespace, name: "wiki_link_ns") } - let(:project) { build(:empty_project, :public, name: "wiki_link_project", namespace: namespace) } + let(:namespace) { build_stubbed(:namespace, name: "wiki_link_ns") } + let(:project) { build_stubbed(:empty_project, :public, name: "wiki_link_project", namespace: namespace) } let(:user) { double } let(:project_wiki) { ProjectWiki.new(project, user) } @@ -13,12 +13,14 @@ describe Banzai::Filter::WikiLinkFilter, lib: true do it "doesn't rewrite non-file links" do link = "Link to Page" filtered_link = filter(link, project_wiki: project_wiki).children[0] + expect(filtered_link.attribute('href').value).to eq('./page') end it "doesn't rewrite file links" do link = "Link to Page" filtered_link = filter(link, project_wiki: project_wiki).children[0] + expect(filtered_link.attribute('href').value).to eq('./page.md') end end @@ -27,12 +29,14 @@ describe Banzai::Filter::WikiLinkFilter, lib: true do it "doesn't rewrite non-file links" do link = "Link to Page" filtered_link = filter(link, project_wiki: project_wiki).children[0] + expect(filtered_link.attribute('href').value).to eq('../page') end it "doesn't rewrite file links" do link = "Link to Page" filtered_link = filter(link, project_wiki: project_wiki).children[0] + expect(filtered_link.attribute('href').value).to eq('../page.md') end end @@ -41,12 +45,14 @@ describe Banzai::Filter::WikiLinkFilter, lib: true do it "doesn't rewrite non-file links" do link = "Link to Page" filtered_link = filter(link, project_wiki: project_wiki).children[0] + expect(filtered_link.attribute('href').value).to eq('./subdirectory/page') end it "doesn't rewrite file links" do link = "Link to Page" filtered_link = filter(link, project_wiki: project_wiki).children[0] + expect(filtered_link.attribute('href').value).to eq('./subdirectory/page.md') end end @@ -62,6 +68,7 @@ describe Banzai::Filter::WikiLinkFilter, lib: true do it "doesn't rewrite file links" do link = "Link to Page" filtered_link = filter(link, project_wiki: project_wiki).children[0] + expect(filtered_link.attribute('href').value).to eq('page.md') end end @@ -71,6 +78,7 @@ describe Banzai::Filter::WikiLinkFilter, lib: true do it "doesn't rewrite links" do link = "Link to Page" filtered_link = filter(link, project_wiki: project_wiki).children[0] + expect(filtered_link.attribute('href').value).to eq('http://example.com/page') end end -- cgit v1.2.1 From 47ee5125e881694b7713f187b48589a2f4bbd747 Mon Sep 17 00:00:00 2001 From: Andrei Gliga Date: Wed, 4 May 2016 15:57:00 +0300 Subject: validate disabled_oauth_sign_in_sources in ApplicationSe --- spec/models/application_setting_spec.rb | 3 +++ 1 file changed, 3 insertions(+) (limited to 'spec') diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index 1ce22feed5c..fb3ea491df7 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -20,6 +20,9 @@ describe ApplicationSetting, models: true do it { is_expected.to allow_value(https).for(:after_sign_out_path) } it { is_expected.not_to allow_value(ftp).for(:after_sign_out_path) } + it { is_expected.to allow_value([:github]).for(:disabled_oauth_sign_in_sources) } + it { is_expected.not_to allow_value([:test]).for(:disabled_oauth_sign_in_sources) } + it { is_expected.to validate_presence_of(:max_attachment_size) } it do -- cgit v1.2.1 From 8c2b72b1c80e8bbd082ac8e1aedb21a1aad07235 Mon Sep 17 00:00:00 2001 From: Andrei Gliga Date: Wed, 4 May 2016 17:04:54 +0300 Subject: tests for enabled_button_based_providers helper method of AuthHelper --- spec/helpers/auth_helper_spec.rb | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) (limited to 'spec') diff --git a/spec/helpers/auth_helper_spec.rb b/spec/helpers/auth_helper_spec.rb index e47a54fdac5..5d66c92d26e 100644 --- a/spec/helpers/auth_helper_spec.rb +++ b/spec/helpers/auth_helper_spec.rb @@ -2,7 +2,9 @@ require "spec_helper" describe AuthHelper do describe "button_based_providers" do - it 'returns all enabled providers' do + let(:settings) { ApplicationSetting.create_from_defaults } + + it 'returns all enabled providers from devise' do allow(helper).to receive(:auth_providers) { [:twitter, :github] } expect(helper.button_based_providers).to include(*[:twitter, :github]) end @@ -16,5 +18,23 @@ describe AuthHelper do allow(helper).to receive(:auth_providers) { [] } expect(helper.button_based_providers).to eq([]) end + + it 'returns all the enabled providers from settings' do + allow(helper).to receive(:auth_providers) { [:twitter, :github] } + expect(helper.enabled_button_based_providers).to include(*['twitter', 'github']) + end + + it 'should not return github as provider because it\'s disabled from settings' do + settings.update_attribute( + :disabled_oauth_sign_in_sources, + ['github'] + ) + + allow(helper).to receive(:auth_providers) { [:twitter, :github] } + allow(helper).to receive(:current_application_settings) { settings } + + expect(helper.enabled_button_based_providers).to include('twitter') + expect(helper.enabled_button_based_providers).to_not include('github') + end end end -- cgit v1.2.1 From 6f69f6a1ab70bd2252803a64927d0c8df1d03612 Mon Sep 17 00:00:00 2001 From: Andrei Gliga Date: Wed, 4 May 2016 17:06:07 +0300 Subject: tests for button_based_providers_enabled? helper method of AuthHelper --- spec/helpers/auth_helper_spec.rb | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) (limited to 'spec') diff --git a/spec/helpers/auth_helper_spec.rb b/spec/helpers/auth_helper_spec.rb index 5d66c92d26e..a6a366bc19a 100644 --- a/spec/helpers/auth_helper_spec.rb +++ b/spec/helpers/auth_helper_spec.rb @@ -36,5 +36,23 @@ describe AuthHelper do expect(helper.enabled_button_based_providers).to include('twitter') expect(helper.enabled_button_based_providers).to_not include('github') end + + it 'returns true for button_based_providers_enabled? because there providers' do + allow(helper).to receive(:auth_providers) { [:twitter, :github] } + + expect(helper.button_based_providers_enabled?).to be true + end + + it 'returns false for button_based_providers_enabled? because there providers' do + settings.update_attribute( + :disabled_oauth_sign_in_sources, + ['github', 'twitter'] + ) + + allow(helper).to receive(:auth_providers) { [:twitter, :github] } + allow(helper).to receive(:current_application_settings) { settings } + + expect(helper.button_based_providers_enabled?).to be false + end end end -- cgit v1.2.1 From deca3da1a7f4e59c97c27bedf7aa26185a4d883b Mon Sep 17 00:00:00 2001 From: Andrei Gliga Date: Thu, 5 May 2016 10:34:51 +0300 Subject: stub Devise.omniauth_providers to return GitHub even if the gitlab.yml has no omniauth provider enabled This will fix failing tests in case gitlab.yml file has no omniauth providers enabled --- spec/models/application_setting_spec.rb | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) (limited to 'spec') diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index fb3ea491df7..d84f3e998f5 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -20,8 +20,14 @@ describe ApplicationSetting, models: true do it { is_expected.to allow_value(https).for(:after_sign_out_path) } it { is_expected.not_to allow_value(ftp).for(:after_sign_out_path) } - it { is_expected.to allow_value([:github]).for(:disabled_oauth_sign_in_sources) } - it { is_expected.not_to allow_value([:test]).for(:disabled_oauth_sign_in_sources) } + describe 'disabled_oauth_sign_in_sources validations' do + before do + allow(Devise).to receive(:omniauth_providers).and_return([:github]) + end + + it { is_expected.to allow_value(['github']).for(:disabled_oauth_sign_in_sources) } + it { is_expected.not_to allow_value(['test']).for(:disabled_oauth_sign_in_sources) } + end it { is_expected.to validate_presence_of(:max_attachment_size) } -- cgit v1.2.1 From 2e4c914ae88b77c8a3871f8415163a51e23254e5 Mon Sep 17 00:00:00 2001 From: Andrei Gliga Date: Mon, 9 May 2016 10:31:25 +0300 Subject: between "" --- spec/helpers/auth_helper_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec') diff --git a/spec/helpers/auth_helper_spec.rb b/spec/helpers/auth_helper_spec.rb index a6a366bc19a..6dc1135927d 100644 --- a/spec/helpers/auth_helper_spec.rb +++ b/spec/helpers/auth_helper_spec.rb @@ -24,7 +24,7 @@ describe AuthHelper do expect(helper.enabled_button_based_providers).to include(*['twitter', 'github']) end - it 'should not return github as provider because it\'s disabled from settings' do + it "should not return github as provider because it's disabled from settings" do settings.update_attribute( :disabled_oauth_sign_in_sources, ['github'] -- cgit v1.2.1 From fc88527c9e21f99a9411423f1c8a6475957c02f3 Mon Sep 17 00:00:00 2001 From: Andrei Gliga Date: Mon, 9 May 2016 10:40:27 +0300 Subject: use stub_application_setting instead --- spec/helpers/auth_helper_spec.rb | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) (limited to 'spec') diff --git a/spec/helpers/auth_helper_spec.rb b/spec/helpers/auth_helper_spec.rb index 6dc1135927d..e6af074a780 100644 --- a/spec/helpers/auth_helper_spec.rb +++ b/spec/helpers/auth_helper_spec.rb @@ -2,8 +2,6 @@ require "spec_helper" describe AuthHelper do describe "button_based_providers" do - let(:settings) { ApplicationSetting.create_from_defaults } - it 'returns all enabled providers from devise' do allow(helper).to receive(:auth_providers) { [:twitter, :github] } expect(helper.button_based_providers).to include(*[:twitter, :github]) @@ -25,13 +23,11 @@ describe AuthHelper do end it "should not return github as provider because it's disabled from settings" do - settings.update_attribute( - :disabled_oauth_sign_in_sources, - ['github'] + stub_application_setting( + disabled_oauth_sign_in_sources: ['github'] ) allow(helper).to receive(:auth_providers) { [:twitter, :github] } - allow(helper).to receive(:current_application_settings) { settings } expect(helper.enabled_button_based_providers).to include('twitter') expect(helper.enabled_button_based_providers).to_not include('github') @@ -44,13 +40,11 @@ describe AuthHelper do end it 'returns false for button_based_providers_enabled? because there providers' do - settings.update_attribute( - :disabled_oauth_sign_in_sources, - ['github', 'twitter'] + stub_application_setting( + disabled_oauth_sign_in_sources: ['github', 'twitter'] ) allow(helper).to receive(:auth_providers) { [:twitter, :github] } - allow(helper).to receive(:current_application_settings) { settings } expect(helper.button_based_providers_enabled?).to be false end -- cgit v1.2.1 From 3922e7ee2249bb5a683b7474b281d8e83d465740 Mon Sep 17 00:00:00 2001 From: Andrei Gliga Date: Mon, 9 May 2016 10:42:57 +0300 Subject: enabled_button_based_providers into their own describe section --- spec/helpers/auth_helper_spec.rb | 2 ++ 1 file changed, 2 insertions(+) (limited to 'spec') diff --git a/spec/helpers/auth_helper_spec.rb b/spec/helpers/auth_helper_spec.rb index e6af074a780..04b51282795 100644 --- a/spec/helpers/auth_helper_spec.rb +++ b/spec/helpers/auth_helper_spec.rb @@ -16,7 +16,9 @@ describe AuthHelper do allow(helper).to receive(:auth_providers) { [] } expect(helper.button_based_providers).to eq([]) end + end + describe 'enabled_button_based_providers' do it 'returns all the enabled providers from settings' do allow(helper).to receive(:auth_providers) { [:twitter, :github] } expect(helper.enabled_button_based_providers).to include(*['twitter', 'github']) -- cgit v1.2.1 From 038dbb6803948e050484ed3f34c99b2f3bf7a6c5 Mon Sep 17 00:00:00 2001 From: Andrei Gliga Date: Mon, 9 May 2016 10:51:24 +0300 Subject: DRYing enabled_button_based_providers tests --- spec/helpers/auth_helper_spec.rb | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) (limited to 'spec') diff --git a/spec/helpers/auth_helper_spec.rb b/spec/helpers/auth_helper_spec.rb index 04b51282795..a6df8204213 100644 --- a/spec/helpers/auth_helper_spec.rb +++ b/spec/helpers/auth_helper_spec.rb @@ -19,8 +19,11 @@ describe AuthHelper do end describe 'enabled_button_based_providers' do - it 'returns all the enabled providers from settings' do + before do allow(helper).to receive(:auth_providers) { [:twitter, :github] } + end + + it 'returns all the enabled providers from settings' do expect(helper.enabled_button_based_providers).to include(*['twitter', 'github']) end @@ -29,15 +32,11 @@ describe AuthHelper do disabled_oauth_sign_in_sources: ['github'] ) - allow(helper).to receive(:auth_providers) { [:twitter, :github] } - expect(helper.enabled_button_based_providers).to include('twitter') expect(helper.enabled_button_based_providers).to_not include('github') end it 'returns true for button_based_providers_enabled? because there providers' do - allow(helper).to receive(:auth_providers) { [:twitter, :github] } - expect(helper.button_based_providers_enabled?).to be true end @@ -46,8 +45,6 @@ describe AuthHelper do disabled_oauth_sign_in_sources: ['github', 'twitter'] ) - allow(helper).to receive(:auth_providers) { [:twitter, :github] } - expect(helper.button_based_providers_enabled?).to be false end end -- cgit v1.2.1 From 96122034cfc2eb7039ae75b20b729f35e9aa832e Mon Sep 17 00:00:00 2001 From: Andrei Gliga Date: Tue, 10 May 2016 11:17:37 +0300 Subject: more readable specs for enabled_button_based_providers and button_based_providers_enabled? --- spec/helpers/auth_helper_spec.rb | 44 ++++++++++++++++++++++++++-------------- 1 file changed, 29 insertions(+), 15 deletions(-) (limited to 'spec') diff --git a/spec/helpers/auth_helper_spec.rb b/spec/helpers/auth_helper_spec.rb index a6df8204213..16fbb5dcecb 100644 --- a/spec/helpers/auth_helper_spec.rb +++ b/spec/helpers/auth_helper_spec.rb @@ -23,29 +23,43 @@ describe AuthHelper do allow(helper).to receive(:auth_providers) { [:twitter, :github] } end - it 'returns all the enabled providers from settings' do - expect(helper.enabled_button_based_providers).to include(*['twitter', 'github']) + context 'all providers are enabled to sign in' do + it 'returns all the enabled providers from settings' do + expect(helper.enabled_button_based_providers).to include('twitter', 'github') + end end - it "should not return github as provider because it's disabled from settings" do - stub_application_setting( - disabled_oauth_sign_in_sources: ['github'] - ) + context 'GitHub OAuth sign in is disabled from application setting' do + it "doesn't return github as provider" do + stub_application_setting( + disabled_oauth_sign_in_sources: ['github'] + ) - expect(helper.enabled_button_based_providers).to include('twitter') - expect(helper.enabled_button_based_providers).to_not include('github') + expect(helper.enabled_button_based_providers).to include('twitter') + expect(helper.enabled_button_based_providers).to_not include('github') + end + end + end + + describe 'button_based_providers_enabled?' do + before do + allow(helper).to receive(:auth_providers) { [:twitter, :github] } end - it 'returns true for button_based_providers_enabled? because there providers' do - expect(helper.button_based_providers_enabled?).to be true + context 'button based providers enabled' do + it 'returns true' do + expect(helper.button_based_providers_enabled?).to be true + end end - it 'returns false for button_based_providers_enabled? because there providers' do - stub_application_setting( - disabled_oauth_sign_in_sources: ['github', 'twitter'] - ) + context 'all the button based providers are disabled via application_setting' do + it 'returns false' do + stub_application_setting( + disabled_oauth_sign_in_sources: ['github', 'twitter'] + ) - expect(helper.button_based_providers_enabled?).to be false + expect(helper.button_based_providers_enabled?).to be false + end end end end -- cgit v1.2.1 From 945c5b3fe6e0552f77da8b1a1efe75cd04434f53 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Thu, 12 May 2016 15:14:14 +0200 Subject: Removed tracking of total method execution times Because method call timings are inclusive (that is, they include the time of any sub method calls) this would lead to the total method execution time often being far greater than the total transaction time. Because this is incredibly confusing it's best to simply _not_ track the total method execution time, after all it's not that useful to begin with. Fixes gitlab-org/gitlab-ce#17239 --- spec/lib/gitlab/metrics/instrumentation_spec.rb | 6 ------ 1 file changed, 6 deletions(-) (limited to 'spec') diff --git a/spec/lib/gitlab/metrics/instrumentation_spec.rb b/spec/lib/gitlab/metrics/instrumentation_spec.rb index 5c885a7a982..7b86450a223 100644 --- a/spec/lib/gitlab/metrics/instrumentation_spec.rb +++ b/spec/lib/gitlab/metrics/instrumentation_spec.rb @@ -56,9 +56,6 @@ describe Gitlab::Metrics::Instrumentation do allow(described_class).to receive(:transaction). and_return(transaction) - expect(transaction).to receive(:increment). - with(:method_duration, a_kind_of(Numeric)) - expect(transaction).to receive(:add_metric). with(described_class::SERIES, an_instance_of(Hash), method: 'Dummy.foo') @@ -139,9 +136,6 @@ describe Gitlab::Metrics::Instrumentation do allow(described_class).to receive(:transaction). and_return(transaction) - expect(transaction).to receive(:increment). - with(:method_duration, a_kind_of(Numeric)) - expect(transaction).to receive(:add_metric). with(described_class::SERIES, an_instance_of(Hash), method: 'Dummy#bar') -- cgit v1.2.1 From 18fdbf0a035b6feec3b576c01ee1a2f3a95e4305 Mon Sep 17 00:00:00 2001 From: Takuya Noguchi Date: Wed, 11 May 2016 23:46:19 +0900 Subject: Fix a description for default scope on builds --- spec/features/admin/admin_builds_spec.rb | 1 + spec/features/builds_spec.rb | 1 + 2 files changed, 2 insertions(+) (limited to 'spec') diff --git a/spec/features/admin/admin_builds_spec.rb b/spec/features/admin/admin_builds_spec.rb index 2e9851fb442..7bbe20fec43 100644 --- a/spec/features/admin/admin_builds_spec.rb +++ b/spec/features/admin/admin_builds_spec.rb @@ -19,6 +19,7 @@ describe 'Admin Builds' do visit admin_builds_path expect(page).to have_selector('.nav-links li.active', text: 'All') + expect(page).to have_selector('.row-content-block', text: 'All builds') expect(page.all('.build-link').size).to eq(4) expect(page).to have_link 'Cancel all' end diff --git a/spec/features/builds_spec.rb b/spec/features/builds_spec.rb index 090a941958f..f83a78308e3 100644 --- a/spec/features/builds_spec.rb +++ b/spec/features/builds_spec.rb @@ -43,6 +43,7 @@ describe "Builds" do end it { expect(page).to have_selector('.nav-links li.active', text: 'All') } + it { expect(page).to have_selector('.row-content-block', text: 'All builds from this project') } it { expect(page).to have_content @build.short_sha } it { expect(page).to have_content @build.ref } it { expect(page).to have_content @build.name } -- cgit v1.2.1 From fc2d985bfaa156ad052858cd2025b0300327ff95 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 12 May 2016 12:47:55 -0500 Subject: Fix CI tests --- spec/lib/jwt/rsa_token_spec.rb | 2 +- spec/lib/jwt/token_spec.rb | 2 +- ...ntainer_registry_authentication_service_spec.rb | 56 +++++++++++----------- 3 files changed, 30 insertions(+), 30 deletions(-) (limited to 'spec') diff --git a/spec/lib/jwt/rsa_token_spec.rb b/spec/lib/jwt/rsa_token_spec.rb index 710801923e7..a5b1d3a67dc 100644 --- a/spec/lib/jwt/rsa_token_spec.rb +++ b/spec/lib/jwt/rsa_token_spec.rb @@ -1,4 +1,4 @@ -describe Jwt::RSAToken do +describe JWT::RSAToken do let(:rsa_key) { generate_key } let(:rsa_token) { described_class.new(nil) } let(:rsa_encoded) { rsa_token.encoded } diff --git a/spec/lib/jwt/token_spec.rb b/spec/lib/jwt/token_spec.rb index a56b4cf39b5..92fdc3f1b7c 100644 --- a/spec/lib/jwt/token_spec.rb +++ b/spec/lib/jwt/token_spec.rb @@ -1,4 +1,4 @@ -describe Jwt::Token do +describe JWT::Token do let(:token) { described_class.new } context 'custom parameters' do diff --git a/spec/services/jwt/container_registry_authentication_service_spec.rb b/spec/services/jwt/container_registry_authentication_service_spec.rb index ea91f499d0a..1873ea2639b 100644 --- a/spec/services/jwt/container_registry_authentication_service_spec.rb +++ b/spec/services/jwt/container_registry_authentication_service_spec.rb @@ -1,23 +1,23 @@ require 'spec_helper' -describe Jwt::ContainerRegistryAuthenticationService, services: true do +describe JWT::ContainerRegistryAuthenticationService, services: true do let(:current_project) { nil } let(:current_user) { nil } let(:current_params) { {} } let(:rsa_key) { OpenSSL::PKey::RSA.generate(512) } - let(:registry_settings) { + let(:registry_settings) do { issuer: 'rspec', key: nil } - } + end let(:payload) { JWT.decode(subject[:token], rsa_key).first } subject { described_class.new(current_project, current_user, current_params).execute } before do allow(Gitlab.config.registry).to receive_messages(registry_settings) - allow_any_instance_of(Jwt::RSAToken).to receive(:key).and_return(rsa_key) + allow_any_instance_of(JWT::RSAToken).to receive(:key).and_return(rsa_key) end shared_examples 'an authenticated' do @@ -26,13 +26,13 @@ describe Jwt::ContainerRegistryAuthenticationService, services: true do end shared_examples 'a accessible' do - let(:access) { + let(:access) do [{ 'type' => 'repository', 'name' => project.path_with_namespace, 'actions' => actions, }] - } + end it_behaves_like 'an authenticated' it { expect(payload).to include('access' => access) } @@ -68,9 +68,9 @@ describe Jwt::ContainerRegistryAuthenticationService, services: true do context 'allow developer to push images' do before { project.team << [current_user, :developer] } - let(:current_params) { + let(:current_params) do { scope: "repository:#{project.path_with_namespace}:push" } - } + end it_behaves_like 'a pushable' end @@ -78,9 +78,9 @@ describe Jwt::ContainerRegistryAuthenticationService, services: true do context 'allow reporter to pull images' do before { project.team << [current_user, :reporter] } - let(:current_params) { + let(:current_params) do { scope: "repository:#{project.path_with_namespace}:pull" } - } + end it_behaves_like 'a pullable' end @@ -88,9 +88,9 @@ describe Jwt::ContainerRegistryAuthenticationService, services: true do context 'return a least of privileges' do before { project.team << [current_user, :reporter] } - let(:current_params) { + let(:current_params) do { scope: "repository:#{project.path_with_namespace}:push,pull" } - } + end it_behaves_like 'a pullable' end @@ -98,9 +98,9 @@ describe Jwt::ContainerRegistryAuthenticationService, services: true do context 'disallow guest to pull or push images' do before { project.team << [current_user, :guest] } - let(:current_params) { + let(:current_params) do { scope: "repository:#{project.path_with_namespace}:pull,push" } - } + end it_behaves_like 'a forbidden' end @@ -110,9 +110,9 @@ describe Jwt::ContainerRegistryAuthenticationService, services: true do let(:current_project) { create(:empty_project) } context 'allow to pull and push images' do - let(:current_params) { + let(:current_params) do { scope: "repository:#{current_project.path_with_namespace}:pull,push" } - } + end it_behaves_like 'a pullable and pushable' do let(:project) { current_project } @@ -121,9 +121,9 @@ describe Jwt::ContainerRegistryAuthenticationService, services: true do context 'for other projects' do context 'when pulling' do - let(:current_params) { + let(:current_params) do { scope: "repository:#{project.path_with_namespace}:pull" } - } + end context 'allow for public' do let(:project) { create(:empty_project, :public) } @@ -137,9 +137,9 @@ describe Jwt::ContainerRegistryAuthenticationService, services: true do end context 'when pushing' do - let(:current_params) { + let(:current_params) do { scope: "repository:#{project.path_with_namespace}:push" } - } + end context 'disallow for all' do let(:project) { create(:empty_project, :public) } @@ -152,9 +152,9 @@ describe Jwt::ContainerRegistryAuthenticationService, services: true do context 'unauthorized' do context 'for invalid scope' do - let(:current_params) { + let(:current_params) do { scope: 'invalid:aa:bb' } - } + end it_behaves_like 'a forbidden' end @@ -162,9 +162,9 @@ describe Jwt::ContainerRegistryAuthenticationService, services: true do context 'for private project' do let(:project) { create(:empty_project, :private) } - let(:current_params) { + let(:current_params) do { scope: "repository:#{project.path_with_namespace}:pull" } - } + end it_behaves_like 'a forbidden' end @@ -173,17 +173,17 @@ describe Jwt::ContainerRegistryAuthenticationService, services: true do let(:project) { create(:empty_project, :public) } context 'when pulling and pushing' do - let(:current_params) { + let(:current_params) do { scope: "repository:#{project.path_with_namespace}:pull,push" } - } + end it_behaves_like 'a pullable' end context 'when pushing' do - let(:current_params) { + let(:current_params) do { scope: "repository:#{project.path_with_namespace}:push" } - } + end it_behaves_like 'a forbidden' end -- cgit v1.2.1 From 0c22698bd4dbe7d0d3e4a6c8bc946ac6f5de1c12 Mon Sep 17 00:00:00 2001 From: Ahmad Sherif Date: Thu, 12 May 2016 22:48:09 +0200 Subject: Add API endpoints for un/subscribing from/to a label Closes #15638 --- spec/models/concerns/subscribable_spec.rb | 10 ++++ spec/requests/api/issues_spec.rb | 12 +++++ spec/requests/api/labels_spec.rb | 82 +++++++++++++++++++++++++++++++ 3 files changed, 104 insertions(+) (limited to 'spec') diff --git a/spec/models/concerns/subscribable_spec.rb b/spec/models/concerns/subscribable_spec.rb index e31fdb0bffb..b7fc5a92497 100644 --- a/spec/models/concerns/subscribable_spec.rb +++ b/spec/models/concerns/subscribable_spec.rb @@ -44,6 +44,16 @@ describe Subscribable, 'Subscribable' do end end + describe '#subscribe' do + it 'subscribes the given user' do + expect(resource.subscribed?(user)).to be_falsey + + resource.subscribe(user) + + expect(resource.subscribed?(user)).to be_truthy + end + end + describe '#unsubscribe' do it 'unsubscribes the given current user' do resource.subscriptions.create(user: user, subscribed: true) diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb index 9dd43f4fab3..37ab9cc8cfe 100644 --- a/spec/requests/api/issues_spec.rb +++ b/spec/requests/api/issues_spec.rb @@ -623,6 +623,12 @@ describe API::API, api: true do expect(response.status).to eq(404) end + + it 'returns 404 if the issue is confidential' do + post api("/projects/#{project.id}/issues/#{confidential_issue.id}/subscription", non_member) + + expect(response.status).to eq(404) + end end describe 'DELETE :id/issues/:issue_id/subscription' do @@ -644,5 +650,11 @@ describe API::API, api: true do expect(response.status).to eq(404) end + + it 'returns 404 if the issue is confidential' do + delete api("/projects/#{project.id}/issues/#{confidential_issue.id}/subscription", non_member) + + expect(response.status).to eq(404) + end end end diff --git a/spec/requests/api/labels_spec.rb b/spec/requests/api/labels_spec.rb index 6943ff9d26c..b2c7f8d9acb 100644 --- a/spec/requests/api/labels_spec.rb +++ b/spec/requests/api/labels_spec.rb @@ -190,4 +190,86 @@ describe API::API, api: true do expect(json_response['message']['color']).to eq(['must be a valid color code']) end end + + describe "POST /projects/:id/labels/:label_id/subscription" do + context "when label_id is a label title" do + it "should subscribe to the label" do + post api("/projects/#{project.id}/labels/#{label1.title}/subscription", user) + + expect(response.status).to eq(201) + expect(json_response["name"]).to eq(label1.title) + expect(json_response["subscribed"]).to be_truthy + end + end + + context "when label_id is a label ID" do + it "should subscribe to the label" do + post api("/projects/#{project.id}/labels/#{label1.id}/subscription", user) + + expect(response.status).to eq(201) + expect(json_response["name"]).to eq(label1.title) + expect(json_response["subscribed"]).to be_truthy + end + end + + context "when user is already subscribed to label" do + before { label1.subscribe(user) } + + it "should return 304" do + post api("/projects/#{project.id}/labels/#{label1.id}/subscription", user) + + expect(response.status).to eq(304) + end + end + + context "when label ID is not found" do + it "should a return 404 error" do + post api("/projects/#{project.id}/labels/1234/subscription", user) + + expect(response.status).to eq(404) + end + end + end + + describe "DELETE /projects/:id/labels/:label_id/subscription" do + before { label1.subscribe(user) } + + context "when label_id is a label title" do + it "should unsubscribe from the label" do + delete api("/projects/#{project.id}/labels/#{label1.title}/subscription", user) + + expect(response.status).to eq(200) + expect(json_response["name"]).to eq(label1.title) + expect(json_response["subscribed"]).to be_falsey + end + end + + context "when label_id is a label ID" do + it "should unsubscribe from the label" do + delete api("/projects/#{project.id}/labels/#{label1.id}/subscription", user) + + expect(response.status).to eq(200) + expect(json_response["name"]).to eq(label1.title) + expect(json_response["subscribed"]).to be_falsey + end + end + + context "when user is already unsubscribed from label" do + before { label1.unsubscribe(user) } + + it "should return 304" do + delete api("/projects/#{project.id}/labels/#{label1.id}/subscription", user) + + expect(response.status).to eq(304) + end + end + + context "when label ID is not found" do + it "should a return 404 error" do + delete api("/projects/#{project.id}/labels/1234/subscription", user) + + expect(response.status).to eq(404) + end + end + end end -- cgit v1.2.1 From 05bbad5817c567b547177441769643042d699d8a Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Wed, 11 May 2016 15:17:16 -0500 Subject: Clean up EventsHelper spec --- spec/helpers/events_helper_spec.rb | 95 +++++++++++++++++++------------------- 1 file changed, 48 insertions(+), 47 deletions(-) (limited to 'spec') diff --git a/spec/helpers/events_helper_spec.rb b/spec/helpers/events_helper_spec.rb index e68a5ec29ab..c0d2be98e85 100644 --- a/spec/helpers/events_helper_spec.rb +++ b/spec/helpers/events_helper_spec.rb @@ -1,64 +1,65 @@ require 'spec_helper' describe EventsHelper do - include ApplicationHelper - include GitlabMarkdownHelper + describe '#event_note' do + before do + allow(helper).to receive(:current_user).and_return(double) + end - let(:current_user) { create(:user, email: "current@email.com") } + it 'should display one line of plain text without alteration' do + input = 'A short, plain note' + expect(helper.event_note(input)).to match(input) + expect(helper.event_note(input)).not_to match(/\.\.\.\z/) + end - it 'should display one line of plain text without alteration' do - input = 'A short, plain note' - expect(event_note(input)).to match(input) - expect(event_note(input)).not_to match(/\.\.\.\z/) - end + it 'should display inline code' do + input = 'A note with `inline code`' + expected = 'A note with inline code' - it 'should display inline code' do - input = 'A note with `inline code`' - expected = 'A note with inline code' + expect(helper.event_note(input)).to match(expected) + end - expect(event_note(input)).to match(expected) - end + it 'should truncate a note with multiple paragraphs' do + input = "Paragraph 1\n\nParagraph 2" + expected = 'Paragraph 1...' - it 'should truncate a note with multiple paragraphs' do - input = "Paragraph 1\n\nParagraph 2" - expected = 'Paragraph 1...' + expect(helper.event_note(input)).to match(expected) + end - expect(event_note(input)).to match(expected) - end + it 'should display the first line of a code block' do + input = "```\nCode block\nwith two lines\n```" + expected = %r{Code block\.\.\.} - it 'should display the first line of a code block' do - input = "```\nCode block\nwith two lines\n```" - expected = %r{Code block\.\.\.} + expect(helper.event_note(input)).to match(expected) + end - expect(event_note(input)).to match(expected) - end + it 'should truncate a single long line of text' do + text = 'The quick brown fox jumped over the lazy dog twice' # 50 chars + input = text * 4 + expected = (text * 2).sub(/.{3}/, '...') - it 'should truncate a single long line of text' do - text = 'The quick brown fox jumped over the lazy dog twice' # 50 chars - input = "#{text}#{text}#{text}#{text}" # 200 chars - expected = "#{text}#{text}".sub(/.{3}/, '...') + expect(helper.event_note(input)).to match(expected) + end - expect(event_note(input)).to match(expected) - end - - it 'should preserve a link href when link text is truncated' do - text = 'The quick brown fox jumped over the lazy dog' # 44 chars - input = "#{text}#{text}#{text} " # 133 chars - link_url = 'http://example.com/foo/bar/baz' # 30 chars - input << link_url - expected_link_text = 'http://example...' + it 'should preserve a link href when link text is truncated' do + text = 'The quick brown fox jumped over the lazy dog' # 44 chars + input = "#{text}#{text}#{text} " # 133 chars + link_url = 'http://example.com/foo/bar/baz' # 30 chars + input << link_url + expected_link_text = 'http://example...' - expect(event_note(input)).to match(link_url) - expect(event_note(input)).to match(expected_link_text) - end + expect(helper.event_note(input)).to match(link_url) + expect(helper.event_note(input)).to match(expected_link_text) + end - it 'should preserve code color scheme' do - input = "```ruby\ndef test\n 'hello world'\nend\n```" - expected = '
' \
-      "def test\n" \
-      "  \'hello world\'\n" \
-      "end" \
-      '
' - expect(event_note(input)).to eq(expected) + it 'should preserve code color scheme' do + input = "```ruby\ndef test\n 'hello world'\nend\n```" + expected = '
' \
+        "def test\n" \
+        "  \'hello world\'\n" \
+        "end" \
+        '
' + expect(helper.event_note(input)).to eq(expected) + end end end -- cgit v1.2.1 From 4fea5cda0bf7ff56dafae90306441b0d65f3ca05 Mon Sep 17 00:00:00 2001 From: Drew Blessing Date: Thu, 12 May 2016 17:11:37 -0500 Subject: Fix minor typos in admin health check page --- spec/features/admin/admin_health_check_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec') diff --git a/spec/features/admin/admin_health_check_spec.rb b/spec/features/admin/admin_health_check_spec.rb index 4fde04b609b..dec2dedf2b5 100644 --- a/spec/features/admin/admin_health_check_spec.rb +++ b/spec/features/admin/admin_health_check_spec.rb @@ -13,7 +13,7 @@ feature "Admin Health Check", feature: true do end it { page.has_text? 'Health Check' } - it { page.has_text? 'Health information can be reteived' } + it { page.has_text? 'Health information can be retrieved' } it 'has a health check access token' do token = current_application_settings.health_check_access_token -- cgit v1.2.1 From f5a0ac0fc197bae2eb5fe1045ed237cdbbaf6ea4 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Tue, 10 May 2016 23:58:06 -0300 Subject: Codestyle: make sure we have space around operators --- spec/lib/ci/gitlab_ci_yaml_processor_spec.rb | 4 ++-- .../gitlab/ci/build/artifacts/metadata/entry_spec.rb | 4 ++-- spec/lib/gitlab/lfs/lfs_router_spec.rb | 4 ++-- spec/models/commit_spec.rb | 2 +- spec/models/hooks/service_hook_spec.rb | 4 ++-- spec/models/hooks/system_hook_spec.rb | 20 ++++++++++---------- .../slack_service/note_message_spec.rb | 4 ++-- spec/requests/api/builds_spec.rb | 4 ++-- spec/requests/api/group_members_spec.rb | 10 +++++----- spec/requests/ci/api/builds_spec.rb | 4 ++-- spec/support/jira_service_helper.rb | 10 +++++----- spec/support/stub_gitlab_calls.rb | 20 ++++++++++---------- 12 files changed, 45 insertions(+), 45 deletions(-) (limited to 'spec') diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb index c7ab3185378..9eef8ea0976 100644 --- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb +++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb @@ -443,12 +443,12 @@ module Ci context 'when job variables are defined' do context 'when syntax is correct' do it 'returns job variables' do - variables = { + variables = { KEY1: 'value1', SOME_KEY_2: 'value2' } - config = YAML.dump( + config = YAML.dump( { before_script: ['pwd'], rspec: { variables: variables, diff --git a/spec/lib/gitlab/ci/build/artifacts/metadata/entry_spec.rb b/spec/lib/gitlab/ci/build/artifacts/metadata/entry_spec.rb index acca0b08bab..46a5b7fce65 100644 --- a/spec/lib/gitlab/ci/build/artifacts/metadata/entry_spec.rb +++ b/spec/lib/gitlab/ci/build/artifacts/metadata/entry_spec.rb @@ -10,8 +10,8 @@ describe Gitlab::Ci::Build::Artifacts::Metadata::Entry do 'path/dir_1/subdir/subfile' => { size: 10 }, 'path/second_dir' => {}, 'path/second_dir/dir_3/file_2' => { size: 10 }, - 'path/second_dir/dir_3/file_3'=> { size: 10 }, - 'another_directory/'=> {}, + 'path/second_dir/dir_3/file_3' => { size: 10 }, + 'another_directory/' => {}, 'another_file' => {}, '/file/with/absolute_path' => {} } end diff --git a/spec/lib/gitlab/lfs/lfs_router_spec.rb b/spec/lib/gitlab/lfs/lfs_router_spec.rb index 5852b31ab3a..3325190789b 100644 --- a/spec/lib/gitlab/lfs/lfs_router_spec.rb +++ b/spec/lib/gitlab/lfs/lfs_router_spec.rb @@ -26,8 +26,8 @@ describe Gitlab::Lfs::Router, lib: true do let(:sample_oid) { "b68143e6463773b1b6c6fd009a76c32aeec041faff32ba2ed42fd7f708a17f80" } let(:sample_size) { 499013 } - let(:respond_with_deprecated) {[ 501, { "Content-Type"=>"application/json; charset=utf-8" }, ["{\"message\":\"Server supports batch API only, please update your Git LFS client to version 1.0.1 and up.\",\"documentation_url\":\"#{Gitlab.config.gitlab.url}/help\"}"]]} - let(:respond_with_disabled) {[ 501, { "Content-Type"=>"application/json; charset=utf-8" }, ["{\"message\":\"Git LFS is not enabled on this GitLab server, contact your admin.\",\"documentation_url\":\"#{Gitlab.config.gitlab.url}/help\"}"]]} + let(:respond_with_deprecated) {[ 501, { "Content-Type" => "application/json; charset=utf-8" }, ["{\"message\":\"Server supports batch API only, please update your Git LFS client to version 1.0.1 and up.\",\"documentation_url\":\"#{Gitlab.config.gitlab.url}/help\"}"]]} + let(:respond_with_disabled) {[ 501, { "Content-Type" => "application/json; charset=utf-8" }, ["{\"message\":\"Git LFS is not enabled on this GitLab server, contact your admin.\",\"documentation_url\":\"#{Gitlab.config.gitlab.url}/help\"}"]]} describe 'when lfs is disabled' do before do diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index ad47e338a33..ccb100cd96f 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -56,7 +56,7 @@ describe Commit, models: true do end it "does not truncates a message with a newline after 80 but less 100 characters" do - message =<'application/json', 'X-Gitlab-Event'=>'Service Hook' } + headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'Service Hook' } ).once end it "POSTs the data as JSON" do @service_hook.execute(@data) expect(WebMock).to have_requested(:post, @service_hook.url).with( - headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'Service Hook' } + headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'Service Hook' } ).once end diff --git a/spec/models/hooks/system_hook_spec.rb b/spec/models/hooks/system_hook_spec.rb index 56a9fbe9720..4078b9e4ff5 100644 --- a/spec/models/hooks/system_hook_spec.rb +++ b/spec/models/hooks/system_hook_spec.rb @@ -33,7 +33,7 @@ describe SystemHook, models: true do Projects::CreateService.new(user, name: 'empty').execute expect(WebMock).to have_requested(:post, system_hook.url).with( body: /project_create/, - headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook' } + headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'System Hook' } ).once end @@ -42,7 +42,7 @@ describe SystemHook, models: true do expect(WebMock).to have_requested(:post, system_hook.url).with( body: /project_destroy/, - headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook' } + headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'System Hook' } ).once end @@ -51,7 +51,7 @@ describe SystemHook, models: true do expect(WebMock).to have_requested(:post, system_hook.url).with( body: /user_create/, - headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook' } + headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'System Hook' } ).once end @@ -60,7 +60,7 @@ describe SystemHook, models: true do expect(WebMock).to have_requested(:post, system_hook.url).with( body: /user_destroy/, - headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook' } + headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'System Hook' } ).once end @@ -69,7 +69,7 @@ describe SystemHook, models: true do expect(WebMock).to have_requested(:post, system_hook.url).with( body: /user_add_to_team/, - headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook' } + headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'System Hook' } ).once end @@ -79,7 +79,7 @@ describe SystemHook, models: true do expect(WebMock).to have_requested(:post, system_hook.url).with( body: /user_remove_from_team/, - headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook' } + headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'System Hook' } ).once end @@ -88,7 +88,7 @@ describe SystemHook, models: true do expect(WebMock).to have_requested(:post, system_hook.url).with( body: /group_create/, - headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook' } + headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'System Hook' } ).once end @@ -97,7 +97,7 @@ describe SystemHook, models: true do expect(WebMock).to have_requested(:post, system_hook.url).with( body: /group_destroy/, - headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook' } + headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'System Hook' } ).once end @@ -106,7 +106,7 @@ describe SystemHook, models: true do expect(WebMock).to have_requested(:post, system_hook.url).with( body: /user_add_to_group/, - headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook' } + headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'System Hook' } ).once end @@ -116,7 +116,7 @@ describe SystemHook, models: true do expect(WebMock).to have_requested(:post, system_hook.url).with( body: /user_remove_from_group/, - headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook' } + headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'System Hook' } ).once end end diff --git a/spec/models/project_services/slack_service/note_message_spec.rb b/spec/models/project_services/slack_service/note_message_spec.rb index d37590cab75..379c3e1219c 100644 --- a/spec/models/project_services/slack_service/note_message_spec.rb +++ b/spec/models/project_services/slack_service/note_message_spec.rb @@ -65,7 +65,7 @@ describe SlackService::NoteMessage, models: true do expect(message.pretext).to eq("Test User commented on " \ " in : " \ "*merge request title*") - expected_attachments = [ + expected_attachments = [ { text: "comment on a merge request", color: color, @@ -117,7 +117,7 @@ describe SlackService::NoteMessage, models: true do expect(message.pretext).to eq("Test User commented on " \ " in : " \ "*snippet title*") - expected_attachments = [ + expected_attachments = [ { text: "comment on a snippet", color: color, diff --git a/spec/requests/api/builds_spec.rb b/spec/requests/api/builds_spec.rb index 5ead735be48..0fbc984c061 100644 --- a/spec/requests/api/builds_spec.rb +++ b/spec/requests/api/builds_spec.rb @@ -106,8 +106,8 @@ describe API::API, api: true do context 'authorized user' do let(:download_headers) do - { 'Content-Transfer-Encoding'=>'binary', - 'Content-Disposition'=>'attachment; filename=ci_build_artifacts.zip' } + { 'Content-Transfer-Encoding' => 'binary', + 'Content-Disposition' => 'attachment; filename=ci_build_artifacts.zip' } end it 'should return specific build artifacts' do diff --git a/spec/requests/api/group_members_spec.rb b/spec/requests/api/group_members_spec.rb index 96d89e69209..02553d0f8e2 100644 --- a/spec/requests/api/group_members_spec.rb +++ b/spec/requests/api/group_members_spec.rb @@ -34,11 +34,11 @@ describe API::API, api: true do expect(response.status).to eq(200) expect(json_response).to be_an Array expect(json_response.size).to eq(5) - expect(json_response.find { |e| e['id']==owner.id }['access_level']).to eq(GroupMember::OWNER) - expect(json_response.find { |e| e['id']==reporter.id }['access_level']).to eq(GroupMember::REPORTER) - expect(json_response.find { |e| e['id']==developer.id }['access_level']).to eq(GroupMember::DEVELOPER) - expect(json_response.find { |e| e['id']==master.id }['access_level']).to eq(GroupMember::MASTER) - expect(json_response.find { |e| e['id']==guest.id }['access_level']).to eq(GroupMember::GUEST) + expect(json_response.find { |e| e['id'] == owner.id }['access_level']).to eq(GroupMember::OWNER) + expect(json_response.find { |e| e['id'] == reporter.id }['access_level']).to eq(GroupMember::REPORTER) + expect(json_response.find { |e| e['id'] == developer.id }['access_level']).to eq(GroupMember::DEVELOPER) + expect(json_response.find { |e| e['id'] == master.id }['access_level']).to eq(GroupMember::MASTER) + expect(json_response.find { |e| e['id'] == guest.id }['access_level']).to eq(GroupMember::GUEST) end end diff --git a/spec/requests/ci/api/builds_spec.rb b/spec/requests/ci/api/builds_spec.rb index dfd361a2cdd..cae4656010f 100644 --- a/spec/requests/ci/api/builds_spec.rb +++ b/spec/requests/ci/api/builds_spec.rb @@ -402,8 +402,8 @@ describe Ci::API::API do context 'build has artifacts' do let(:build) { create(:ci_build, :artifacts) } let(:download_headers) do - { 'Content-Transfer-Encoding'=>'binary', - 'Content-Disposition'=>'attachment; filename=ci_build_artifacts.zip' } + { 'Content-Transfer-Encoding' => 'binary', + 'Content-Disposition' => 'attachment; filename=ci_build_artifacts.zip' } end it 'should download artifact' do diff --git a/spec/support/jira_service_helper.rb b/spec/support/jira_service_helper.rb index a3f496359b1..5ebe095743b 100644 --- a/spec/support/jira_service_helper.rb +++ b/spec/support/jira_service_helper.rb @@ -2,11 +2,11 @@ module JiraServiceHelper def jira_service_settings properties = { - "title"=>"JIRA tracker", - "project_url"=>"http://jira.example/issues/?jql=project=A", - "issues_url"=>"http://jira.example/browse/JIRA-1", - "new_issue_url"=>"http://jira.example/secure/CreateIssue.jspa", - "api_url"=>"http://jira.example/rest/api/2" + "title" => "JIRA tracker", + "project_url" => "http://jira.example/issues/?jql=project=A", + "issues_url" => "http://jira.example/browse/JIRA-1", + "new_issue_url" => "http://jira.example/secure/CreateIssue.jspa", + "api_url" => "http://jira.example/rest/api/2" } jira_tracker.update_attributes(properties: properties, active: true) diff --git a/spec/support/stub_gitlab_calls.rb b/spec/support/stub_gitlab_calls.rb index eec2e681117..b5ca34bc028 100644 --- a/spec/support/stub_gitlab_calls.rb +++ b/spec/support/stub_gitlab_calls.rb @@ -36,20 +36,20 @@ module StubGitlabCalls stub_request(:post, "#{gitlab_url}api/v3/session.json"). with(body: "{\"email\":\"test@test.com\",\"password\":\"123456\"}", - headers: { 'Content-Type'=>'application/json' }). - to_return(status: 201, body: f, headers: { 'Content-Type'=>'application/json' }) + headers: { 'Content-Type' => 'application/json' }). + to_return(status: 201, body: f, headers: { 'Content-Type' => 'application/json' }) end def stub_user f = File.read(Rails.root.join('spec/support/gitlab_stubs/user.json')) stub_request(:get, "#{gitlab_url}api/v3/user?private_token=Wvjy2Krpb7y8xi93owUz"). - with(headers: { 'Content-Type'=>'application/json' }). - to_return(status: 200, body: f, headers: { 'Content-Type'=>'application/json' }) + with(headers: { 'Content-Type' => 'application/json' }). + to_return(status: 200, body: f, headers: { 'Content-Type' => 'application/json' }) stub_request(:get, "#{gitlab_url}api/v3/user?access_token=some_token"). - with(headers: { 'Content-Type'=>'application/json' }). - to_return(status: 200, body: f, headers: { 'Content-Type'=>'application/json' }) + with(headers: { 'Content-Type' => 'application/json' }). + to_return(status: 200, body: f, headers: { 'Content-Type' => 'application/json' }) end def stub_project_8 @@ -66,19 +66,19 @@ module StubGitlabCalls f = File.read(Rails.root.join('spec/support/gitlab_stubs/projects.json')) stub_request(:get, "#{gitlab_url}api/v3/projects.json?archived=false&ci_enabled_first=true&private_token=Wvjy2Krpb7y8xi93owUz"). - with(headers: { 'Content-Type'=>'application/json' }). - to_return(status: 200, body: f, headers: { 'Content-Type'=>'application/json' }) + with(headers: { 'Content-Type' => 'application/json' }). + to_return(status: 200, body: f, headers: { 'Content-Type' => 'application/json' }) end def stub_projects_owned stub_request(:get, "#{gitlab_url}api/v3/projects/owned.json?archived=false&ci_enabled_first=true&private_token=Wvjy2Krpb7y8xi93owUz"). - with(headers: { 'Content-Type'=>'application/json' }). + with(headers: { 'Content-Type' => 'application/json' }). to_return(status: 200, body: "", headers: {}) end def stub_ci_enable stub_request(:put, "#{gitlab_url}api/v3/projects/2/services/gitlab-ci.json?private_token=Wvjy2Krpb7y8xi93owUz"). - with(headers: { 'Content-Type'=>'application/json' }). + with(headers: { 'Content-Type' => 'application/json' }). to_return(status: 200, body: "", headers: {}) end -- cgit v1.2.1 From e5c97101478829fe8200557afd02bde92c3eb4e4 Mon Sep 17 00:00:00 2001 From: Artem Sidorenko Date: Tue, 3 May 2016 17:33:43 +0200 Subject: Use the relative url prefix for links in Wiki --- spec/models/project_wiki_spec.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'spec') diff --git a/spec/models/project_wiki_spec.rb b/spec/models/project_wiki_spec.rb index 532e3f013fd..91ebb612baa 100644 --- a/spec/models/project_wiki_spec.rb +++ b/spec/models/project_wiki_spec.rb @@ -38,7 +38,8 @@ describe ProjectWiki, models: true do describe "#wiki_base_path" do it "returns the wiki base path" do - wiki_base_path = "/#{project.path_with_namespace}/wikis" + wiki_base_path = "#{Gitlab.config.gitlab.relative_url_root}/#{project.path_with_namespace}/wikis" + expect(subject.wiki_base_path).to eq(wiki_base_path) end end -- cgit v1.2.1 From 3b50867550818413eed01fcbe85e5442707150be Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Tue, 26 Apr 2016 12:03:48 -0300 Subject: Fix spec for Gitlab::GithubImport::PullRequestFormatter --- .../github_import/pull_request_formatter_spec.rb | 92 ++++++++++++++++------ 1 file changed, 70 insertions(+), 22 deletions(-) (limited to 'spec') diff --git a/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb b/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb index e59c0ca110e..b1f3d17373b 100644 --- a/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb +++ b/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb @@ -4,9 +4,9 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do let(:project) { create(:project) } let(:repository) { double(id: 1, fork: false) } let(:source_repo) { repository } - let(:source_branch) { double(ref: 'feature', repo: source_repo) } + let(:source_branch) { double(ref: 'feature', repo: source_repo, sha: '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b') } let(:target_repo) { repository } - let(:target_branch) { double(ref: 'master', repo: target_repo) } + let(:target_branch) { double(ref: 'master', repo: target_repo, sha: '8ffb3c15a5475e59ae909384297fede4badcb4c7') } let(:octocat) { double(id: 123456, login: 'octocat') } let(:created_at) { DateTime.strptime('2011-01-26T19:01:12Z') } let(:updated_at) { DateTime.strptime('2011-01-27T19:01:12Z') } @@ -137,11 +137,11 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do let(:milestone) { double(number: 45) } let(:raw_data) { double(base_data.merge(milestone: milestone)) } - it 'returns nil when milestone does not exists' do + it 'returns nil when milestone does not exist' do expect(pull_request.attributes.fetch(:milestone)).to be_nil end - it 'returns milestone when is exists' do + it 'returns milestone when it exists' do milestone = create(:milestone, project: project, iid: 45) expect(pull_request.attributes.fetch(:milestone)).to eq milestone @@ -157,32 +157,80 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do end end - describe '#valid?' do - let(:invalid_branch) { double(ref: 'invalid-branch').as_null_object } + describe '#source_branch' do + let(:raw_data) { double(base_data) } + + it 'returns head ref' do + expect(pull_request.source_branch).to eq 'feature' + end + end + + describe '#source_sha' do + let(:raw_data) { double(base_data) } + + it 'returns head sha' do + expect(pull_request.source_sha).to eq '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b' + end + end + + describe '#source_branch_exists?' do + context 'when source branch exists' do + let(:raw_data) { double(base_data) } + + it 'returns true' do + expect(pull_request.source_branch_exists?).to eq true + end + end + + context 'when source branch does not exist' do + let(:raw_data) { double(base_data.merge(head: double(ref: 'removed-branch').as_null_object)) } + + it 'returns false' do + expect(pull_request.source_branch_exists?).to eq false + end + end + end + + describe '#target_branch' do + let(:raw_data) { double(base_data) } - context 'when source, and target repositories are the same' do - context 'and source and target branches exists' do - let(:raw_data) { double(base_data.merge(head: source_branch, base: target_branch)) } + it 'returns base ref' do + expect(pull_request.target_branch).to eq 'master' + end + end + + describe '#target_sha' do + let(:raw_data) { double(base_data) } + + it 'returns base sha' do + expect(pull_request.target_sha).to eq '8ffb3c15a5475e59ae909384297fede4badcb4c7' + end + end - it 'returns true' do - expect(pull_request.valid?).to eq true - end + describe '#target_branch_exists?' do + context 'when target branch exists' do + let(:raw_data) { double(base_data) } + + it 'returns true' do + expect(pull_request.target_branch_exists?).to eq true end + end - context 'and source branch doesn not exists' do - let(:raw_data) { double(base_data.merge(head: invalid_branch, base: target_branch)) } + context 'when target branch does not exist' do + let(:raw_data) { double(base_data.merge(base: double(ref: 'removed-branch').as_null_object)) } - it 'returns false' do - expect(pull_request.valid?).to eq false - end + it 'returns false' do + expect(pull_request.target_branch_exists?).to eq false end + end + end - context 'and target branch doesn not exists' do - let(:raw_data) { double(base_data.merge(head: source_branch, base: invalid_branch)) } + describe '#valid?' do + context 'when source, and target repos are not a fork' do + let(:raw_data) { double(base_data) } - it 'returns false' do - expect(pull_request.valid?).to eq false - end + it 'returns true' do + expect(pull_request.valid?).to eq true end end -- cgit v1.2.1 From 99d3e21f19ffb5cccb58fdfeac4fb6174e7e65e2 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 10 May 2016 17:41:46 -0500 Subject: Extract LegacyDiffNote out of Note --- spec/models/note_spec.rb | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) (limited to 'spec') diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 4b788b57882..264888cb376 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -43,12 +43,8 @@ describe Note, models: true do expect(note.noteable.id).to eq(commit.id) end - it "should be recognized by #for_diff_line?" do - expect(note).to be_for_diff_line - end - - it "should be recognized by #for_commit_diff_line?" do - expect(note).to be_for_commit_diff_line + it "should be recognized by #legacy_diff_note?" do + expect(note).to be_legacy_diff_note end end -- cgit v1.2.1 From 7848d54f5b0024f58d8ce2b0259347816be5bdbc Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Fri, 13 May 2016 14:53:31 -0500 Subject: Clean up LegacyDiffNote somewhat --- spec/models/legacy_diff_note_spec.rb | 74 ++++++++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 spec/models/legacy_diff_note_spec.rb (limited to 'spec') diff --git a/spec/models/legacy_diff_note_spec.rb b/spec/models/legacy_diff_note_spec.rb new file mode 100644 index 00000000000..7c29bef54e4 --- /dev/null +++ b/spec/models/legacy_diff_note_spec.rb @@ -0,0 +1,74 @@ +require 'spec_helper' + +describe LegacyDiffNote, models: true do + describe "Commit diff line notes" do + let!(:note) { create(:note_on_commit_diff, note: "+1 from me") } + let!(:commit) { note.noteable } + + it "should save a valid note" do + expect(note.commit_id).to eq(commit.id) + expect(note.noteable.id).to eq(commit.id) + end + + it "should be recognized by #legacy_diff_note?" do + expect(note).to be_legacy_diff_note + end + end + + describe '#active?' do + it 'is always true when the note has no associated diff' do + note = build(:note_on_merge_request_diff) + + expect(note).to receive(:diff).and_return(nil) + + expect(note).to be_active + end + + it 'is never true when the note has no noteable associated' do + note = build(:note_on_merge_request_diff) + + expect(note).to receive(:diff).and_return(double) + expect(note).to receive(:noteable).and_return(nil) + + expect(note).not_to be_active + end + + it 'returns the memoized value if defined' do + note = build(:note_on_merge_request_diff) + + note.instance_variable_set(:@active, 'foo') + expect(note).not_to receive(:find_noteable_diff) + + expect(note.active?).to eq 'foo' + end + + context 'for a merge request noteable' do + it 'is false when noteable has no matching diff' do + merge = build_stubbed(:merge_request, :simple) + note = build(:note_on_merge_request_diff, noteable: merge) + + allow(note).to receive(:diff).and_return(double) + expect(note).to receive(:find_noteable_diff).and_return(nil) + + expect(note).not_to be_active + end + + it 'is true when noteable has a matching diff' do + merge = create(:merge_request, :simple) + + # Generate a real line_code value so we know it will match. We use a + # random line from a random diff just for funsies. + diff = merge.diffs.to_a.sample + line = Gitlab::Diff::Parser.new.parse(diff.diff.each_line).to_a.sample + code = Gitlab::Diff::LineCode.generate(diff.new_path, line.new_pos, line.old_pos) + + # We're persisting in order to trigger the set_diff callback + note = create(:note_on_merge_request_diff, noteable: merge, line_code: code) + + # Make sure we don't get a false positive from a guard clause + expect(note).to receive(:find_noteable_diff).and_call_original + expect(note).to be_active + end + end + end +end -- cgit v1.2.1 From c452fa8124ffe18e2e74e14491dcb3e419e60057 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Fri, 13 May 2016 14:53:42 -0500 Subject: Update specs --- spec/factories/notes.rb | 4 +- spec/features/notes_on_merge_requests_spec.rb | 2 +- spec/models/note_spec.rb | 76 +-------------------------- 3 files changed, 4 insertions(+), 78 deletions(-) (limited to 'spec') diff --git a/spec/factories/notes.rb b/spec/factories/notes.rb index 840b13196a6..26719f2652c 100644 --- a/spec/factories/notes.rb +++ b/spec/factories/notes.rb @@ -9,10 +9,10 @@ FactoryGirl.define do author factory :note_on_commit, traits: [:on_commit] - factory :note_on_commit_diff, traits: [:on_commit, :on_diff] + factory :note_on_commit_diff, traits: [:on_commit, :on_diff], class: LegacyDiffNote factory :note_on_issue, traits: [:on_issue], aliases: [:votable_note] factory :note_on_merge_request, traits: [:on_merge_request] - factory :note_on_merge_request_diff, traits: [:on_merge_request, :on_diff] + factory :note_on_merge_request_diff, traits: [:on_merge_request, :on_diff], class: LegacyDiffNote factory :note_on_project_snippet, traits: [:on_project_snippet] factory :system_note, traits: [:system] factory :downvote_note, traits: [:award, :downvote] diff --git a/spec/features/notes_on_merge_requests_spec.rb b/spec/features/notes_on_merge_requests_spec.rb index 389812ff7e1..9e9fec01943 100644 --- a/spec/features/notes_on_merge_requests_spec.rb +++ b/spec/features/notes_on_merge_requests_spec.rb @@ -192,7 +192,7 @@ describe 'Comments', feature: true do end it 'should be removed when canceled' do - page.within(".diff-file form[id$='#{line_code}']") do + page.within(".diff-file form[id$='#{line_code}-true']") do find('.js-close-discussion-note-form').trigger('click') end diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 264888cb376..5d916f0e6a6 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -34,20 +34,6 @@ describe Note, models: true do end end - describe "Commit diff line notes" do - let!(:note) { create(:note_on_commit_diff, note: "+1 from me") } - let!(:commit) { note.noteable } - - it "should save a valid note" do - expect(note.commit_id).to eq(commit.id) - expect(note.noteable.id).to eq(commit.id) - end - - it "should be recognized by #legacy_diff_note?" do - expect(note).to be_legacy_diff_note - end - end - describe 'authorization' do before do @p1 = create(:project) @@ -144,66 +130,6 @@ describe Note, models: true do end end - describe '#active?' do - it 'is always true when the note has no associated diff' do - note = build(:note) - - expect(note).to receive(:diff).and_return(nil) - - expect(note).to be_active - end - - it 'is never true when the note has no noteable associated' do - note = build(:note) - - expect(note).to receive(:diff).and_return(double) - expect(note).to receive(:noteable).and_return(nil) - - expect(note).not_to be_active - end - - it 'returns the memoized value if defined' do - note = build(:note) - - expect(note).to receive(:diff).and_return(double) - expect(note).to receive(:noteable).and_return(double) - - note.instance_variable_set(:@active, 'foo') - expect(note).not_to receive(:find_noteable_diff) - - expect(note.active?).to eq 'foo' - end - - context 'for a merge request noteable' do - it 'is false when noteable has no matching diff' do - merge = build_stubbed(:merge_request, :simple) - note = build(:note, noteable: merge) - - allow(note).to receive(:diff).and_return(double) - expect(note).to receive(:find_noteable_diff).and_return(nil) - - expect(note).not_to be_active - end - - it 'is true when noteable has a matching diff' do - merge = create(:merge_request, :simple) - - # Generate a real line_code value so we know it will match. We use a - # random line from a random diff just for funsies. - diff = merge.diffs.to_a.sample - line = Gitlab::Diff::Parser.new.parse(diff.diff.each_line).to_a.sample - code = Gitlab::Diff::LineCode.generate(diff.new_path, line.new_pos, line.old_pos) - - # We're persisting in order to trigger the set_diff callback - note = create(:note, noteable: merge, line_code: code) - - # Make sure we don't get a false positive from a guard clause - expect(note).to receive(:find_noteable_diff).and_call_original - expect(note).to be_active - end - end - end - describe "editable?" do it "returns true" do note = build(:note) @@ -254,7 +180,7 @@ describe Note, models: true do end it "is not an award emoji when comment is on a diff" do - note = create(:note, note: ":blowfish:", noteable: merge_request, line_code: "11d5d2e667e9da4f7f610f81d86c974b146b13bd_0_2") + note = create(:note_on_merge_request_diff, note: ":blowfish:", noteable: merge_request, line_code: "11d5d2e667e9da4f7f610f81d86c974b146b13bd_0_2") note = note.reload expect(note.note).to eq(":blowfish:") -- cgit v1.2.1 From 7e1f14e21517d3907a0e096d44b30797612f69cd Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Tue, 26 Apr 2016 19:57:37 -0300 Subject: Preserve commits/diff/comments for PRs that were merged on GitHub --- .../gitlab/github_import/pull_request_formatter_spec.rb | 6 ++++++ spec/models/merge_request_spec.rb | 15 +++++++++++++-- 2 files changed, 19 insertions(+), 2 deletions(-) (limited to 'spec') diff --git a/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb b/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb index b1f3d17373b..1d15d3d937e 100644 --- a/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb +++ b/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb @@ -41,8 +41,10 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do description: "*Created by: octocat*\n\nPlease pull these awesome changes", source_project: project, source_branch: 'feature', + head_source_sha: '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b', target_project: project, target_branch: 'master', + base_target_sha: '8ffb3c15a5475e59ae909384297fede4badcb4c7', state: 'opened', milestone: nil, author_id: project.creator_id, @@ -66,8 +68,10 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do description: "*Created by: octocat*\n\nPlease pull these awesome changes", source_project: project, source_branch: 'feature', + head_source_sha: '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b', target_project: project, target_branch: 'master', + base_target_sha: '8ffb3c15a5475e59ae909384297fede4badcb4c7', state: 'closed', milestone: nil, author_id: project.creator_id, @@ -91,8 +95,10 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do description: "*Created by: octocat*\n\nPlease pull these awesome changes", source_project: project, source_branch: 'feature', + head_source_sha: '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b', target_project: project, target_branch: 'master', + base_target_sha: '8ffb3c15a5475e59ae909384297fede4badcb4c7', state: 'merged', milestone: nil, author_id: project.creator_id, diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index c8578749b21..9eef08c6d00 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -64,7 +64,13 @@ describe MergeRequest, models: true do describe '#target_sha' do context 'when the target branch does not exist anymore' do - subject { create(:merge_request).tap { |mr| mr.update_attribute(:target_branch, 'deleted') } } + let(:project) { create(:project) } + + subject { create(:merge_request, source_project: project, target_project: project) } + + before do + project.repository.raw_repository.delete_branch(subject.target_branch) + end it 'returns nil' do expect(subject.target_sha).to be_nil @@ -289,7 +295,12 @@ describe MergeRequest, models: true do let(:fork_project) { create(:project, forked_from_project: project) } context 'when the target branch does not exist anymore' do - subject { create(:merge_request).tap { |mr| mr.update_attribute(:target_branch, 'deleted') } } + subject { create(:merge_request, source_project: project, target_project: project) } + + before do + project.repository.raw_repository.delete_branch(subject.target_branch) + subject.reload + end it 'does not crash' do expect{ subject.diverged_commits_count }.not_to raise_error -- cgit v1.2.1 From e001bd5e3d813fa72f724fc0e661b36099f678ea Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Mon, 9 May 2016 16:17:05 -0500 Subject: Import PRs where branch names were reused across PRs --- .../github_import/pull_request_formatter_spec.rb | 32 ++++++++++++++++++---- 1 file changed, 26 insertions(+), 6 deletions(-) (limited to 'spec') diff --git a/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb b/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb index 1d15d3d937e..adedc57719e 100644 --- a/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb +++ b/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb @@ -164,10 +164,20 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do end describe '#source_branch' do - let(:raw_data) { double(base_data) } + context 'when source branch exists' do + let(:raw_data) { double(base_data) } - it 'returns head ref' do - expect(pull_request.source_branch).to eq 'feature' + it 'returns head ref' do + expect(pull_request.source_branch).to eq 'feature' + end + end + + context 'when source branch does not exist' do + let(:raw_data) { double(base_data.merge(head: double(ref: 'removed-branch', sha: '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b'))) } + + it 'returns head ref' do + expect(pull_request.source_branch).to eq 'removed-branch-2e5d3239' + end end end @@ -198,10 +208,20 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do end describe '#target_branch' do - let(:raw_data) { double(base_data) } + context 'when target branch exists' do + let(:raw_data) { double(base_data) } - it 'returns base ref' do - expect(pull_request.target_branch).to eq 'master' + it 'returns base ref' do + expect(pull_request.target_branch).to eq 'master' + end + end + + context 'when target branch does not exist' do + let(:raw_data) { double(base_data.merge(base: double(ref: 'removed-branch', sha: '8ffb3c15a5475e59ae909384297fede4badcb4c7'))) } + + it 'returns head ref' do + expect(pull_request.target_branch).to eq 'removed-branch-8ffb3c15' + end end end -- cgit v1.2.1 From 795a7ca8f171a1634134e8b8ce5cf80a65381903 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Mon, 9 May 2016 17:45:37 -0500 Subject: Extract GitHub branch formatter --- .../gitlab/github_import/branch_formatter_spec.rb | 57 ++++++++++++++ .../github_import/pull_request_formatter_spec.rb | 88 ---------------------- 2 files changed, 57 insertions(+), 88 deletions(-) create mode 100644 spec/lib/gitlab/github_import/branch_formatter_spec.rb (limited to 'spec') diff --git a/spec/lib/gitlab/github_import/branch_formatter_spec.rb b/spec/lib/gitlab/github_import/branch_formatter_spec.rb new file mode 100644 index 00000000000..58bfdd6c72b --- /dev/null +++ b/spec/lib/gitlab/github_import/branch_formatter_spec.rb @@ -0,0 +1,57 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::BranchFormatter, lib: true do + let(:project) { create(:project) } + let(:repo) { double } + let(:raw) do + { + ref: 'feature', + repo: repo, + sha: '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b' + } + end + + describe '#exists?' do + it 'returns true when branch exists' do + branch = described_class.new(project, double(raw)) + + expect(branch.exists?).to eq true + end + + it 'returns false when branch does not exist' do + branch = described_class.new(project, double(raw.merge(ref: 'removed-branch'))) + + expect(branch.exists?).to eq false + end + end + + describe '#name' do + it 'returns raw ref when branch exists' do + branch = described_class.new(project, double(raw)) + + expect(branch.name).to eq 'feature' + end + + it 'returns formatted ref when branch does not exist' do + branch = described_class.new(project, double(raw.merge(ref: 'removed-branch'))) + + expect(branch.name).to eq 'removed-branch-2e5d3239' + end + end + + describe '#repo' do + it 'returns raw repo' do + branch = described_class.new(project, double(raw)) + + expect(branch.repo).to eq repo + end + end + + describe '#sha' do + it 'returns raw sha' do + branch = described_class.new(project, double(raw)) + + expect(branch.sha).to eq '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b' + end + end +end diff --git a/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb b/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb index adedc57719e..5fed98e3926 100644 --- a/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb +++ b/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb @@ -163,94 +163,6 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do end end - describe '#source_branch' do - context 'when source branch exists' do - let(:raw_data) { double(base_data) } - - it 'returns head ref' do - expect(pull_request.source_branch).to eq 'feature' - end - end - - context 'when source branch does not exist' do - let(:raw_data) { double(base_data.merge(head: double(ref: 'removed-branch', sha: '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b'))) } - - it 'returns head ref' do - expect(pull_request.source_branch).to eq 'removed-branch-2e5d3239' - end - end - end - - describe '#source_sha' do - let(:raw_data) { double(base_data) } - - it 'returns head sha' do - expect(pull_request.source_sha).to eq '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b' - end - end - - describe '#source_branch_exists?' do - context 'when source branch exists' do - let(:raw_data) { double(base_data) } - - it 'returns true' do - expect(pull_request.source_branch_exists?).to eq true - end - end - - context 'when source branch does not exist' do - let(:raw_data) { double(base_data.merge(head: double(ref: 'removed-branch').as_null_object)) } - - it 'returns false' do - expect(pull_request.source_branch_exists?).to eq false - end - end - end - - describe '#target_branch' do - context 'when target branch exists' do - let(:raw_data) { double(base_data) } - - it 'returns base ref' do - expect(pull_request.target_branch).to eq 'master' - end - end - - context 'when target branch does not exist' do - let(:raw_data) { double(base_data.merge(base: double(ref: 'removed-branch', sha: '8ffb3c15a5475e59ae909384297fede4badcb4c7'))) } - - it 'returns head ref' do - expect(pull_request.target_branch).to eq 'removed-branch-8ffb3c15' - end - end - end - - describe '#target_sha' do - let(:raw_data) { double(base_data) } - - it 'returns base sha' do - expect(pull_request.target_sha).to eq '8ffb3c15a5475e59ae909384297fede4badcb4c7' - end - end - - describe '#target_branch_exists?' do - context 'when target branch exists' do - let(:raw_data) { double(base_data) } - - it 'returns true' do - expect(pull_request.target_branch_exists?).to eq true - end - end - - context 'when target branch does not exist' do - let(:raw_data) { double(base_data.merge(base: double(ref: 'removed-branch').as_null_object)) } - - it 'returns false' do - expect(pull_request.target_branch_exists?).to eq false - end - end - end - describe '#valid?' do context 'when source, and target repos are not a fork' do let(:raw_data) { double(base_data) } -- cgit v1.2.1 From ebaa19c162bec7dce64db25124e448d832c17384 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Tue, 10 May 2016 16:23:59 -0500 Subject: Fix validation method for Gitlab::GithubImport::PullRequestFormatter --- spec/lib/gitlab/github_import/branch_formatter_spec.rb | 14 ++++++++++++++ .../gitlab/github_import/pull_request_formatter_spec.rb | 4 ++-- 2 files changed, 16 insertions(+), 2 deletions(-) (limited to 'spec') diff --git a/spec/lib/gitlab/github_import/branch_formatter_spec.rb b/spec/lib/gitlab/github_import/branch_formatter_spec.rb index 58bfdd6c72b..3cb634ba010 100644 --- a/spec/lib/gitlab/github_import/branch_formatter_spec.rb +++ b/spec/lib/gitlab/github_import/branch_formatter_spec.rb @@ -54,4 +54,18 @@ describe Gitlab::GithubImport::BranchFormatter, lib: true do expect(branch.sha).to eq '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b' end end + + describe '#valid?' do + it 'returns true when repository exists' do + branch = described_class.new(project, double(raw)) + + expect(branch.valid?).to eq true + end + + it 'returns false when repository does not exist' do + branch = described_class.new(project, double(raw.merge(repo: nil))) + + expect(branch.valid?).to eq false + end + end end diff --git a/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb b/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb index 5fed98e3926..120f59e6e71 100644 --- a/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb +++ b/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb @@ -173,7 +173,7 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do end context 'when source repo is a fork' do - let(:source_repo) { double(id: 2, fork: true) } + let(:source_repo) { double(id: 2) } let(:raw_data) { double(base_data) } it 'returns false' do @@ -182,7 +182,7 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do end context 'when target repo is a fork' do - let(:target_repo) { double(id: 2, fork: true) } + let(:target_repo) { double(id: 2) } let(:raw_data) { double(base_data) } it 'returns false' do -- cgit v1.2.1 From 774a5107822d3d451b88ed3a7257aeac8d91c35a Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Sat, 14 May 2016 09:39:33 -0500 Subject: Fix specs --- spec/services/jwt/container_registry_authentication_service_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec') diff --git a/spec/services/jwt/container_registry_authentication_service_spec.rb b/spec/services/jwt/container_registry_authentication_service_spec.rb index 1873ea2639b..7c879852520 100644 --- a/spec/services/jwt/container_registry_authentication_service_spec.rb +++ b/spec/services/jwt/container_registry_authentication_service_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe JWT::ContainerRegistryAuthenticationService, services: true do +describe Gitlab::JWT::ContainerRegistryAuthenticationService, services: true do let(:current_project) { nil } let(:current_user) { nil } let(:current_params) { {} } -- cgit v1.2.1 From 63cdf1aeb04b9694c0b6d44b1141868fcc5a0904 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Sat, 14 May 2016 11:11:48 -0500 Subject: Use Auth::ContainerRegistryAuthenticationService --- spec/services/jwt/container_registry_authentication_service_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec') diff --git a/spec/services/jwt/container_registry_authentication_service_spec.rb b/spec/services/jwt/container_registry_authentication_service_spec.rb index 7c879852520..1873ea2639b 100644 --- a/spec/services/jwt/container_registry_authentication_service_spec.rb +++ b/spec/services/jwt/container_registry_authentication_service_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::JWT::ContainerRegistryAuthenticationService, services: true do +describe JWT::ContainerRegistryAuthenticationService, services: true do let(:current_project) { nil } let(:current_user) { nil } let(:current_params) { {} } -- cgit v1.2.1 From 0a9979d9ef3b5205e36ecfb66ff6322fa824492f Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Sat, 14 May 2016 11:16:11 -0500 Subject: Rename specs --- ...ntainer_registry_authentication_service_spec.rb | 192 +++++++++++++++++++++ ...ntainer_registry_authentication_service_spec.rb | 192 --------------------- 2 files changed, 192 insertions(+), 192 deletions(-) create mode 100644 spec/services/auth/container_registry_authentication_service_spec.rb delete mode 100644 spec/services/jwt/container_registry_authentication_service_spec.rb (limited to 'spec') diff --git a/spec/services/auth/container_registry_authentication_service_spec.rb b/spec/services/auth/container_registry_authentication_service_spec.rb new file mode 100644 index 00000000000..8dc47a24ee2 --- /dev/null +++ b/spec/services/auth/container_registry_authentication_service_spec.rb @@ -0,0 +1,192 @@ +require 'spec_helper' + +describe Auth::ContainerRegistryAuthenticationService, services: true do + let(:current_project) { nil } + let(:current_user) { nil } + let(:current_params) { {} } + let(:rsa_key) { OpenSSL::PKey::RSA.generate(512) } + let(:registry_settings) do + { + issuer: 'rspec', + key: nil + } + end + let(:payload) { JWT.decode(subject[:token], rsa_key).first } + + subject { described_class.new(current_project, current_user, current_params).execute } + + before do + allow(Gitlab.config.registry).to receive_messages(registry_settings) + allow_any_instance_of(JWT::RSAToken).to receive(:key).and_return(rsa_key) + end + + shared_examples 'an authenticated' do + it { is_expected.to include(:token) } + it { expect(payload).to include('access') } + end + + shared_examples 'a accessible' do + let(:access) do + [{ + 'type' => 'repository', + 'name' => project.path_with_namespace, + 'actions' => actions, + }] + end + + it_behaves_like 'an authenticated' + it { expect(payload).to include('access' => access) } + end + + shared_examples 'a pullable' do + it_behaves_like 'a accessible' do + let(:actions) { ['pull'] } + end + end + + shared_examples 'a pushable' do + it_behaves_like 'a accessible' do + let(:actions) { ['push'] } + end + end + + shared_examples 'a pullable and pushable' do + it_behaves_like 'a accessible' do + let(:actions) { ['pull', 'push'] } + end + end + + shared_examples 'a forbidden' do + it { is_expected.to include(http_status: 401) } + it { is_expected.to_not include(:token) } + end + + context 'user authorization' do + let(:project) { create(:project) } + let(:current_user) { create(:user) } + + context 'allow developer to push images' do + before { project.team << [current_user, :developer] } + + let(:current_params) do + { scope: "repository:#{project.path_with_namespace}:push" } + end + + it_behaves_like 'a pushable' + end + + context 'allow reporter to pull images' do + before { project.team << [current_user, :reporter] } + + let(:current_params) do + { scope: "repository:#{project.path_with_namespace}:pull" } + end + + it_behaves_like 'a pullable' + end + + context 'return a least of privileges' do + before { project.team << [current_user, :reporter] } + + let(:current_params) do + { scope: "repository:#{project.path_with_namespace}:push,pull" } + end + + it_behaves_like 'a pullable' + end + + context 'disallow guest to pull or push images' do + before { project.team << [current_user, :guest] } + + let(:current_params) do + { scope: "repository:#{project.path_with_namespace}:pull,push" } + end + + it_behaves_like 'a forbidden' + end + end + + context 'project authorization' do + let(:current_project) { create(:empty_project) } + + context 'allow to pull and push images' do + let(:current_params) do + { scope: "repository:#{current_project.path_with_namespace}:pull,push" } + end + + it_behaves_like 'a pullable and pushable' do + let(:project) { current_project } + end + end + + context 'for other projects' do + context 'when pulling' do + let(:current_params) do + { scope: "repository:#{project.path_with_namespace}:pull" } + end + + context 'allow for public' do + let(:project) { create(:empty_project, :public) } + it_behaves_like 'a pullable' + end + + context 'disallow for private' do + let(:project) { create(:empty_project, :private) } + it_behaves_like 'a forbidden' + end + end + + context 'when pushing' do + let(:current_params) do + { scope: "repository:#{project.path_with_namespace}:push" } + end + + context 'disallow for all' do + let(:project) { create(:empty_project, :public) } + it_behaves_like 'a forbidden' + end + end + + end + end + + context 'unauthorized' do + context 'for invalid scope' do + let(:current_params) do + { scope: 'invalid:aa:bb' } + end + + it_behaves_like 'a forbidden' + end + + context 'for private project' do + let(:project) { create(:empty_project, :private) } + + let(:current_params) do + { scope: "repository:#{project.path_with_namespace}:pull" } + end + + it_behaves_like 'a forbidden' + end + + context 'for public project' do + let(:project) { create(:empty_project, :public) } + + context 'when pulling and pushing' do + let(:current_params) do + { scope: "repository:#{project.path_with_namespace}:pull,push" } + end + + it_behaves_like 'a pullable' + end + + context 'when pushing' do + let(:current_params) do + { scope: "repository:#{project.path_with_namespace}:push" } + end + + it_behaves_like 'a forbidden' + end + end + end +end diff --git a/spec/services/jwt/container_registry_authentication_service_spec.rb b/spec/services/jwt/container_registry_authentication_service_spec.rb deleted file mode 100644 index 1873ea2639b..00000000000 --- a/spec/services/jwt/container_registry_authentication_service_spec.rb +++ /dev/null @@ -1,192 +0,0 @@ -require 'spec_helper' - -describe JWT::ContainerRegistryAuthenticationService, services: true do - let(:current_project) { nil } - let(:current_user) { nil } - let(:current_params) { {} } - let(:rsa_key) { OpenSSL::PKey::RSA.generate(512) } - let(:registry_settings) do - { - issuer: 'rspec', - key: nil - } - end - let(:payload) { JWT.decode(subject[:token], rsa_key).first } - - subject { described_class.new(current_project, current_user, current_params).execute } - - before do - allow(Gitlab.config.registry).to receive_messages(registry_settings) - allow_any_instance_of(JWT::RSAToken).to receive(:key).and_return(rsa_key) - end - - shared_examples 'an authenticated' do - it { is_expected.to include(:token) } - it { expect(payload).to include('access') } - end - - shared_examples 'a accessible' do - let(:access) do - [{ - 'type' => 'repository', - 'name' => project.path_with_namespace, - 'actions' => actions, - }] - end - - it_behaves_like 'an authenticated' - it { expect(payload).to include('access' => access) } - end - - shared_examples 'a pullable' do - it_behaves_like 'a accessible' do - let(:actions) { ['pull'] } - end - end - - shared_examples 'a pushable' do - it_behaves_like 'a accessible' do - let(:actions) { ['push'] } - end - end - - shared_examples 'a pullable and pushable' do - it_behaves_like 'a accessible' do - let(:actions) { ['pull', 'push'] } - end - end - - shared_examples 'a forbidden' do - it { is_expected.to include(http_status: 401) } - it { is_expected.to_not include(:token) } - end - - context 'user authorization' do - let(:project) { create(:project) } - let(:current_user) { create(:user) } - - context 'allow developer to push images' do - before { project.team << [current_user, :developer] } - - let(:current_params) do - { scope: "repository:#{project.path_with_namespace}:push" } - end - - it_behaves_like 'a pushable' - end - - context 'allow reporter to pull images' do - before { project.team << [current_user, :reporter] } - - let(:current_params) do - { scope: "repository:#{project.path_with_namespace}:pull" } - end - - it_behaves_like 'a pullable' - end - - context 'return a least of privileges' do - before { project.team << [current_user, :reporter] } - - let(:current_params) do - { scope: "repository:#{project.path_with_namespace}:push,pull" } - end - - it_behaves_like 'a pullable' - end - - context 'disallow guest to pull or push images' do - before { project.team << [current_user, :guest] } - - let(:current_params) do - { scope: "repository:#{project.path_with_namespace}:pull,push" } - end - - it_behaves_like 'a forbidden' - end - end - - context 'project authorization' do - let(:current_project) { create(:empty_project) } - - context 'allow to pull and push images' do - let(:current_params) do - { scope: "repository:#{current_project.path_with_namespace}:pull,push" } - end - - it_behaves_like 'a pullable and pushable' do - let(:project) { current_project } - end - end - - context 'for other projects' do - context 'when pulling' do - let(:current_params) do - { scope: "repository:#{project.path_with_namespace}:pull" } - end - - context 'allow for public' do - let(:project) { create(:empty_project, :public) } - it_behaves_like 'a pullable' - end - - context 'disallow for private' do - let(:project) { create(:empty_project, :private) } - it_behaves_like 'a forbidden' - end - end - - context 'when pushing' do - let(:current_params) do - { scope: "repository:#{project.path_with_namespace}:push" } - end - - context 'disallow for all' do - let(:project) { create(:empty_project, :public) } - it_behaves_like 'a forbidden' - end - end - - end - end - - context 'unauthorized' do - context 'for invalid scope' do - let(:current_params) do - { scope: 'invalid:aa:bb' } - end - - it_behaves_like 'a forbidden' - end - - context 'for private project' do - let(:project) { create(:empty_project, :private) } - - let(:current_params) do - { scope: "repository:#{project.path_with_namespace}:pull" } - end - - it_behaves_like 'a forbidden' - end - - context 'for public project' do - let(:project) { create(:empty_project, :public) } - - context 'when pulling and pushing' do - let(:current_params) do - { scope: "repository:#{project.path_with_namespace}:pull,push" } - end - - it_behaves_like 'a pullable' - end - - context 'when pushing' do - let(:current_params) do - { scope: "repository:#{project.path_with_namespace}:push" } - end - - it_behaves_like 'a forbidden' - end - end - end -end -- cgit v1.2.1 From 715a8cfa2f4639bf36b604f6e3eb2814187367c0 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Sat, 14 May 2016 14:22:45 -0500 Subject: Fix authentication service --- spec/services/auth/container_registry_authentication_service_spec.rb | 1 + 1 file changed, 1 insertion(+) (limited to 'spec') diff --git a/spec/services/auth/container_registry_authentication_service_spec.rb b/spec/services/auth/container_registry_authentication_service_spec.rb index 8dc47a24ee2..6e86a3dcf56 100644 --- a/spec/services/auth/container_registry_authentication_service_spec.rb +++ b/spec/services/auth/container_registry_authentication_service_spec.rb @@ -7,6 +7,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do let(:rsa_key) { OpenSSL::PKey::RSA.generate(512) } let(:registry_settings) do { + enabled: true, issuer: 'rspec', key: nil } -- cgit v1.2.1 From f4f9184a01bc7442411bbcffd9b6a86784fa5f53 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Sat, 14 May 2016 18:23:31 -0500 Subject: Rename JWT to JSONWebToken --- spec/lib/json_web_token/rsa_token_spec.rb | 31 ++++++++++++++++++++++ spec/lib/json_web_token/token_spec.rb | 18 +++++++++++++ spec/lib/jwt/rsa_token_spec.rb | 31 ---------------------- spec/lib/jwt/token_spec.rb | 18 ------------- ...ntainer_registry_authentication_service_spec.rb | 2 +- 5 files changed, 50 insertions(+), 50 deletions(-) create mode 100644 spec/lib/json_web_token/rsa_token_spec.rb create mode 100644 spec/lib/json_web_token/token_spec.rb delete mode 100644 spec/lib/jwt/rsa_token_spec.rb delete mode 100644 spec/lib/jwt/token_spec.rb (limited to 'spec') diff --git a/spec/lib/json_web_token/rsa_token_spec.rb b/spec/lib/json_web_token/rsa_token_spec.rb new file mode 100644 index 00000000000..4462cdde9a3 --- /dev/null +++ b/spec/lib/json_web_token/rsa_token_spec.rb @@ -0,0 +1,31 @@ +describe JSONWebToken::RSAToken do + let(:rsa_key) { generate_key } + let(:rsa_token) { described_class.new(nil) } + let(:rsa_encoded) { rsa_token.encoded } + + before { allow_any_instance_of(described_class).to receive(:key).and_return(rsa_key) } + + context 'token' do + context 'for valid key to be validated' do + before { rsa_token['key'] = 'value' } + + subject { JWT.decode(rsa_encoded, rsa_key) } + + it { expect{subject}.to_not raise_error } + it { expect(subject.first).to include('key' => 'value') } + end + + context 'for invalid key to raise an exception' do + let(:new_key) { generate_key } + subject { JWT.decode(rsa_encoded, new_key) } + + it { expect{subject}.to raise_error(JWT::DecodeError) } + end + end + + private + + def generate_key + OpenSSL::PKey::RSA.generate(512) + end +end diff --git a/spec/lib/json_web_token/token_spec.rb b/spec/lib/json_web_token/token_spec.rb new file mode 100644 index 00000000000..3d955e4d774 --- /dev/null +++ b/spec/lib/json_web_token/token_spec.rb @@ -0,0 +1,18 @@ +describe JSONWebToken::Token do + let(:token) { described_class.new } + + context 'custom parameters' do + let(:value) { 'value' } + before { token[:key] = value } + + it { expect(token[:key]).to eq(value) } + it { expect(token.payload).to include(key: value) } + end + + context 'embeds default payload' do + subject { token.payload } + let(:default) { token.send(:default_payload) } + + it { is_expected.to include(default) } + end +end diff --git a/spec/lib/jwt/rsa_token_spec.rb b/spec/lib/jwt/rsa_token_spec.rb deleted file mode 100644 index a5b1d3a67dc..00000000000 --- a/spec/lib/jwt/rsa_token_spec.rb +++ /dev/null @@ -1,31 +0,0 @@ -describe JWT::RSAToken do - let(:rsa_key) { generate_key } - let(:rsa_token) { described_class.new(nil) } - let(:rsa_encoded) { rsa_token.encoded } - - before { allow_any_instance_of(described_class).to receive(:key).and_return(rsa_key) } - - context 'token' do - context 'for valid key to be validated' do - before { rsa_token['key'] = 'value' } - - subject { JWT.decode(rsa_encoded, rsa_key) } - - it { expect{subject}.to_not raise_error } - it { expect(subject.first).to include('key' => 'value') } - end - - context 'for invalid key to raise an exception' do - let(:new_key) { generate_key } - subject { JWT.decode(rsa_encoded, new_key) } - - it { expect{subject}.to raise_error(JWT::DecodeError) } - end - end - - private - - def generate_key - OpenSSL::PKey::RSA.generate(512) - end -end diff --git a/spec/lib/jwt/token_spec.rb b/spec/lib/jwt/token_spec.rb deleted file mode 100644 index 92fdc3f1b7c..00000000000 --- a/spec/lib/jwt/token_spec.rb +++ /dev/null @@ -1,18 +0,0 @@ -describe JWT::Token do - let(:token) { described_class.new } - - context 'custom parameters' do - let(:value) { 'value' } - before { token[:key] = value } - - it { expect(token[:key]).to eq(value) } - it { expect(token.payload).to include(key: value) } - end - - context 'embeds default payload' do - subject { token.payload } - let(:default) { token.send(:default_payload) } - - it { is_expected.to include(default) } - end -end diff --git a/spec/services/auth/container_registry_authentication_service_spec.rb b/spec/services/auth/container_registry_authentication_service_spec.rb index 6e86a3dcf56..a2937368136 100644 --- a/spec/services/auth/container_registry_authentication_service_spec.rb +++ b/spec/services/auth/container_registry_authentication_service_spec.rb @@ -18,7 +18,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do before do allow(Gitlab.config.registry).to receive_messages(registry_settings) - allow_any_instance_of(JWT::RSAToken).to receive(:key).and_return(rsa_key) + allow_any_instance_of(JSONWebToken::RSAToken).to receive(:key).and_return(rsa_key) end shared_examples 'an authenticated' do -- cgit v1.2.1 From 0d93e93e794d179a4cbc5302aece27296e4abfe4 Mon Sep 17 00:00:00 2001 From: Takuya Noguchi Date: Sat, 14 May 2016 23:14:25 +0900 Subject: Add tests for unintentional filtering bug in MR (cf. !3872) --- .../merge_requests/user_lists_merge_requests_spec.rb | 15 +++++++++++++++ 1 file changed, 15 insertions(+) (limited to 'spec') diff --git a/spec/features/merge_requests/user_lists_merge_requests_spec.rb b/spec/features/merge_requests/user_lists_merge_requests_spec.rb index cc7f78e7325..2c7e1c748ad 100644 --- a/spec/features/merge_requests/user_lists_merge_requests_spec.rb +++ b/spec/features/merge_requests/user_lists_merge_requests_spec.rb @@ -38,6 +38,7 @@ describe 'Projects > Merge requests > User lists merge requests', feature: true expect(page).to have_content 'lfs' expect(page).not_to have_content 'fix' expect(page).not_to have_content 'markdown' + expect(count_merge_requests).to eq(1) end it 'filters on a specific assignee' do @@ -46,6 +47,7 @@ describe 'Projects > Merge requests > User lists merge requests', feature: true expect(page).not_to have_content 'lfs' expect(page).to have_content 'fix' expect(page).to have_content 'markdown' + expect(count_merge_requests).to eq(2) end it 'sorts by newest' do @@ -53,6 +55,7 @@ describe 'Projects > Merge requests > User lists merge requests', feature: true expect(first_merge_request).to include('lfs') expect(last_merge_request).to include('fix') + expect(count_merge_requests).to eq(3) end it 'sorts by oldest' do @@ -60,30 +63,35 @@ describe 'Projects > Merge requests > User lists merge requests', feature: true expect(first_merge_request).to include('fix') expect(last_merge_request).to include('lfs') + expect(count_merge_requests).to eq(3) end it 'sorts by last updated' do visit_merge_requests(project, sort: sort_value_recently_updated) expect(first_merge_request).to include('lfs') + expect(count_merge_requests).to eq(3) end it 'sorts by oldest updated' do visit_merge_requests(project, sort: sort_value_oldest_updated) expect(first_merge_request).to include('markdown') + expect(count_merge_requests).to eq(3) end it 'sorts by milestone due soon' do visit_merge_requests(project, sort: sort_value_milestone_soon) expect(first_merge_request).to include('fix') + expect(count_merge_requests).to eq(3) end it 'sorts by milestone due later' do visit_merge_requests(project, sort: sort_value_milestone_later) expect(first_merge_request).to include('markdown') + expect(count_merge_requests).to eq(3) end it 'filters on one label and sorts by due soon' do @@ -94,6 +102,7 @@ describe 'Projects > Merge requests > User lists merge requests', feature: true sort: sort_value_due_date_soon) expect(first_merge_request).to include('fix') + expect(count_merge_requests).to eq(1) end context 'while filtering on two labels' do @@ -110,6 +119,7 @@ describe 'Projects > Merge requests > User lists merge requests', feature: true sort: sort_value_due_date_soon) expect(first_merge_request).to include('fix') + expect(count_merge_requests).to eq(1) end context 'filter on assignee and' do @@ -119,6 +129,7 @@ describe 'Projects > Merge requests > User lists merge requests', feature: true sort: sort_value_due_date_soon) expect(first_merge_request).to include('fix') + expect(count_merge_requests).to eq(1) end end end @@ -134,4 +145,8 @@ describe 'Projects > Merge requests > User lists merge requests', feature: true def last_merge_request page.all('ul.mr-list > li').last.text end + + def count_merge_requests + page.all('ul.mr-list > li').count + end end -- cgit v1.2.1 From e8f7e5516bf0449f0d7f435ae720aa35f40e1dd0 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Sat, 14 May 2016 19:45:48 -0500 Subject: Added specs for JwtController --- spec/requests/jwt_controller_spec.rb | 87 ++++++++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) create mode 100644 spec/requests/jwt_controller_spec.rb (limited to 'spec') diff --git a/spec/requests/jwt_controller_spec.rb b/spec/requests/jwt_controller_spec.rb new file mode 100644 index 00000000000..782d710e1b4 --- /dev/null +++ b/spec/requests/jwt_controller_spec.rb @@ -0,0 +1,87 @@ +require 'spec_helper' + +describe JwtController do + let(:services) { { 'test' => TestService } } + let(:parameters) { { service: 'test' } } + let(:ok_status) { { status: 'OK' } } + + before { allow_any_instance_of(JwtController).to receive(:SERVICES).and_return services } + + context 'existing service' do + before { expect_any_instance_of(TestService).to receive(:execute).and_return(ok_status) } + + subject! { get '/jwt/auth', parameters } + + it { expect(response.status).to eq(200) } + end + + context 'when using authorized request' do + context 'using CI token' do + let(:project) { create(:empty_project, runners_token: 'token', builds_enabled: builds_enabled) } + let(:headers) { { HTTP_AUTHENTICATION: authorize('gitlab-ci-token', project.runners_token) } } + + context 'project with enabled CI' do + let(:builds_enabled) { true } + + it do + expect(TestService).to receive(:new).with(project, nil, parameters).and_call_original + + get '/jwt/auth', parameters, headers + end + end + + context 'project with disabled CI' do + let(:builds_enabled) { false } + + it do + expect(TestService).to receive(:new).with(project, nil, parameters).and_call_original + + get '/jwt/auth', parameters, headers + end + end + end + + context 'using User login' do + let(:user) { create(:user) } + let(:headers) { { HTTP_AUTHENTICATION: authorize('user', 'password') } } + + before { expect_any_instance_of(Gitlab::Auth).to receive(:find).with('user', 'password').and_return(user) } + + it do + expect(TestService).to receive(:new).with(nil, user, parameters).and_call_original + + get '/jwt/auth', parameters, headers + end + end + + context 'using invalid login' do + let(:headers) { { HTTP_AUTHENTICATION: authorize('invalid', 'password') } } + + subject! { get '/jwt/auth', parameters, headers } + + it { expect(response.status).to eq(403) } + end + end + + context 'unknown service' do + subject! { get '/jwt/auth', service: 'unknown' } + + it { expect(response.status).to eq(404) } + end + + def authorize(login, password) + ActionController::HttpAuthentication::Basic.encode_credentials(login, password) + end + + class TestService + attr_accessor :project, :current_user, :params + + def initialize(project, user, params = {}) + @project, @current_user, @params = project, user, params.dup + end + + def execute + { status: 'OK' } + end + end +end -- cgit v1.2.1 From 7b88dca77eeb2a93b5a343d27af513ea28222379 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Sun, 15 May 2016 00:33:06 -0500 Subject: Update JwtController specs --- spec/requests/jwt_controller_spec.rb | 57 +++++++++++++----------------------- 1 file changed, 21 insertions(+), 36 deletions(-) (limited to 'spec') diff --git a/spec/requests/jwt_controller_spec.rb b/spec/requests/jwt_controller_spec.rb index 782d710e1b4..7bb71365a48 100644 --- a/spec/requests/jwt_controller_spec.rb +++ b/spec/requests/jwt_controller_spec.rb @@ -1,61 +1,58 @@ require 'spec_helper' describe JwtController do - let(:services) { { 'test' => TestService } } - let(:parameters) { { service: 'test' } } - let(:ok_status) { { status: 'OK' } } + let(:service) { double(execute: {}) } + let(:service_class) { double(new: service) } + let(:service_name) { 'test' } + let(:parameters) { { service: service_name } } - before { allow_any_instance_of(JwtController).to receive(:SERVICES).and_return services } + before { stub_const('JwtController::SERVICES', service_name => service_class) } context 'existing service' do - before { expect_any_instance_of(TestService).to receive(:execute).and_return(ok_status) } - subject! { get '/jwt/auth', parameters } it { expect(response.status).to eq(200) } + + context 'returning custom http code' do + let(:service) { double(execute: { http_status: 505 }) } + + it { expect(response.status).to eq(505) } + end end context 'when using authorized request' do context 'using CI token' do let(:project) { create(:empty_project, runners_token: 'token', builds_enabled: builds_enabled) } - let(:headers) { { HTTP_AUTHENTICATION: authorize('gitlab-ci-token', project.runners_token) } } + let(:headers) { { authorization: credentials('gitlab_ci_token', project.runners_token) } } + + subject! { get '/jwt/auth', parameters, headers } context 'project with enabled CI' do let(:builds_enabled) { true } - it do - expect(TestService).to receive(:new).with(project, nil, parameters).and_call_original - - get '/jwt/auth', parameters, headers - end + it { expect(service_class).to have_received(:new).with(project, nil, parameters) } end context 'project with disabled CI' do let(:builds_enabled) { false } - it do - expect(TestService).to receive(:new).with(project, nil, parameters).and_call_original - - get '/jwt/auth', parameters, headers - end + it { expect(response.status).to eq(403) } end end context 'using User login' do let(:user) { create(:user) } - let(:headers) { { HTTP_AUTHENTICATION: authorize('user', 'password') } } + let(:headers) { { authorization: credentials('user', 'password') } } before { expect_any_instance_of(Gitlab::Auth).to receive(:find).with('user', 'password').and_return(user) } - it do - expect(TestService).to receive(:new).with(nil, user, parameters).and_call_original + subject! { get '/jwt/auth', parameters, headers } - get '/jwt/auth', parameters, headers - end + it { expect(service_class).to have_received(:new).with(nil, user, parameters) } end context 'using invalid login' do - let(:headers) { { HTTP_AUTHENTICATION: authorize('invalid', 'password') } } + let(:headers) { { authorization: credentials('invalid', 'password') } } subject! { get '/jwt/auth', parameters, headers } @@ -69,19 +66,7 @@ describe JwtController do it { expect(response.status).to eq(404) } end - def authorize(login, password) + def credentials(login, password) ActionController::HttpAuthentication::Basic.encode_credentials(login, password) end - - class TestService - attr_accessor :project, :current_user, :params - - def initialize(project, user, params = {}) - @project, @current_user, @params = project, user, params.dup - end - - def execute - { status: 'OK' } - end - end end -- cgit v1.2.1 From dfd0e2450aabc3b5c322c4a4382edb84caa7101b Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Sun, 15 May 2016 08:52:26 -0500 Subject: Improve authentication service specs --- spec/lib/json_web_token/rsa_token_spec.rb | 28 ++++++++++---- ...ntainer_registry_authentication_service_spec.rb | 44 ++++++++++++++++++---- 2 files changed, 56 insertions(+), 16 deletions(-) (limited to 'spec') diff --git a/spec/lib/json_web_token/rsa_token_spec.rb b/spec/lib/json_web_token/rsa_token_spec.rb index 4462cdde9a3..0c3d3ea7019 100644 --- a/spec/lib/json_web_token/rsa_token_spec.rb +++ b/spec/lib/json_web_token/rsa_token_spec.rb @@ -1,5 +1,17 @@ describe JSONWebToken::RSAToken do - let(:rsa_key) { generate_key } + let(:rsa_key) do + OpenSSL::PKey::RSA.new <<-eos.strip_heredoc + -----BEGIN RSA PRIVATE KEY----- + MIIBOgIBAAJBAMA5sXIBE0HwgIB40iNidN4PGWzOyLQK0bsdOBNgpEXkDlZBvnak + OUgAPF+rME4PB0Yl415DabUI40T5UNmlwxcCAwEAAQJAZtY2pSwIFm3JAXIh0cZZ + iXcAfiJ+YzuqinUOS+eW2sBCAEzjcARlU/o6sFQgtsOi4FOMczAd1Yx8UDMXMmrw + 2QIhAPBgVhJiTF09pdmeFWutCvTJDlFFAQNbrbo2X2x/9WF9AiEAzLgqMKeStSRu + H9N16TuDrUoO8R+DPqriCwkKrSHaWyMCIFzMhE4inuKcSywBaLmiG4m3GQzs++Al + A6PRG/PSTpQtAiBxtBg6zdf+JC3GH3zt/dA0/10tL4OF2wORfYQghRzyYQIhAL2l + 0ZQW+yLIZAGrdBFWYEAa52GZosncmzBNlsoTgwE4 + -----END RSA PRIVATE KEY----- + eos + end let(:rsa_token) { described_class.new(nil) } let(:rsa_encoded) { rsa_token.encoded } @@ -13,19 +25,19 @@ describe JSONWebToken::RSAToken do it { expect{subject}.to_not raise_error } it { expect(subject.first).to include('key' => 'value') } + it do + expect(subject.second).to eq( + "typ" => "JWT", + "alg" => "RS256", + "kid" => "OGXY:4TR7:FAVO:WEM2:XXEW:E4FP:TKL7:7ACK:TZAF:D54P:SUIA:P3B2") + end end context 'for invalid key to raise an exception' do - let(:new_key) { generate_key } + let(:new_key) { OpenSSL::PKey::RSA.generate(512) } subject { JWT.decode(rsa_encoded, new_key) } it { expect{subject}.to raise_error(JWT::DecodeError) } end end - - private - - def generate_key - OpenSSL::PKey::RSA.generate(512) - end end diff --git a/spec/services/auth/container_registry_authentication_service_spec.rb b/spec/services/auth/container_registry_authentication_service_spec.rb index a2937368136..4a6cd132e8d 100644 --- a/spec/services/auth/container_registry_authentication_service_spec.rb +++ b/spec/services/auth/container_registry_authentication_service_spec.rb @@ -57,15 +57,28 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do end end - shared_examples 'a forbidden' do + shared_examples 'a unauthorized' do it { is_expected.to include(http_status: 401) } it { is_expected.to_not include(:token) } end + shared_examples 'a forbidden' do + it { is_expected.to include(http_status: 403) } + it { is_expected.to_not include(:token) } + end + context 'user authorization' do let(:project) { create(:project) } let(:current_user) { create(:user) } + context 'allow to use offline_token' do + let(:current_params) do + { offline_token: true } + end + + it_behaves_like 'an authenticated' + end + context 'allow developer to push images' do before { project.team << [current_user, :developer] } @@ -103,13 +116,21 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do { scope: "repository:#{project.path_with_namespace}:pull,push" } end - it_behaves_like 'a forbidden' + it_behaves_like 'a unauthorized' end end context 'project authorization' do let(:current_project) { create(:empty_project) } + context 'disallow to use offline_token' do + let(:current_params) do + { offline_token: true } + end + + it_behaves_like 'a forbidden' + end + context 'allow to pull and push images' do let(:current_params) do { scope: "repository:#{current_project.path_with_namespace}:pull,push" } @@ -133,7 +154,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do context 'disallow for private' do let(:project) { create(:empty_project, :private) } - it_behaves_like 'a forbidden' + it_behaves_like 'a unauthorized' end end @@ -144,20 +165,27 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do context 'disallow for all' do let(:project) { create(:empty_project, :public) } - it_behaves_like 'a forbidden' + it_behaves_like 'a unauthorized' end end - end end context 'unauthorized' do + context 'disallow to use offline_token' do + let(:current_params) do + { offline_token: true } + end + + it_behaves_like 'a forbidden' + end + context 'for invalid scope' do let(:current_params) do { scope: 'invalid:aa:bb' } end - it_behaves_like 'a forbidden' + it_behaves_like 'a unauthorized' end context 'for private project' do @@ -167,7 +195,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do { scope: "repository:#{project.path_with_namespace}:pull" } end - it_behaves_like 'a forbidden' + it_behaves_like 'a unauthorized' end context 'for public project' do @@ -186,7 +214,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do { scope: "repository:#{project.path_with_namespace}:push" } end - it_behaves_like 'a forbidden' + it_behaves_like 'a unauthorized' end end end -- cgit v1.2.1 From 28ed9907047dd07089833d5b7bb8cd800e0ddff6 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Sun, 15 May 2016 10:46:54 -0500 Subject: Fix http status codes for container registry authentication service --- .../container_registry_authentication_service_spec.rb | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) (limited to 'spec') diff --git a/spec/services/auth/container_registry_authentication_service_spec.rb b/spec/services/auth/container_registry_authentication_service_spec.rb index 4a6cd132e8d..3ea252ed44f 100644 --- a/spec/services/auth/container_registry_authentication_service_spec.rb +++ b/spec/services/auth/container_registry_authentication_service_spec.rb @@ -57,11 +57,6 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do end end - shared_examples 'a unauthorized' do - it { is_expected.to include(http_status: 401) } - it { is_expected.to_not include(:token) } - end - shared_examples 'a forbidden' do it { is_expected.to include(http_status: 403) } it { is_expected.to_not include(:token) } @@ -116,7 +111,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do { scope: "repository:#{project.path_with_namespace}:pull,push" } end - it_behaves_like 'a unauthorized' + it_behaves_like 'a forbidden' end end @@ -154,7 +149,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do context 'disallow for private' do let(:project) { create(:empty_project, :private) } - it_behaves_like 'a unauthorized' + it_behaves_like 'a forbidden' end end @@ -165,7 +160,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do context 'disallow for all' do let(:project) { create(:empty_project, :public) } - it_behaves_like 'a unauthorized' + it_behaves_like 'a forbidden' end end end @@ -185,7 +180,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do { scope: 'invalid:aa:bb' } end - it_behaves_like 'a unauthorized' + it_behaves_like 'a forbidden' end context 'for private project' do @@ -195,7 +190,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do { scope: "repository:#{project.path_with_namespace}:pull" } end - it_behaves_like 'a unauthorized' + it_behaves_like 'a forbidden' end context 'for public project' do @@ -214,7 +209,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do { scope: "repository:#{project.path_with_namespace}:push" } end - it_behaves_like 'a unauthorized' + it_behaves_like 'a forbidden' end end end -- cgit v1.2.1 From b9306c2e82fec5b74416ccdd5481dfed3f8fbf51 Mon Sep 17 00:00:00 2001 From: Pablo Carranza Date: Sun, 15 May 2016 19:47:41 +0100 Subject: Add cache count metrics to rails cache --- spec/lib/gitlab/metrics/subscribers/rails_cache_spec.rb | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) (limited to 'spec') diff --git a/spec/lib/gitlab/metrics/subscribers/rails_cache_spec.rb b/spec/lib/gitlab/metrics/subscribers/rails_cache_spec.rb index e01b0b4bd21..d824dc54438 100644 --- a/spec/lib/gitlab/metrics/subscribers/rails_cache_spec.rb +++ b/spec/lib/gitlab/metrics/subscribers/rails_cache_spec.rb @@ -9,7 +9,7 @@ describe Gitlab::Metrics::Subscribers::RailsCache do describe '#cache_read' do it 'increments the cache_read duration' do expect(subscriber).to receive(:increment). - with(:cache_read_duration, event.duration) + with(:cache_read, event.duration) subscriber.cache_read(event) end @@ -18,7 +18,7 @@ describe Gitlab::Metrics::Subscribers::RailsCache do describe '#cache_write' do it 'increments the cache_write duration' do expect(subscriber).to receive(:increment). - with(:cache_write_duration, event.duration) + with(:cache_write, event.duration) subscriber.cache_write(event) end @@ -27,7 +27,7 @@ describe Gitlab::Metrics::Subscribers::RailsCache do describe '#cache_delete' do it 'increments the cache_delete duration' do expect(subscriber).to receive(:increment). - with(:cache_delete_duration, event.duration) + with(:cache_delete, event.duration) subscriber.cache_delete(event) end @@ -36,7 +36,7 @@ describe Gitlab::Metrics::Subscribers::RailsCache do describe '#cache_exist?' do it 'increments the cache_exists duration' do expect(subscriber).to receive(:increment). - with(:cache_exists_duration, event.duration) + with(:cache_exists, event.duration) subscriber.cache_exist?(event) end @@ -61,10 +61,16 @@ describe Gitlab::Metrics::Subscribers::RailsCache do expect(transaction).to receive(:increment). with(:cache_duration, event.duration) + expect(transaction).to receive(:increment). + with(:cache_count, 1) + expect(transaction).to receive(:increment). with(:cache_delete_duration, event.duration) - subscriber.increment(:cache_delete_duration, event.duration) + expect(transaction).to receive(:increment). + with(:cache_delete_count, 1) + + subscriber.increment(:cache_delete, event.duration) end end end -- cgit v1.2.1 From bec350528cdc81e26476780f1ca3db8171b3ece8 Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Tue, 3 May 2016 12:42:55 +0100 Subject: Force password change after admin reset When an admin changes a user's password for them, force the user to reset the password after logging in by expiring the new password immediately. --- spec/controllers/admin/users_controller_spec.rb | 76 +++++++++++++++++++++++++ spec/features/admin/admin_users_spec.rb | 3 + 2 files changed, 79 insertions(+) (limited to 'spec') diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb index ce2a62ae1fd..6caf37ddc2c 100644 --- a/spec/controllers/admin/users_controller_spec.rb +++ b/spec/controllers/admin/users_controller_spec.rb @@ -114,6 +114,82 @@ describe Admin::UsersController do end end + describe 'POST update' do + context 'when the password has changed' do + def update_password(user, password, password_confirmation = nil) + params = { + id: user.to_param, + user: { + password: password, + password_confirmation: password_confirmation || password + } + } + + post :update, params + end + + context 'when the new password is valid' do + it 'redirects to the user' do + update_password(user, 'AValidPassword1') + + expect(response).to redirect_to(admin_user_path(user)) + end + + it 'updates the password' do + update_password(user, 'AValidPassword1') + + expect { user.reload }.to change { user.encrypted_password } + end + + it 'sets the new password to expire immediately' do + update_password(user, 'AValidPassword1') + + expect { user.reload }.to change { user.password_expires_at }.to(a_value <= Time.now) + end + end + + context 'when the new password is invalid' do + it 'shows the edit page again' do + update_password(user, 'invalid') + + expect(response).to render_template(:edit) + end + + it 'returns the error message' do + update_password(user, 'invalid') + + expect(assigns[:user].errors).to contain_exactly(a_string_matching(/too short/)) + end + + it 'does not update the password' do + update_password(user, 'invalid') + + expect { user.reload }.not_to change { user.encrypted_password } + end + end + + context 'when the new password does not match the password confirmation' do + it 'shows the edit page again' do + update_password(user, 'AValidPassword1', 'AValidPassword2') + + expect(response).to render_template(:edit) + end + + it 'returns the error message' do + update_password(user, 'AValidPassword1', 'AValidPassword2') + + expect(assigns[:user].errors).to contain_exactly(a_string_matching(/doesn't match/)) + end + + it 'does not update the password' do + update_password(user, 'AValidPassword1', 'AValidPassword2') + + expect { user.reload }.not_to change { user.encrypted_password } + end + end + end + end + describe "POST impersonate" do context "when the user is blocked" do before do diff --git a/spec/features/admin/admin_users_spec.rb b/spec/features/admin/admin_users_spec.rb index 4570e409128..6dee0cd8d47 100644 --- a/spec/features/admin/admin_users_spec.rb +++ b/spec/features/admin/admin_users_spec.rb @@ -210,6 +210,8 @@ describe "Admin::Users", feature: true do before do fill_in "user_name", with: "Big Bang" fill_in "user_email", with: "bigbang@mail.com" + fill_in "user_password", with: "AValidPassword1" + fill_in "user_password_confirmation", with: "AValidPassword1" check "user_admin" click_button "Save changes" end @@ -223,6 +225,7 @@ describe "Admin::Users", feature: true do @simple_user.reload expect(@simple_user.name).to eq('Big Bang') expect(@simple_user.is_admin?).to be_truthy + expect(@simple_user.password_expires_at).to be <= Time.now end end end -- cgit v1.2.1 From 91480e5e7f2fb5e732839958b53c521bf7206939 Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Thu, 12 May 2016 12:20:09 +0100 Subject: Tidy up IssuesFinder specs - Don't do setup in spec bodies. - Don't `describe` a symbol. - Don't use 'should'. --- spec/finders/issues_finder_spec.rb | 148 ++++++++++++++++++++----------------- 1 file changed, 80 insertions(+), 68 deletions(-) (limited to 'spec') diff --git a/spec/finders/issues_finder_spec.rb b/spec/finders/issues_finder_spec.rb index bc607a29751..f905703dd6b 100644 --- a/spec/finders/issues_finder_spec.rb +++ b/spec/finders/issues_finder_spec.rb @@ -1,10 +1,10 @@ require 'spec_helper' describe IssuesFinder do - let(:user) { create :user } - let(:user2) { create :user } - let(:project1) { create(:project) } - let(:project2) { create(:project) } + let(:user) { create(:user) } + let(:user2) { create(:user) } + let(:project1) { create(:empty_project) } + let(:project2) { create(:empty_project) } let(:milestone) { create(:milestone, project: project1) } let(:label) { create(:label, project: project2) } let(:issue1) { create(:issue, author: user, assignee: user, project: project1, milestone: milestone) } @@ -16,101 +16,113 @@ describe IssuesFinder do project1.team << [user, :master] project2.team << [user, :developer] project2.team << [user2, :developer] + + issue1 + issue2 + issue3 end - describe :execute do - before :each do - issue1 - issue2 - issue3 - end + describe '#execute' do + let(:search_user) { user } + let(:params) { {} } + let(:issues) { IssuesFinder.new(search_user, params.merge(scope: scope, state: 'opened')).execute } context 'scope: all' do - it 'should filter by all' do - params = { scope: "all", state: 'opened' } - issues = IssuesFinder.new(user, params).execute - expect(issues.size).to eq(3) - end + let(:scope) { 'all' } - it 'should filter by assignee id' do - params = { scope: "all", assignee_id: user.id, state: 'opened' } - issues = IssuesFinder.new(user, params).execute - expect(issues.size).to eq(2) + it 'returns all issues' do + expect(issues).to contain_exactly(issue1, issue2, issue3) end - it 'should filter by author id' do - params = { scope: "all", author_id: user2.id, state: 'opened' } - issues = IssuesFinder.new(user, params).execute - expect(issues).to eq([issue3]) + context 'filtering by assignee ID' do + let(:params) { { assignee_id: user.id } } + + it 'returns issues assigned to that user' do + expect(issues).to contain_exactly(issue1, issue2) + end end - it 'should filter by milestone id' do - params = { scope: "all", milestone_title: milestone.title, state: 'opened' } - issues = IssuesFinder.new(user, params).execute - expect(issues).to eq([issue1]) + context 'filtering by author ID' do + let(:params) { { author_id: user2.id } } + + it 'returns issues created by that user' do + expect(issues).to contain_exactly(issue3) + end end - it 'should filter by no milestone id' do - params = { scope: "all", milestone_title: Milestone::None.title, state: 'opened' } - issues = IssuesFinder.new(user, params).execute - expect(issues).to match_array([issue2, issue3]) + context 'filtering by milestone' do + let(:params) { { milestone_title: milestone.title } } + + it 'returns issues assigned to that milestone' do + expect(issues).to contain_exactly(issue1) + end end - it 'should filter by label name' do - params = { scope: "all", label_name: label.title, state: 'opened' } - issues = IssuesFinder.new(user, params).execute - expect(issues).to eq([issue2]) + context 'filtering by no milestone' do + let(:params) { { milestone_title: Milestone::None.title } } + + it 'returns issues with no milestone' do + expect(issues).to contain_exactly(issue2, issue3) + end end - it 'returns unique issues when filtering by multiple labels' do - label2 = create(:label, project: project2) + context 'filtering by label' do + let(:params) { { label_name: label.title } } - create(:label_link, label: label2, target: issue2) + it 'returns issues with that label' do + expect(issues).to contain_exactly(issue2) + end + end - params = { - scope: 'all', - label_name: [label.title, label2.title].join(','), - state: 'opened' - } + context 'filtering by multiple labels' do + let(:params) { { label_name: [label.title, label2.title].join(',') } } + let(:label2) { create(:label, project: project2) } - issues = IssuesFinder.new(user, params).execute + before { create(:label_link, label: label2, target: issue2) } - expect(issues).to eq([issue2]) + it 'returns the unique issues with any of those labels' do + expect(issues).to contain_exactly(issue2) + end end - it 'should filter by no label name' do - params = { scope: "all", label_name: Label::None.title, state: 'opened' } - issues = IssuesFinder.new(user, params).execute - expect(issues).to match_array([issue1, issue3]) + context 'filtering by no label' do + let(:params) { { label_name: Label::None.title } } + + it 'returns issues with no labels' do + expect(issues).to contain_exactly(issue1, issue3) + end end - it 'should be empty for unauthorized user' do - params = { scope: "all", state: 'opened' } - issues = IssuesFinder.new(nil, params).execute - expect(issues.size).to be_zero + context 'when the user is unauthorized' do + let(:search_user) { nil } + + it 'returns no results' do + expect(issues).to be_empty + end end - it 'should not include unauthorized issues' do - params = { scope: "all", state: 'opened' } - issues = IssuesFinder.new(user2, params).execute - expect(issues.size).to eq(2) - expect(issues).not_to include(issue1) - expect(issues).to include(issue2) - expect(issues).to include(issue3) + context 'when the user can see some, but not all, issues' do + let(:search_user) { user2 } + + it 'returns only issues they can see' do + expect(issues).to contain_exactly(issue2, issue3) + end end end context 'personal scope' do - it 'should filter by assignee' do - params = { scope: "assigned-to-me", state: 'opened' } - issues = IssuesFinder.new(user, params).execute - expect(issues.size).to eq(2) + let(:scope) { 'assigned-to-me' } + + it 'returns issue assigned to the user' do + expect(issues).to contain_exactly(issue1, issue2) end - it 'should filter by project' do - params = { scope: "assigned-to-me", state: 'opened', project_id: project1.id } - issues = IssuesFinder.new(user, params).execute - expect(issues.size).to eq(1) + context 'filtering by project' do + let(:params) { { project_id: project1.id } } + + it 'returns issues assigned to the user in that project' do + expect(issues).to contain_exactly(issue1) + end end end end -- cgit v1.2.1 From 750b2ff0eec67926e737a40c7975cce2b58e27f7 Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Wed, 11 May 2016 17:38:34 +0100 Subject: Make upcoming milestone work across projects Before: we took the next milestone due across all projects in the search and found issues whose milestone title matched that one. Problems: 1. The milestone could be closed. 2. Different projects have milestones with different schedules. 3. Different projects have milestones with different titles. 4. Different projects can have milestones with different schedules, but the _same_ title. That means we could show issues from a past milestone, or one that's far in the future. After: gather the ID of the next milestone on each project we're looking at, and find issues with those milestone IDs. Problems: 1. For a lot of projects, this can return a lot of IDs. 2. The SQL query has to be different between Postgres and MySQL, because MySQL is much more lenient with HAVING: as well as the columns appearing in GROUP BY or in aggregate clauses, MySQL allows them to appear in the SELECT list (un-aggregated). --- spec/finders/issues_finder_spec.rb | 34 ++++++++++++++++++++++++++++++++++ spec/models/milestone_spec.rb | 31 +++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+) (limited to 'spec') diff --git a/spec/finders/issues_finder_spec.rb b/spec/finders/issues_finder_spec.rb index f905703dd6b..ec8809e6926 100644 --- a/spec/finders/issues_finder_spec.rb +++ b/spec/finders/issues_finder_spec.rb @@ -66,6 +66,40 @@ describe IssuesFinder do end end + context 'filtering by upcoming milestone' do + let(:params) { { milestone_title: Milestone::Upcoming.name } } + + let(:project_no_upcoming_milestones) { create(:empty_project, :public) } + let(:project_next_1_1) { create(:empty_project, :public) } + let(:project_next_8_8) { create(:empty_project, :public) } + + let(:yesterday) { Date.today - 1.day } + let(:tomorrow) { Date.today + 1.day } + let(:two_days_from_now) { Date.today + 2.days } + let(:ten_days_from_now) { Date.today + 10.days } + + let(:milestones) do + [ + create(:milestone, :closed, project: project_no_upcoming_milestones), + create(:milestone, project: project_next_1_1, title: '1.1', due_date: two_days_from_now), + create(:milestone, project: project_next_1_1, title: '8.8', due_date: ten_days_from_now), + create(:milestone, project: project_next_8_8, title: '1.1', due_date: yesterday), + create(:milestone, project: project_next_8_8, title: '8.8', due_date: tomorrow) + ] + end + + before do + milestones.each do |milestone| + create(:issue, project: milestone.project, milestone: milestone, author: user, assignee: user) + end + end + + it 'returns issues in the upcoming milestone for each project' do + expect(issues.map { |issue| issue.milestone.title }).to contain_exactly('1.1', '8.8') + expect(issues.map { |issue| issue.milestone.due_date }).to contain_exactly(tomorrow, two_days_from_now) + end + end + context 'filtering by label' do let(:params) { { label_name: label.title } } diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb index 247a9fa9910..210c5f7eb4f 100644 --- a/spec/models/milestone_spec.rb +++ b/spec/models/milestone_spec.rb @@ -204,4 +204,35 @@ describe Milestone, models: true do to eq([milestone]) end end + + describe '.upcoming_ids_by_projects' do + let(:project_1) { create(:empty_project) } + let(:project_2) { create(:empty_project) } + let(:project_3) { create(:empty_project) } + let(:projects) { [project_1, project_2, project_3] } + + let!(:past_milestone_project_1) { create(:milestone, project: project_1, due_date: Time.now - 1.day) } + let!(:current_milestone_project_1) { create(:milestone, project: project_1, due_date: Time.now + 1.day) } + let!(:future_milestone_project_1) { create(:milestone, project: project_1, due_date: Time.now + 2.days) } + + let!(:past_milestone_project_2) { create(:milestone, project: project_2, due_date: Time.now - 1.day) } + let!(:closed_milestone_project_2) { create(:milestone, :closed, project: project_2, due_date: Time.now + 1.day) } + let!(:current_milestone_project_2) { create(:milestone, project: project_2, due_date: Time.now + 2.days) } + + let!(:past_milestone_project_3) { create(:milestone, project: project_3, due_date: Time.now - 1.day) } + + let(:milestone_ids) { Milestone.upcoming_ids_by_projects(projects) } + + it 'returns the next upcoming open milestone ID for each project' do + expect(milestone_ids).to contain_exactly(current_milestone_project_1.id, current_milestone_project_2.id) + end + + context 'when the projects have no open upcoming milestones' do + let(:projects) { [project_3] } + + it 'returns no results' do + expect(milestone_ids).to be_empty + end + end + end end -- cgit v1.2.1 From e8058bd23100949607ac8c353f482067c0ecd25a Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Mon, 16 May 2016 10:23:21 +0100 Subject: Return a relation with Postgres Postgres only needs to select a single column, so that can used as a sub-query where `Milestone.upcoming_ids_by_projects` is actually used in `IssuableFinder`. MySQL needs to select the `due_date` column because it's used in the `HAVING` clause, so it has to return an array of IDs. --- spec/models/milestone_spec.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'spec') diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb index 210c5f7eb4f..1e18c788b50 100644 --- a/spec/models/milestone_spec.rb +++ b/spec/models/milestone_spec.rb @@ -221,7 +221,9 @@ describe Milestone, models: true do let!(:past_milestone_project_3) { create(:milestone, project: project_3, due_date: Time.now - 1.day) } - let(:milestone_ids) { Milestone.upcoming_ids_by_projects(projects) } + # The call to `#try` is because this returns a relation with a Postgres DB, + # and an array of IDs with a MySQL DB. + let(:milestone_ids) { Milestone.upcoming_ids_by_projects(projects).map { |id| id.try(:id) || id } } it 'returns the next upcoming open milestone ID for each project' do expect(milestone_ids).to contain_exactly(current_milestone_project_1.id, current_milestone_project_2.id) -- cgit v1.2.1 From 71ca2de7aabf3191c4f486ca15a78a5b7f6abd94 Mon Sep 17 00:00:00 2001 From: Felipe Artur Date: Thu, 21 Apr 2016 15:55:54 -0300 Subject: Toggle email signup confirmation in admin settings --- spec/controllers/registrations_controller_spec.rb | 33 +++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 spec/controllers/registrations_controller_spec.rb (limited to 'spec') diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb new file mode 100644 index 00000000000..b4ab767f73e --- /dev/null +++ b/spec/controllers/registrations_controller_spec.rb @@ -0,0 +1,33 @@ +require 'spec_helper' + +describe RegistrationsController do + describe '#create' do + around(:each) do |example| + perform_enqueued_jobs do + example.run + end + end + + let(:user_params) { { "user"=> {"name"=>"new_user", "username"=>"new_username", "email"=>"new@user.com", "password"=>"Any_password"} } } + + context 'when skipping email confirmation' do + before { allow(current_application_settings).to receive(:skip_user_confirmation_email).and_return(true) } + + it 'logs user in directly' do + post(:create, user_params) + expect(ActionMailer::Base.deliveries.last).to be_nil + expect(subject.current_user).to be + end + end + + context 'when not skipping email confirmation' do + before { allow(current_application_settings).to receive(:skip_user_confirmation_email).and_return(false) } + + it 'does not authenticate user and sends confirmation email' do + post(:create, user_params) + expect(ActionMailer::Base.deliveries.last.to.first).to eq(user_params["user"]["email"]) + expect(subject.current_user).to be_nil + end + end + end +end -- cgit v1.2.1 From c5526a2d9a946d99d7b4a72fc488fe6e0a9ad60b Mon Sep 17 00:00:00 2001 From: Felipe Artur Date: Thu, 28 Apr 2016 17:09:15 -0300 Subject: Change skip_user_confirmation_email to send_user_confirmation_email --- spec/controllers/registrations_controller_spec.rb | 12 ++++++------ spec/features/signup_spec.rb | 2 ++ spec/models/user_spec.rb | 1 + 3 files changed, 9 insertions(+), 6 deletions(-) (limited to 'spec') diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb index b4ab767f73e..29f1847d9a1 100644 --- a/spec/controllers/registrations_controller_spec.rb +++ b/spec/controllers/registrations_controller_spec.rb @@ -8,10 +8,10 @@ describe RegistrationsController do end end - let(:user_params) { { "user"=> {"name"=>"new_user", "username"=>"new_username", "email"=>"new@user.com", "password"=>"Any_password"} } } + let(:user_params) { { user: { name: "new_user", username: "new_username", email: "new@user.com", password: "Any_password" } } } - context 'when skipping email confirmation' do - before { allow(current_application_settings).to receive(:skip_user_confirmation_email).and_return(true) } + context 'when sending email confirmation' do + before { allow(current_application_settings).to receive(:send_user_confirmation_email).and_return(false) } it 'logs user in directly' do post(:create, user_params) @@ -20,12 +20,12 @@ describe RegistrationsController do end end - context 'when not skipping email confirmation' do - before { allow(current_application_settings).to receive(:skip_user_confirmation_email).and_return(false) } + context 'when not sending email confirmation' do + before { allow(current_application_settings).to receive(:send_user_confirmation_email).and_return(true) } it 'does not authenticate user and sends confirmation email' do post(:create, user_params) - expect(ActionMailer::Base.deliveries.last.to.first).to eq(user_params["user"]["email"]) + expect(ActionMailer::Base.deliveries.last.to.first).to eq(user_params[:user][:email]) expect(subject.current_user).to be_nil end end diff --git a/spec/features/signup_spec.rb b/spec/features/signup_spec.rb index 58aabd913eb..c7840f26d8f 100644 --- a/spec/features/signup_spec.rb +++ b/spec/features/signup_spec.rb @@ -2,6 +2,8 @@ require 'spec_helper' feature 'Signup', feature: true do describe 'signup with no errors' do + before { allow_any_instance_of(ApplicationSetting).to receive(:send_user_confirmation_email).and_return(true) } + it 'creates the user account and sends a confirmation email' do user = build(:user) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 10e7e693571..9581990666b 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -141,6 +141,7 @@ describe User, models: true do end describe '#confirm' do + before { allow(current_application_settings).to receive(:send_user_confirmation_email).and_return(true) } let(:user) { create(:user, confirmed_at: nil, unconfirmed_email: 'test@gitlab.com') } it 'returns unconfirmed' do -- cgit v1.2.1 From 7bb84e64979edda8e76f077bd58aeb35857aec23 Mon Sep 17 00:00:00 2001 From: Felipe Artur Date: Fri, 6 May 2016 17:59:45 -0300 Subject: Change landing page when skipping confirmation email and add documentation --- spec/controllers/registrations_controller_spec.rb | 2 +- spec/features/signup_spec.rb | 45 +++++++++++++++++------ 2 files changed, 35 insertions(+), 12 deletions(-) (limited to 'spec') diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb index 29f1847d9a1..df70a589a89 100644 --- a/spec/controllers/registrations_controller_spec.rb +++ b/spec/controllers/registrations_controller_spec.rb @@ -16,7 +16,7 @@ describe RegistrationsController do it 'logs user in directly' do post(:create, user_params) expect(ActionMailer::Base.deliveries.last).to be_nil - expect(subject.current_user).to be + expect(subject.current_user).to_not be_nil end end diff --git a/spec/features/signup_spec.rb b/spec/features/signup_spec.rb index c7840f26d8f..4229e82b443 100644 --- a/spec/features/signup_spec.rb +++ b/spec/features/signup_spec.rb @@ -2,22 +2,45 @@ require 'spec_helper' feature 'Signup', feature: true do describe 'signup with no errors' do - before { allow_any_instance_of(ApplicationSetting).to receive(:send_user_confirmation_email).and_return(true) } - it 'creates the user account and sends a confirmation email' do - user = build(:user) + context "when sending confirmation email" do + before { allow_any_instance_of(ApplicationSetting).to receive(:send_user_confirmation_email).and_return(true) } - visit root_path + it 'creates the user account and sends a confirmation email' do + user = build(:user) - fill_in 'new_user_name', with: user.name - fill_in 'new_user_username', with: user.username - fill_in 'new_user_email', with: user.email - fill_in 'new_user_password', with: user.password - click_button "Sign up" + visit root_path + + fill_in 'new_user_name', with: user.name + fill_in 'new_user_username', with: user.username + fill_in 'new_user_email', with: user.email + fill_in 'new_user_password', with: user.password + click_button "Sign up" - expect(current_path).to eq users_almost_there_path - expect(page).to have_content("Please check your email to confirm your account") + expect(current_path).to eq users_almost_there_path + expect(page).to have_content("Please check your email to confirm your account") + end end + + context "when not sending confirmation email" do + before { allow_any_instance_of(ApplicationSetting).to receive(:send_user_confirmation_email).and_return(false) } + + it 'creates the user account and goes to dashboard' do + user = build(:user) + + visit root_path + + fill_in 'new_user_name', with: user.name + fill_in 'new_user_username', with: user.username + fill_in 'new_user_email', with: user.email + fill_in 'new_user_password', with: user.password + click_button "Sign up" + + expect(current_path).to eq dashboard_projects_path + expect(page).to have_content("Welcome! You have signed up successfully.") + end + end + end describe 'signup with errors' do -- cgit v1.2.1 From c9be74e24797c1dab5b443728349bb0c5ce969c3 Mon Sep 17 00:00:00 2001 From: Felipe Artur Date: Mon, 16 May 2016 16:43:19 -0300 Subject: Fix single note api request --- spec/requests/api/notes_spec.rb | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) (limited to 'spec') diff --git a/spec/requests/api/notes_spec.rb b/spec/requests/api/notes_spec.rb index f9bfee94424..ed1ed5aeb95 100644 --- a/spec/requests/api/notes_spec.rb +++ b/spec/requests/api/notes_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe API::API, api: true do include ApiHelpers let(:user) { create(:user) } - let!(:project) { create(:project, namespace: user.namespace) } + let!(:project) { create(:project, :public, namespace: user.namespace) } let!(:issue) { create(:issue, project: project, author: user) } let!(:merge_request) { create(:merge_request, source_project: project, target_project: project, author: user) } let!(:snippet) { create(:project_snippet, project: project, author: user) } @@ -51,7 +51,7 @@ describe API::API, api: true do expect(response.status).to eq(404) end - context "that references a private issue" do + context "and current user cannot view the notes" do it "should return an empty array" do get api("/projects/#{ext_proj.id}/issues/#{ext_issue.id}/notes", user) @@ -142,13 +142,24 @@ describe API::API, api: true do expect(response.status).to eq(404) end - context "that references a private issue" do + context "and current user cannot view the note" do it "should return a 404 error" do get api("/projects/#{ext_proj.id}/issues/#{ext_issue.id}/notes/#{cross_reference_note.id}", user) expect(response.status).to eq(404) end + context "when issue is confidential" do + before { issue.update_attributes(confidential: true) } + + it "returns 404" do + get api("/projects/#{project.id}/issues/#{issue.id}/notes/#{issue_note.id}", private_user) + + expect(response.status).to eq(404) + end + end + + context "and current user can view the note" do it "should return an issue note by id" do get api("/projects/#{ext_proj.id}/issues/#{ext_issue.id}/notes/#{cross_reference_note.id}", private_user) -- cgit v1.2.1 From 80817644a74e6ead62c2c66d4bd2826ecf634393 Mon Sep 17 00:00:00 2001 From: Jeroen van Baarsen Date: Wed, 16 Sep 2015 13:19:46 +0200 Subject: Improve issue formatting in Slack service Signed-off-by: Jeroen van Baarsen --- .../project_services/slack_service/issue_message_spec.rb | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) (limited to 'spec') diff --git a/spec/models/project_services/slack_service/issue_message_spec.rb b/spec/models/project_services/slack_service/issue_message_spec.rb index f648cbe2dee..0f8889bdf3c 100644 --- a/spec/models/project_services/slack_service/issue_message_spec.rb +++ b/spec/models/project_services/slack_service/issue_message_spec.rb @@ -25,7 +25,7 @@ describe SlackService::IssueMessage, models: true do } end - let(:color) { '#345' } + let(:color) { '#C95823' } context '#initialize' do before do @@ -40,10 +40,11 @@ describe SlackService::IssueMessage, models: true do context 'open' do it 'returns a message regarding opening of issues' do expect(subject.pretext).to eq( - 'Test User opened in : '\ - '*Issue title*') + '] Issue opened by Test User') expect(subject.attachments).to eq([ { + title: "#100 Issue title", + title_link: "url", text: "issue description", color: color, } @@ -56,10 +57,10 @@ describe SlackService::IssueMessage, models: true do args[:object_attributes][:action] = 'close' args[:object_attributes][:state] = 'closed' end + it 'returns a message regarding closing of issues' do expect(subject.pretext). to eq( - 'Test User closed in : '\ - '*Issue title*') + '] Issue closed by Test User') expect(subject.attachments).to be_empty end end -- cgit v1.2.1