From 852b9c28dd80af4a49336b0ff921a1a863dbc929 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Wed, 15 Aug 2012 23:16:33 -0400 Subject: [PATCH 1/5] Move observer specs out of spec/models into spec/observers --- spec/{models => observers}/activity_observer_spec.rb | 0 spec/{models => observers}/issue_observer_spec.rb | 0 spec/{models => observers}/user_observer_spec.rb | 0 3 files changed, 0 insertions(+), 0 deletions(-) rename spec/{models => observers}/activity_observer_spec.rb (100%) rename spec/{models => observers}/issue_observer_spec.rb (100%) rename spec/{models => observers}/user_observer_spec.rb (100%) diff --git a/spec/models/activity_observer_spec.rb b/spec/observers/activity_observer_spec.rb similarity index 100% rename from spec/models/activity_observer_spec.rb rename to spec/observers/activity_observer_spec.rb diff --git a/spec/models/issue_observer_spec.rb b/spec/observers/issue_observer_spec.rb similarity index 100% rename from spec/models/issue_observer_spec.rb rename to spec/observers/issue_observer_spec.rb diff --git a/spec/models/user_observer_spec.rb b/spec/observers/user_observer_spec.rb similarity index 100% rename from spec/models/user_observer_spec.rb rename to spec/observers/user_observer_spec.rb From fba174e9bc4e4ef5c0c4d6a4282f37e5265b87e2 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Thu, 23 Aug 2012 05:19:40 -0400 Subject: [PATCH 2/5] Cleanup spec/support folder and spec/spec_helper Changes: * Move spec/monkeypatch to spec/support * Remove unused support/shared_examples * Move support/api to support/api_helpers to match module name * Move support/login to support/login_helpers to match module name * Move API specs to requests/api (convention over configuration) * Remove unused support/js_patch * Simplify login_as helper * Move DatabaseCleaner stuff to its own support file * Remove unnecessary configuration and requires from spec_helper --- .../project/projects_steps.rb | 2 +- features/support/env.rb | 4 +- spec/{ => requests}/api/issues_spec.rb | 0 spec/{ => requests}/api/projects_spec.rb | 0 spec/{ => requests}/api/users_spec.rb | 0 spec/spec_helper.rb | 37 +++---------------- spec/support/api.rb | 7 ---- spec/support/api_helpers.rb | 9 +++++ spec/support/db_cleaner.rb | 18 +++++++++ spec/support/js_patch.rb | 6 --- spec/support/login.rb | 30 --------------- spec/support/login_helpers.rb | 23 ++++++++++++ spec/{ => support}/monkeypatch.rb | 0 spec/support/shared_examples.rb | 16 -------- 14 files changed, 59 insertions(+), 93 deletions(-) rename spec/{ => requests}/api/issues_spec.rb (100%) rename spec/{ => requests}/api/projects_spec.rb (100%) rename spec/{ => requests}/api/users_spec.rb (100%) delete mode 100644 spec/support/api.rb create mode 100644 spec/support/api_helpers.rb create mode 100644 spec/support/db_cleaner.rb delete mode 100644 spec/support/js_patch.rb delete mode 100644 spec/support/login.rb create mode 100644 spec/support/login_helpers.rb rename spec/{ => support}/monkeypatch.rb (100%) delete mode 100644 spec/support/shared_examples.rb diff --git a/features/step_definitions/project/projects_steps.rb b/features/step_definitions/project/projects_steps.rb index 3ff08d58..d981e1f3 100644 --- a/features/step_definitions/project/projects_steps.rb +++ b/features/step_definitions/project/projects_steps.rb @@ -1,4 +1,4 @@ -include LoginMacros +include LoginHelpers Given /^I signin as a user$/ do login_as :user diff --git a/features/support/env.rb b/features/support/env.rb index b69a5fe7..80a465b4 100644 --- a/features/support/env.rb +++ b/features/support/env.rb @@ -7,9 +7,9 @@ require 'cucumber/rails' require 'webmock/cucumber' WebMock.allow_net_connect! -require Rails.root.join 'spec/monkeypatch' require Rails.root.join 'spec/factories' -require Rails.root.join 'spec/support/login' +require Rails.root.join 'spec/support/monkeypatch' +require Rails.root.join 'spec/support/login_helpers' require Rails.root.join 'spec/support/valid_commit' Capybara.default_selector = :css diff --git a/spec/api/issues_spec.rb b/spec/requests/api/issues_spec.rb similarity index 100% rename from spec/api/issues_spec.rb rename to spec/requests/api/issues_spec.rb diff --git a/spec/api/projects_spec.rb b/spec/requests/api/projects_spec.rb similarity index 100% rename from spec/api/projects_spec.rb rename to spec/requests/api/projects_spec.rb diff --git a/spec/api/users_spec.rb b/spec/requests/api/users_spec.rb similarity index 100% rename from spec/api/users_spec.rb rename to spec/requests/api/users_spec.rb diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index cd931475..d3635ed1 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -9,10 +9,8 @@ require File.expand_path("../../config/environment", __FILE__) require 'rspec/rails' require 'capybara/rails' require 'capybara/rspec' -require 'capybara/dsl' require 'webmock/rspec' require 'factories' -require 'monkeypatch' require 'email_spec' require 'headless' @@ -23,10 +21,13 @@ Dir[Rails.root.join("spec/support/**/*.rb")].each {|f| require f} # Use capybara-webkit Capybara.javascript_driver = :webkit +WebMock.disable_net_connect!(allow_localhost: true) + RSpec.configure do |config| config.mock_with :rspec - config.include LoginMacros + config.include LoginHelpers, type: :request + config.include ApiHelpers, type: :request # If you're not using ActiveRecord, or you'd prefer not to run each of your # examples within a transaction, remove the following line or assign false @@ -38,35 +39,9 @@ RSpec.configure do |config| headless.start end - config.before :each, type: :integration do - DeviseSessionMock.disable - end - config.before do - if example.metadata[:js] - DatabaseCleaner.strategy = :truncation - Capybara::Selenium::Driver::DEFAULT_OPTIONS[:resynchronize] = true - else - DatabaseCleaner.strategy = :transaction - end - - DatabaseCleaner.start - - WebMock.disable_net_connect!(allow_localhost: true) - # !!! Observers disabled by default in tests - # - # Use next code to enable observers - # before(:each) { ActiveRecord::Base.observers.enable(:all) } - # - ActiveRecord::Base.observers.disable :all + ActiveRecord::Base.observers.disable(:all) + # ActiveRecord::Base.observers.enable(:all) end - - config.after do - DatabaseCleaner.clean - end - - config.include RSpec::Rails::RequestExampleGroup, type: :request, example_group: { - file_path: /spec\/api/ - } end diff --git a/spec/support/api.rb b/spec/support/api.rb deleted file mode 100644 index d363d8b9..00000000 --- a/spec/support/api.rb +++ /dev/null @@ -1,7 +0,0 @@ -def api_prefix - "/api/#{Gitlab::API::VERSION}" -end - -def json_response - JSON.parse(response.body) -end diff --git a/spec/support/api_helpers.rb b/spec/support/api_helpers.rb new file mode 100644 index 00000000..e35d5735 --- /dev/null +++ b/spec/support/api_helpers.rb @@ -0,0 +1,9 @@ +module ApiHelpers + def api_prefix + "/api/#{Gitlab::API::VERSION}" + end + + def json_response + JSON.parse(response.body) + end +end diff --git a/spec/support/db_cleaner.rb b/spec/support/db_cleaner.rb new file mode 100644 index 00000000..f1e072aa --- /dev/null +++ b/spec/support/db_cleaner.rb @@ -0,0 +1,18 @@ +require 'database_cleaner' + +RSpec.configure do |config| + config.before do + if example.metadata[:js] + DatabaseCleaner.strategy = :truncation + Capybara::Selenium::Driver::DEFAULT_OPTIONS[:resynchronize] = true + else + DatabaseCleaner.strategy = :transaction + end + + DatabaseCleaner.start + end + + config.after do + DatabaseCleaner.clean + end +end diff --git a/spec/support/js_patch.rb b/spec/support/js_patch.rb deleted file mode 100644 index 0d4ab264..00000000 --- a/spec/support/js_patch.rb +++ /dev/null @@ -1,6 +0,0 @@ -module JsPatch - def confirm_js_popup - page.evaluate_script("window.alert = function(msg) { return true; }") - page.evaluate_script("window.confirm = function(msg) { return true; }") - end -end diff --git a/spec/support/login.rb b/spec/support/login.rb deleted file mode 100644 index 78a907ba..00000000 --- a/spec/support/login.rb +++ /dev/null @@ -1,30 +0,0 @@ -module LoginMacros - def login_as role - @user = User.create(email: "user#{User.count}@mail.com", - name: "John Smith", - password: "123456", - password_confirmation: "123456", - skype: 'user_skype') - - if role == :admin - @user.admin = true - @user.save! - end - - visit new_user_session_path - fill_in "user_email", with: @user.email - fill_in "user_password", with: "123456" - click_button "Sign in" - end - - def login_with(user) - visit new_user_session_path - fill_in "user_email", with: user.email - fill_in "user_password", with: "123456" - click_button "Sign in" - end - - def logout - click_link "Logout" rescue nil - end -end diff --git a/spec/support/login_helpers.rb b/spec/support/login_helpers.rb new file mode 100644 index 00000000..769034e2 --- /dev/null +++ b/spec/support/login_helpers.rb @@ -0,0 +1,23 @@ +module LoginHelpers + # Internal: Create and log in as a user of the specified role + # + # role - User role (e.g., :admin, :user) + def login_as(role) + @user = Factory(role) + login_with(@user) + end + + # Internal: Login as the specified user + # + # user - User instance to login with + def login_with(user) + visit new_user_session_path + fill_in "user_email", with: user.email + fill_in "user_password", with: "123456" + click_button "Sign in" + end + + def logout + click_link "Logout" rescue nil + end +end diff --git a/spec/monkeypatch.rb b/spec/support/monkeypatch.rb similarity index 100% rename from spec/monkeypatch.rb rename to spec/support/monkeypatch.rb diff --git a/spec/support/shared_examples.rb b/spec/support/shared_examples.rb deleted file mode 100644 index 9fd207d0..00000000 --- a/spec/support/shared_examples.rb +++ /dev/null @@ -1,16 +0,0 @@ -shared_examples_for :project_side_pane do - subject { page } - it { should have_content((@project || project).name) } - it { should have_content("Commits") } - it { should have_content("Files") } -end - -shared_examples_for :tree_view do - subject { page } - - it "should have Tree View of project" do - should have_content("app") - should have_content("History") - should have_content("Gemfile") - end -end From b2a5344a2d68922d5c6cb8de228fb9b41ce3efc4 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Sat, 25 Aug 2012 13:31:50 -0400 Subject: [PATCH 3/5] Add a simple `api` method to ApiHelpers, replacing api_prefix See docs for usage --- spec/requests/api/issues_spec.rb | 2 ++ spec/requests/api/projects_spec.rb | 2 ++ spec/requests/api/users_spec.rb | 2 ++ spec/spec_helper.rb | 1 - spec/support/api_helpers.rb | 29 +++++++++++++++++++++++++++-- 5 files changed, 33 insertions(+), 3 deletions(-) diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb index f6d8e379..4701ca2f 100644 --- a/spec/requests/api/issues_spec.rb +++ b/spec/requests/api/issues_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe Gitlab::API do + include ApiHelpers + let(:user) { Factory :user } let!(:project) { Factory :project, owner: user } let!(:issue) { Factory :issue, author: user, assignee: user, project: project } diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index ff45619e..a721ab3a 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe Gitlab::API do + include ApiHelpers + let(:user) { Factory :user } let!(:project) { Factory :project, owner: user } let!(:snippet) { Factory :snippet, author: user, project: project, title: 'example' } diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 32b9379d..fcfea131 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe Gitlab::API do + include ApiHelpers + let(:user) { Factory :user } describe "GET /users" do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index d3635ed1..9fb0ad7e 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -27,7 +27,6 @@ RSpec.configure do |config| config.mock_with :rspec config.include LoginHelpers, type: :request - config.include ApiHelpers, type: :request # If you're not using ActiveRecord, or you'd prefer not to run each of your # examples within a transaction, remove the following line or assign false diff --git a/spec/support/api_helpers.rb b/spec/support/api_helpers.rb index e35d5735..7d901197 100644 --- a/spec/support/api_helpers.rb +++ b/spec/support/api_helpers.rb @@ -1,6 +1,31 @@ module ApiHelpers - def api_prefix - "/api/#{Gitlab::API::VERSION}" + # Public: Prepend a request path with the path to the API + # + # path - Path to append + # user - User object - If provided, automatically appends private_token query + # string for authenticated requests + # + # Examples + # + # >> api('/issues') + # => "/api/v2/issues" + # + # >> api('/issues', User.last) + # => "/api/v2/issues?private_token=..." + # + # >> api('/issues?foo=bar', User.last) + # => "/api/v2/issues?foo=bar&private_token=..." + # + # Returns the relative path to the requested API resource + def api(path, user = nil) + "/api/#{Gitlab::API::VERSION}#{path}" + + + # Normalize query string + (path.index('?') ? '' : '?') + + + # Append private_token if given a User object + (user.respond_to?(:private_token) ? + "&private_token=#{user.private_token}" : "") end def json_response From 232389f4e8ebaffd125985ce63439cea407e85d5 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Sat, 25 Aug 2012 13:43:55 -0400 Subject: [PATCH 4/5] Clean up request specs --- spec/requests/admin/security_spec.rb | 24 +- spec/requests/api/issues_spec.rb | 14 +- spec/requests/api/projects_spec.rb | 35 ++- spec/requests/api/users_spec.rb | 8 +- spec/requests/security/profile_access_spec.rb | 30 ++- spec/requests/security/project_access_spec.rb | 230 ++++++++++-------- 6 files changed, 191 insertions(+), 150 deletions(-) diff --git a/spec/requests/admin/security_spec.rb b/spec/requests/admin/security_spec.rb index 0c369740..63068326 100644 --- a/spec/requests/admin/security_spec.rb +++ b/spec/requests/admin/security_spec.rb @@ -2,20 +2,26 @@ require 'spec_helper' describe "Admin::Projects" do describe "GET /admin/projects" do - it { admin_projects_path.should be_allowed_for :admin } - it { admin_projects_path.should be_denied_for :user } - it { admin_projects_path.should be_denied_for :visitor } + subject { admin_projects_path } + + it { should be_allowed_for :admin } + it { should be_denied_for :user } + it { should be_denied_for :visitor } end describe "GET /admin/users" do - it { admin_users_path.should be_allowed_for :admin } - it { admin_users_path.should be_denied_for :user } - it { admin_users_path.should be_denied_for :visitor } + subject { admin_users_path } + + it { should be_allowed_for :admin } + it { should be_denied_for :user } + it { should be_denied_for :visitor } end describe "GET /admin/hooks" do - it { admin_hooks_path.should be_allowed_for :admin } - it { admin_hooks_path.should be_denied_for :user } - it { admin_hooks_path.should be_denied_for :visitor } + subject { admin_hooks_path } + + it { should be_allowed_for :admin } + it { should be_denied_for :user } + it { should be_denied_for :visitor } end end diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb index 4701ca2f..c00a056d 100644 --- a/spec/requests/api/issues_spec.rb +++ b/spec/requests/api/issues_spec.rb @@ -10,13 +10,13 @@ describe Gitlab::API do describe "GET /issues" do it "should return authentication error" do - get "#{api_prefix}/issues" + get api("/issues") response.status.should == 401 end describe "authenticated GET /issues" do it "should return an array of issues" do - get "#{api_prefix}/issues?private_token=#{user.private_token}" + get api("/issues", user) response.status.should == 200 json_response.should be_an Array json_response.first['title'].should == issue.title @@ -26,7 +26,7 @@ describe Gitlab::API do describe "GET /projects/:id/issues" do it "should return project issues" do - get "#{api_prefix}/projects/#{project.code}/issues?private_token=#{user.private_token}" + get api("/projects/#{project.code}/issues", user) response.status.should == 200 json_response.should be_an Array json_response.first['title'].should == issue.title @@ -35,7 +35,7 @@ describe Gitlab::API do describe "GET /projects/:id/issues/:issue_id" do it "should return a project issue by id" do - get "#{api_prefix}/projects/#{project.code}/issues/#{issue.id}?private_token=#{user.private_token}" + get api("/projects/#{project.code}/issues/#{issue.id}", user) response.status.should == 200 json_response['title'].should == issue.title end @@ -43,7 +43,7 @@ describe Gitlab::API do describe "POST /projects/:id/issues" do it "should create a new project issue" do - post "#{api_prefix}/projects/#{project.code}/issues?private_token=#{user.private_token}", + post api("/projects/#{project.code}/issues", user), title: 'new issue', labels: 'label, label2' response.status.should == 201 json_response['title'].should == 'new issue' @@ -54,7 +54,7 @@ describe Gitlab::API do describe "PUT /projects/:id/issues/:issue_id" do it "should update a project issue" do - put "#{api_prefix}/projects/#{project.code}/issues/#{issue.id}?private_token=#{user.private_token}", + put api("/projects/#{project.code}/issues/#{issue.id}", user), title: 'updated title', labels: 'label2', closed: 1 response.status.should == 200 json_response['title'].should == 'updated title' @@ -66,7 +66,7 @@ describe Gitlab::API do describe "DELETE /projects/:id/issues/:issue_id" do it "should delete a project issue" do expect { - delete "#{api_prefix}/projects/#{project.code}/issues/#{issue.id}?private_token=#{user.private_token}" + delete api("/projects/#{project.code}/issues/#{issue.id}", user) }.to change { Issue.count }.by(-1) end end diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index a721ab3a..0cbc12af 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -10,13 +10,13 @@ describe Gitlab::API do describe "GET /projects" do it "should return authentication error" do - get "#{api_prefix}/projects" + get api("/projects") response.status.should == 401 end describe "authenticated GET /projects" do it "should return an array of projects" do - get "#{api_prefix}/projects?private_token=#{user.private_token}" + get api("/projects", user) response.status.should == 200 json_response.should be_an Array json_response.first['name'].should == project.name @@ -27,20 +27,20 @@ describe Gitlab::API do describe "GET /projects/:id" do it "should return a project by id" do - get "#{api_prefix}/projects/#{project.id}?private_token=#{user.private_token}" + get api("/projects/#{project.id}", user) response.status.should == 200 json_response['name'].should == project.name json_response['owner']['email'].should == user.email end it "should return a project by code name" do - get "#{api_prefix}/projects/#{project.code}?private_token=#{user.private_token}" + get api("/projects/#{project.code}", user) response.status.should == 200 json_response['name'].should == project.name end it "should return a 404 error if not found" do - get "#{api_prefix}/projects/42?private_token=#{user.private_token}" + get api("/projects/42", user) response.status.should == 404 json_response['message'].should == '404 Not found' end @@ -48,7 +48,7 @@ describe Gitlab::API do describe "GET /projects/:id/repository/branches" do it "should return an array of project branches" do - get "#{api_prefix}/projects/#{project.code}/repository/branches?private_token=#{user.private_token}" + get api("/projects/#{project.code}/repository/branches", user) response.status.should == 200 json_response.should be_an Array json_response.first['name'].should == project.repo.heads.sort_by(&:name).first.name @@ -57,7 +57,7 @@ describe Gitlab::API do describe "GET /projects/:id/repository/branches/:branch" do it "should return the branch information for a single branch" do - get "#{api_prefix}/projects/#{project.code}/repository/branches/new_design?private_token=#{user.private_token}" + get api("/projects/#{project.code}/repository/branches/new_design", user) response.status.should == 200 json_response['name'].should == 'new_design' @@ -67,7 +67,7 @@ describe Gitlab::API do describe "GET /projects/:id/repository/tags" do it "should return an array of project tags" do - get "#{api_prefix}/projects/#{project.code}/repository/tags?private_token=#{user.private_token}" + get api("/projects/#{project.code}/repository/tags", user) response.status.should == 200 json_response.should be_an Array json_response.first['name'].should == project.repo.tags.sort_by(&:name).reverse.first.name @@ -76,7 +76,7 @@ describe Gitlab::API do describe "GET /projects/:id/snippets/:snippet_id" do it "should return a project snippet" do - get "#{api_prefix}/projects/#{project.code}/snippets/#{snippet.id}?private_token=#{user.private_token}" + get api("/projects/#{project.code}/snippets/#{snippet.id}", user) response.status.should == 200 json_response['title'].should == snippet.title end @@ -84,7 +84,7 @@ describe Gitlab::API do describe "POST /projects/:id/snippets" do it "should create a new project snippet" do - post "#{api_prefix}/projects/#{project.code}/snippets?private_token=#{user.private_token}", + post api("/projects/#{project.code}/snippets", user), title: 'api test', file_name: 'sample.rb', code: 'test' response.status.should == 201 json_response['title'].should == 'api test' @@ -93,7 +93,7 @@ describe Gitlab::API do describe "PUT /projects/:id/snippets" do it "should update an existing project snippet" do - put "#{api_prefix}/projects/#{project.code}/snippets/#{snippet.id}?private_token=#{user.private_token}", + put api("/projects/#{project.code}/snippets/#{snippet.id}", user), code: 'updated code' response.status.should == 200 json_response['title'].should == 'example' @@ -104,34 +104,31 @@ describe Gitlab::API do describe "DELETE /projects/:id/snippets/:snippet_id" do it "should delete existing project snippet" do expect { - delete "#{api_prefix}/projects/#{project.code}/snippets/#{snippet.id}?private_token=#{user.private_token}" + delete api("/projects/#{project.code}/snippets/#{snippet.id}", user) }.to change { Snippet.count }.by(-1) end end describe "GET /projects/:id/snippets/:snippet_id/raw" do it "should get a raw project snippet" do - get "#{api_prefix}/projects/#{project.code}/snippets/#{snippet.id}/raw?private_token=#{user.private_token}" + get api("/projects/#{project.code}/snippets/#{snippet.id}/raw", user) response.status.should == 200 end end describe "GET /projects/:id/:sha/blob" do it "should get the raw file contents" do - get "#{api_prefix}/projects/#{project.code}/repository/commits/master/blob?filepath=README.md&private_token=#{user.private_token}" - + get api("/projects/#{project.code}/repository/commits/master/blob?filepath=README.md", user) response.status.should == 200 end it "should return 404 for invalid branch_name" do - get "#{api_prefix}/projects/#{project.code}/repository/commits/invalid_branch_name/blob?filepath=README.md&private_token=#{user.private_token}" - + get api("/projects/#{project.code}/repository/commits/invalid_branch_name/blob?filepath=README.md", user) response.status.should == 404 end it "should return 404 for invalid file" do - get "#{api_prefix}/projects/#{project.code}/repository/commits/master/blob?filepath=README.invalid&private_token=#{user.private_token}" - + get api("/projects/#{project.code}/repository/commits/master/blob?filepath=README.invalid", user) response.status.should == 404 end end diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index fcfea131..d791962a 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -7,13 +7,13 @@ describe Gitlab::API do describe "GET /users" do it "should return authentication error" do - get "#{api_prefix}/users" + get api("/users") response.status.should == 401 end describe "authenticated GET /users" do it "should return an array of users" do - get "#{api_prefix}/users?private_token=#{user.private_token}" + get api("/users", user) response.status.should == 200 json_response.should be_an Array json_response.first['email'].should == user.email @@ -23,7 +23,7 @@ describe Gitlab::API do describe "GET /users/:id" do it "should return a user by id" do - get "#{api_prefix}/users/#{user.id}?private_token=#{user.private_token}" + get api("/users/#{user.id}", user) response.status.should == 200 json_response['email'].should == user.email end @@ -31,7 +31,7 @@ describe Gitlab::API do describe "GET /user" do it "should return current user" do - get "#{api_prefix}/user?private_token=#{user.private_token}" + get api("/user", user) response.status.should == 200 json_response['email'].should == user.email end diff --git a/spec/requests/security/profile_access_spec.rb b/spec/requests/security/profile_access_spec.rb index b8ed27f0..9f6fe6a2 100644 --- a/spec/requests/security/profile_access_spec.rb +++ b/spec/requests/security/profile_access_spec.rb @@ -11,24 +11,30 @@ describe "Users Security" do end describe "GET /keys" do - it { keys_path.should be_allowed_for @u1 } - it { keys_path.should be_allowed_for :admin } - it { keys_path.should be_allowed_for :user } - it { keys_path.should be_denied_for :visitor } + subject { keys_path } + + it { should be_allowed_for @u1 } + it { should be_allowed_for :admin } + it { should be_allowed_for :user } + it { should be_denied_for :visitor } end describe "GET /profile" do - it { profile_path.should be_allowed_for @u1 } - it { profile_path.should be_allowed_for :admin } - it { profile_path.should be_allowed_for :user } - it { profile_path.should be_denied_for :visitor } + subject { profile_path } + + it { should be_allowed_for @u1 } + it { should be_allowed_for :admin } + it { should be_allowed_for :user } + it { should be_denied_for :visitor } end describe "GET /profile/password" do - it { profile_password_path.should be_allowed_for @u1 } - it { profile_password_path.should be_allowed_for :admin } - it { profile_password_path.should be_allowed_for :user } - it { profile_password_path.should be_denied_for :visitor } + subject { profile_password_path } + + it { should be_allowed_for @u1 } + it { should be_allowed_for :admin } + it { should be_allowed_for :user } + it { should be_denied_for :visitor } end end end diff --git a/spec/requests/security/project_access_spec.rb b/spec/requests/security/project_access_spec.rb index d503cf85..0cdf43bf 100644 --- a/spec/requests/security/project_access_spec.rb +++ b/spec/requests/security/project_access_spec.rb @@ -26,64 +26,76 @@ describe "Application access" do end describe "GET /project_code" do - it { project_path(@project).should be_allowed_for @u1 } - it { project_path(@project).should be_allowed_for @u3 } - it { project_path(@project).should be_denied_for :admin } - it { project_path(@project).should be_denied_for @u2 } - it { project_path(@project).should be_denied_for :user } - it { project_path(@project).should be_denied_for :visitor } + subject { project_path(@project) } + + it { should be_allowed_for @u1 } + it { should be_allowed_for @u3 } + it { should be_denied_for :admin } + it { should be_denied_for @u2 } + it { should be_denied_for :user } + it { should be_denied_for :visitor } end describe "GET /project_code/master/tree" do - it { tree_project_ref_path(@project, @project.root_ref).should be_allowed_for @u1 } - it { tree_project_ref_path(@project, @project.root_ref).should be_allowed_for @u3 } - it { tree_project_ref_path(@project, @project.root_ref).should be_denied_for :admin } - it { tree_project_ref_path(@project, @project.root_ref).should be_denied_for @u2 } - it { tree_project_ref_path(@project, @project.root_ref).should be_denied_for :user } - it { tree_project_ref_path(@project, @project.root_ref).should be_denied_for :visitor } + subject { tree_project_ref_path(@project, @project.root_ref) } + + it { should be_allowed_for @u1 } + it { should be_allowed_for @u3 } + it { should be_denied_for :admin } + it { should be_denied_for @u2 } + it { should be_denied_for :user } + it { should be_denied_for :visitor } end describe "GET /project_code/commits" do - it { project_commits_path(@project).should be_allowed_for @u1 } - it { project_commits_path(@project).should be_allowed_for @u3 } - it { project_commits_path(@project).should be_denied_for :admin } - it { project_commits_path(@project).should be_denied_for @u2 } - it { project_commits_path(@project).should be_denied_for :user } - it { project_commits_path(@project).should be_denied_for :visitor } + subject { project_commits_path(@project) } + + it { should be_allowed_for @u1 } + it { should be_allowed_for @u3 } + it { should be_denied_for :admin } + it { should be_denied_for @u2 } + it { should be_denied_for :user } + it { should be_denied_for :visitor } end describe "GET /project_code/commit" do - it { project_commit_path(@project, @project.commit.id).should be_allowed_for @u1 } - it { project_commit_path(@project, @project.commit.id).should be_allowed_for @u3 } - it { project_commit_path(@project, @project.commit.id).should be_denied_for :admin } - it { project_commit_path(@project, @project.commit.id).should be_denied_for @u2 } - it { project_commit_path(@project, @project.commit.id).should be_denied_for :user } - it { project_commit_path(@project, @project.commit.id).should be_denied_for :visitor } + subject { project_commit_path(@project, @project.commit.id) } + + it { should be_allowed_for @u1 } + it { should be_allowed_for @u3 } + it { should be_denied_for :admin } + it { should be_denied_for @u2 } + it { should be_denied_for :user } + it { should be_denied_for :visitor } end describe "GET /project_code/team" do - it { team_project_path(@project).should be_allowed_for @u1 } - it { team_project_path(@project).should be_allowed_for @u3 } - it { team_project_path(@project).should be_denied_for :admin } - it { team_project_path(@project).should be_denied_for @u2 } - it { team_project_path(@project).should be_denied_for :user } - it { team_project_path(@project).should be_denied_for :visitor } + subject { team_project_path(@project) } + + it { should be_allowed_for @u1 } + it { should be_allowed_for @u3 } + it { should be_denied_for :admin } + it { should be_denied_for @u2 } + it { should be_denied_for :user } + it { should be_denied_for :visitor } end describe "GET /project_code/wall" do - it { wall_project_path(@project).should be_allowed_for @u1 } - it { wall_project_path(@project).should be_allowed_for @u3 } - it { wall_project_path(@project).should be_denied_for :admin } - it { wall_project_path(@project).should be_denied_for @u2 } - it { wall_project_path(@project).should be_denied_for :user } - it { wall_project_path(@project).should be_denied_for :visitor } + subject { wall_project_path(@project) } + + it { should be_allowed_for @u1 } + it { should be_allowed_for @u3 } + it { should be_denied_for :admin } + it { should be_denied_for @u2 } + it { should be_denied_for :user } + it { should be_denied_for :visitor } end describe "GET /project_code/blob" do before do - @commit = @project.commit - @path = @commit.tree.contents.select { |i| i.is_a?(Grit::Blob)}.first.name - @blob_path = blob_project_ref_path(@project, @commit.id, path: @path) + commit = @project.commit + path = commit.tree.contents.select { |i| i.is_a?(Grit::Blob)}.first.name + @blob_path = blob_project_ref_path(@project, commit.id, path: path) end it { @blob_path.should be_allowed_for @u1 } @@ -95,93 +107,113 @@ describe "Application access" do end describe "GET /project_code/edit" do - it { edit_project_path(@project).should be_allowed_for @u1 } - it { edit_project_path(@project).should be_denied_for @u3 } - it { edit_project_path(@project).should be_denied_for :admin } - it { edit_project_path(@project).should be_denied_for @u2 } - it { edit_project_path(@project).should be_denied_for :user } - it { edit_project_path(@project).should be_denied_for :visitor } + subject { edit_project_path(@project) } + + it { should be_allowed_for @u1 } + it { should be_denied_for @u3 } + it { should be_denied_for :admin } + it { should be_denied_for @u2 } + it { should be_denied_for :user } + it { should be_denied_for :visitor } end describe "GET /project_code/deploy_keys" do - it { project_deploy_keys_path(@project).should be_allowed_for @u1 } - it { project_deploy_keys_path(@project).should be_denied_for @u3 } - it { project_deploy_keys_path(@project).should be_denied_for :admin } - it { project_deploy_keys_path(@project).should be_denied_for @u2 } - it { project_deploy_keys_path(@project).should be_denied_for :user } - it { project_deploy_keys_path(@project).should be_denied_for :visitor } + subject { project_deploy_keys_path(@project) } + + it { should be_allowed_for @u1 } + it { should be_denied_for @u3 } + it { should be_denied_for :admin } + it { should be_denied_for @u2 } + it { should be_denied_for :user } + it { should be_denied_for :visitor } end describe "GET /project_code/issues" do - it { project_issues_path(@project).should be_allowed_for @u1 } - it { project_issues_path(@project).should be_allowed_for @u3 } - it { project_issues_path(@project).should be_denied_for :admin } - it { project_issues_path(@project).should be_denied_for @u2 } - it { project_issues_path(@project).should be_denied_for :user } - it { project_issues_path(@project).should be_denied_for :visitor } + subject { project_issues_path(@project) } + + it { should be_allowed_for @u1 } + it { should be_allowed_for @u3 } + it { should be_denied_for :admin } + it { should be_denied_for @u2 } + it { should be_denied_for :user } + it { should be_denied_for :visitor } end describe "GET /project_code/snippets" do - it { project_snippets_path(@project).should be_allowed_for @u1 } - it { project_snippets_path(@project).should be_allowed_for @u3 } - it { project_snippets_path(@project).should be_denied_for :admin } - it { project_snippets_path(@project).should be_denied_for @u2 } - it { project_snippets_path(@project).should be_denied_for :user } - it { project_snippets_path(@project).should be_denied_for :visitor } + subject { project_snippets_path(@project) } + + it { should be_allowed_for @u1 } + it { should be_allowed_for @u3 } + it { should be_denied_for :admin } + it { should be_denied_for @u2 } + it { should be_denied_for :user } + it { should be_denied_for :visitor } end describe "GET /project_code/merge_requests" do - it { project_merge_requests_path(@project).should be_allowed_for @u1 } - it { project_merge_requests_path(@project).should be_allowed_for @u3 } - it { project_merge_requests_path(@project).should be_denied_for :admin } - it { project_merge_requests_path(@project).should be_denied_for @u2 } - it { project_merge_requests_path(@project).should be_denied_for :user } - it { project_merge_requests_path(@project).should be_denied_for :visitor } + subject { project_merge_requests_path(@project) } + + it { should be_allowed_for @u1 } + it { should be_allowed_for @u3 } + it { should be_denied_for :admin } + it { should be_denied_for @u2 } + it { should be_denied_for :user } + it { should be_denied_for :visitor } end describe "GET /project_code/repository" do - it { project_repository_path(@project).should be_allowed_for @u1 } - it { project_repository_path(@project).should be_allowed_for @u3 } - it { project_repository_path(@project).should be_denied_for :admin } - it { project_repository_path(@project).should be_denied_for @u2 } - it { project_repository_path(@project).should be_denied_for :user } - it { project_repository_path(@project).should be_denied_for :visitor } + subject { project_repository_path(@project) } + + it { should be_allowed_for @u1 } + it { should be_allowed_for @u3 } + it { should be_denied_for :admin } + it { should be_denied_for @u2 } + it { should be_denied_for :user } + it { should be_denied_for :visitor } end describe "GET /project_code/repository/branches" do - it { branches_project_repository_path(@project).should be_allowed_for @u1 } - it { branches_project_repository_path(@project).should be_allowed_for @u3 } - it { branches_project_repository_path(@project).should be_denied_for :admin } - it { branches_project_repository_path(@project).should be_denied_for @u2 } - it { branches_project_repository_path(@project).should be_denied_for :user } - it { branches_project_repository_path(@project).should be_denied_for :visitor } + subject { branches_project_repository_path(@project) } + + it { should be_allowed_for @u1 } + it { should be_allowed_for @u3 } + it { should be_denied_for :admin } + it { should be_denied_for @u2 } + it { should be_denied_for :user } + it { should be_denied_for :visitor } end describe "GET /project_code/repository/tags" do - it { tags_project_repository_path(@project).should be_allowed_for @u1 } - it { tags_project_repository_path(@project).should be_allowed_for @u3 } - it { tags_project_repository_path(@project).should be_denied_for :admin } - it { tags_project_repository_path(@project).should be_denied_for @u2 } - it { tags_project_repository_path(@project).should be_denied_for :user } - it { tags_project_repository_path(@project).should be_denied_for :visitor } + subject { tags_project_repository_path(@project) } + + it { should be_allowed_for @u1 } + it { should be_allowed_for @u3 } + it { should be_denied_for :admin } + it { should be_denied_for @u2 } + it { should be_denied_for :user } + it { should be_denied_for :visitor } end describe "GET /project_code/hooks" do - it { project_hooks_path(@project).should be_allowed_for @u1 } - it { project_hooks_path(@project).should be_allowed_for @u3 } - it { project_hooks_path(@project).should be_denied_for :admin } - it { project_hooks_path(@project).should be_denied_for @u2 } - it { project_hooks_path(@project).should be_denied_for :user } - it { project_hooks_path(@project).should be_denied_for :visitor } + subject { project_hooks_path(@project) } + + it { should be_allowed_for @u1 } + it { should be_allowed_for @u3 } + it { should be_denied_for :admin } + it { should be_denied_for @u2 } + it { should be_denied_for :user } + it { should be_denied_for :visitor } end describe "GET /project_code/files" do - it { files_project_path(@project).should be_allowed_for @u1 } - it { files_project_path(@project).should be_allowed_for @u3 } - it { files_project_path(@project).should be_denied_for :admin } - it { files_project_path(@project).should be_denied_for @u2 } - it { files_project_path(@project).should be_denied_for :user } - it { files_project_path(@project).should be_denied_for :visitor } + subject { files_project_path(@project) } + + it { should be_allowed_for @u1 } + it { should be_allowed_for @u3 } + it { should be_denied_for :admin } + it { should be_denied_for @u2 } + it { should be_denied_for :user } + it { should be_denied_for :visitor } end end end From 580ce4f2d85f89e81825c3b5993fd3ee68f4d93a Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Sat, 25 Aug 2012 13:54:38 -0400 Subject: [PATCH 5/5] Minor cleanup to Milestone model and spec Back-ported from my still-in-progress major cleanup. --- app/models/milestone.rb | 14 +++----------- spec/models/milestone_spec.rb | 27 ++++++++++++++++++--------- 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/app/models/milestone.rb b/app/models/milestone.rb index 592c57f9..d416fb63 100644 --- a/app/models/milestone.rb +++ b/app/models/milestone.rb @@ -28,17 +28,9 @@ class Milestone < ActiveRecord::Base end def percent_complete - @percent_complete ||= begin - total_i = self.issues.count - closed_i = self.issues.closed.count - if total_i > 0 - (closed_i * 100) / total_i - else - 100 - end - rescue => ex - 0 - end + ((self.issues.closed.count * 100) / self.issues.count).abs + rescue ZeroDivisionError + 100 end def expires_at diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb index 880d3f30..e9acc4e2 100644 --- a/spec/models/milestone_spec.rb +++ b/spec/models/milestone_spec.rb @@ -31,24 +31,33 @@ describe Milestone do it { milestone.should be_valid } - describe "Issues" do - before do + describe "#percent_complete" do + it "should not count open issues" do milestone.issues << issue + milestone.percent_complete.should == 0 end - it { milestone.percent_complete.should == 0 } + it "should count closed issues" do + issue.update_attributes(closed: true) + milestone.issues << issue + milestone.percent_complete.should == 100 + end - it do - issue.update_attributes closed: true + it "should recover from dividing by zero" do + milestone.issues.should_receive(:count).and_return(0) milestone.percent_complete.should == 100 end end - describe :expires_at do - before do - milestone.update_attributes due_date: Date.today + 1.day + describe "#expires_at" do + it "should be nil when due_date is unset" do + milestone.update_attributes(due_date: nil) + milestone.expires_at.should be_nil end - it { milestone.expires_at.should_not be_nil } + it "should not be nil when due_date is set" do + milestone.update_attributes(due_date: Date.tomorrow) + milestone.expires_at.should be_present + end end end