Merge pull request #1300 from tsigo/spec_reorganize

Spec cleanup/reorganization

app/models/milestone.rb

@@ -28,18 +28,10 @@ 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
+    ((self.issues.closed.count * 100) / self.issues.count).abs
+  rescue ZeroDivisionError
     100
   end
-                          rescue => ex
-                            0
-                          end
-  end
 
   def expires_at
     "expires at #{due_date.stamp("Aug 21, 2011")}" if due_date

features/step_definitions/project/projects_steps.rb

-include LoginMacros
+include LoginHelpers
 
 Given /^I signin as a user$/ do
   login_as :user

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

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

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

spec/api/issues_spec.rb → spec/requests/api/issues_spec.rb

 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 }
@@ -8,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
@@ -24,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
@@ -33,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
@@ -41,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'
@@ -52,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'
@@ -64,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

spec/api/projects_spec.rb → spec/requests/api/projects_spec.rb

 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' }
@@ -8,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
@@ -25,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
@@ -46,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
@@ -55,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'
@@ -65,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
@@ -74,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
@@ -82,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'
@@ -91,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'
@@ -102,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

spec/api/users_spec.rb → spec/requests/api/users_spec.rb

 require 'spec_helper'
 
 describe Gitlab::API do
+  include ApiHelpers
+
   let(:user) { Factory :user }
 
   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
@@ -21,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
@@ -29,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

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

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,12 @@ 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
 
   # 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 +38,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

spec/support/api.rb

-def api_prefix
-  "/api/#{Gitlab::API::VERSION}"
-end
-
-def json_response
-  JSON.parse(response.body)
-end

spec/support/api_helpers.rb

+module ApiHelpers
+  # 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
+    JSON.parse(response.body)
+  end
+end

spec/support/db_cleaner.rb

+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

spec/support/js_patch.rb

-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

spec/support/login.rb → spec/support/login_helpers.rb

-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"
+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

spec/support/shared_examples.rb

-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