From a04b919a4c5ef570e46ae94d6d11b09eb893b7cd Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 28 Aug 2012 01:28:09 -0400 Subject: [PATCH 1/9] Add factory_girl_rails gem --- Gemfile | 1 + Gemfile.lock | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/Gemfile b/Gemfile index 045baa36974..4905169f65e 100644 --- a/Gemfile +++ b/Gemfile @@ -107,6 +107,7 @@ group :development, :test do gem "awesome_print" gem "database_cleaner" gem "launchy" + gem 'factory_girl_rails' end group :test do diff --git a/Gemfile.lock b/Gemfile.lock index 656bede4766..b8e97aa0204 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -166,6 +166,11 @@ GEM eventmachine (0.12.10) execjs (1.4.0) multi_json (~> 1.0) + factory_girl (4.0.0) + activesupport (>= 3.0.0) + factory_girl_rails (4.0.0) + factory_girl (~> 4.0.0) + railties (>= 3.0.0) ffaker (1.14.0) ffi (1.0.11) foreman (0.47.0) @@ -388,6 +393,7 @@ DEPENDENCIES devise (~> 2.1.0) draper email_spec + factory_girl_rails ffaker foreman git From 4805c64f2a96e8a9ea5a0e94a820d840fa1675e0 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 28 Aug 2012 01:28:47 -0400 Subject: [PATCH 2/9] Remove spec/factory and spec/factories --- features/support/env.rb | 1 - spec/factories.rb | 97 ----------------------------------------- spec/factory.rb | 29 ------------ spec/spec_helper.rb | 1 - 4 files changed, 128 deletions(-) delete mode 100644 spec/factories.rb delete mode 100644 spec/factory.rb diff --git a/features/support/env.rb b/features/support/env.rb index 0d9a9ed4281..da6c1b70168 100644 --- a/features/support/env.rb +++ b/features/support/env.rb @@ -8,7 +8,6 @@ require 'webmock/cucumber' WebMock.allow_net_connect! -require Rails.root.join 'spec/factories' require Rails.root.join 'spec/support/monkeypatch' require Rails.root.join 'spec/support/gitolite_stub' require Rails.root.join 'spec/support/login_helpers' diff --git a/spec/factories.rb b/spec/factories.rb deleted file mode 100644 index 26868462797..00000000000 --- a/spec/factories.rb +++ /dev/null @@ -1,97 +0,0 @@ -require File.join(Rails.root, 'spec', 'factory') - -Factory.add(:project, Project) do |obj| - obj.name = Faker::Internet.user_name - obj.path = 'gitlabhq' - obj.owner = Factory(:user) - obj.code = 'LGT' -end - -Factory.add(:project_without_owner, Project) do |obj| - obj.name = Faker::Internet.user_name - obj.path = 'gitlabhq' - obj.code = 'LGT' -end - -Factory.add(:public_project, Project) do |obj| - obj.name = Faker::Internet.user_name - obj.path = 'gitlabhq' - obj.private_flag = false - obj.owner = Factory(:user) - obj.code = 'LGT' -end - -Factory.add(:user, User) do |obj| - obj.email = Faker::Internet.email - obj.password = "123456" - obj.name = Faker::Name.name - obj.password_confirmation = "123456" -end - -Factory.add(:admin, User) do |obj| - obj.email = Faker::Internet.email - obj.password = "123456" - obj.name = Faker::Name.name - obj.password_confirmation = "123456" - obj.admin = true -end - -Factory.add(:issue, Issue) do |obj| - obj.title = Faker::Lorem.sentence - obj.author = Factory :user - obj.assignee = Factory :user -end - -Factory.add(:merge_request, MergeRequest) do |obj| - obj.title = Faker::Lorem.sentence - obj.author = Factory :user - obj.assignee = Factory :user - obj.source_branch = "master" - obj.target_branch = "stable" - obj.closed = false -end - -Factory.add(:snippet, Snippet) do |obj| - obj.title = Faker::Lorem.sentence - obj.file_name = Faker::Lorem.sentence - obj.content = Faker::Lorem.sentences -end - -Factory.add(:note, Note) do |obj| - obj.note = Faker::Lorem.sentence -end - -Factory.add(:key, Key) do |obj| - obj.title = "Example key" - obj.key = File.read(File.join(Rails.root, "db", "pkey.example")) -end - -Factory.add(:project_hook, ProjectHook) do |obj| - obj.url = Faker::Internet.uri("http") -end - -Factory.add(:system_hook, SystemHook) do |obj| - obj.url = Faker::Internet.uri("http") -end - -Factory.add(:wiki, Wiki) do |obj| - obj.title = Faker::Lorem.sentence - obj.content = Faker::Lorem.sentence - obj.user = Factory(:user) - obj.project = Factory(:project) -end - -Factory.add(:event, Event) do |obj| - obj.title = Faker::Lorem.sentence - obj.project = Factory(:project) -end - -Factory.add(:milestone, Milestone) do |obj| - obj.title = Faker::Lorem.sentence - obj.due_date = Date.today + 1.month -end - -Factory.add(:users_project, UsersProject) do |obj| - obj.user = Factory :user - obj.project = Factory :project -end diff --git a/spec/factory.rb b/spec/factory.rb deleted file mode 100644 index 1758b4d69d7..00000000000 --- a/spec/factory.rb +++ /dev/null @@ -1,29 +0,0 @@ -class Factory - @factories = {} - - class << self - def add(name, klass, &block) - @factories[name] = [klass, block] - end - - def create(name, opts = {}) - new(name, opts).tap(&:save!) - end - - def new(name, opts = {}) - factory= @factories[name] - factory[0].new.tap do |obj| - factory[1].call(obj) - end.tap do |obj| - opts.each do |k, opt| - obj.send("#{k}=", opt) - end - end - end - end -end - -def Factory(name, opts={}) - Factory.create name, opts -end - diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 06909f392bb..fc5ba14659a 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -10,7 +10,6 @@ require 'rspec/rails' require 'capybara/rails' require 'capybara/rspec' require 'webmock/rspec' -require 'factories' require 'email_spec' require 'headless' From c9c1f76e002d899dd6765c4c1630697cc5068f27 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 28 Aug 2012 01:42:28 -0400 Subject: [PATCH 3/9] All specs and features currently passing with FactoryGirl --- features/support/env.rb | 6 +- spec/factories.rb | 107 ++++++++++++++++++ spec/factories_spec.rb | 83 ++++++++++++++ spec/helpers/gitlab_flavored_markdown_spec.rb | 4 +- spec/models/system_hook_spec.rb | 11 +- spec/spec_helper.rb | 1 + spec/support/monkeypatch.rb | 21 ---- spec/support/stubbed_repository.rb | 60 ++++++++++ 8 files changed, 262 insertions(+), 31 deletions(-) create mode 100644 spec/factories.rb create mode 100644 spec/factories_spec.rb delete mode 100644 spec/support/monkeypatch.rb create mode 100644 spec/support/stubbed_repository.rb diff --git a/features/support/env.rb b/features/support/env.rb index da6c1b70168..5357815201a 100644 --- a/features/support/env.rb +++ b/features/support/env.rb @@ -8,8 +8,8 @@ require 'webmock/cucumber' WebMock.allow_net_connect! -require Rails.root.join 'spec/support/monkeypatch' require Rails.root.join 'spec/support/gitolite_stub' +require Rails.root.join 'spec/support/stubbed_repository' require Rails.root.join 'spec/support/login_helpers' require Rails.root.join 'spec/support/valid_commit' @@ -52,6 +52,8 @@ require 'cucumber/rspec/doubles' include GitoliteStub -Before do +Before do stub_gitolite! end + +World(FactoryGirl::Syntax::Methods) diff --git a/spec/factories.rb b/spec/factories.rb new file mode 100644 index 00000000000..eb3882a6bee --- /dev/null +++ b/spec/factories.rb @@ -0,0 +1,107 @@ +# Backwards compatibility with the old method +def Factory(type, *args) + FactoryGirl.create(type, *args) +end + +module Factory + def self.create(type, *args) + FactoryGirl.create(type, *args) + end + + def self.new(type, *args) + FactoryGirl.build(type, *args) + end +end + +FactoryGirl.define do + sequence :sentence, aliases: [:title, :content] do + Faker::Lorem.sentence + end + + sequence(:url) { Faker::Internet.uri('http') } + + factory :user, aliases: [:author, :assignee, :owner] do + email { Faker::Internet.email } + name { Faker::Name.name } + password "123456" + password_confirmation "123456" + + trait :admin do + admin true + end + + factory :admin, traits: [:admin] + end + + factory :project do + sequence(:name) { |n| "project#{n}" } + path { name } + code { name } + owner + end + + factory :users_project do + user + project + end + + factory :issue do + title + author + project + + trait :closed do + closed true + end + + factory :closed_issue, traits: [:closed] + end + + factory :merge_request do + title + author + project + source_branch "master" + target_branch "stable" + end + + factory :note do + project + note "Note" + end + + factory :event do + end + + factory :key do + title + key { File.read(File.join(Rails.root, "db", "pkey.example")) } + end + + factory :milestone do + title + project + end + + factory :system_hook do + url + end + + factory :project_hook do + url + end + + factory :wiki do + title + content + user + end + + factory :snippet do + project + author + title + content + file_name { Faker::Lorem.sentence } + end +end diff --git a/spec/factories_spec.rb b/spec/factories_spec.rb new file mode 100644 index 00000000000..126a5b2309f --- /dev/null +++ b/spec/factories_spec.rb @@ -0,0 +1,83 @@ +require 'spec_helper' + +describe "Factories" do + describe 'User' do + it "builds a valid instance" do + build(:user).should be_valid + end + + it "builds a valid admin instance" do + build(:admin).should be_valid + end + end + + describe 'Project' do + it "builds a valid instance" do + build(:project).should be_valid + end + end + + describe 'Issue' do + it "builds a valid instance" do + build(:issue).should be_valid + end + + it "builds a valid closed instance" do + build(:closed_issue).should be_valid + end + end + + describe 'MergeRequest' do + it "builds a valid instance" do + build(:merge_request).should be_valid + end + end + + describe 'Note' do + it "builds a valid instance" do + build(:note).should be_valid + end + end + + describe 'Event' do + it "builds a valid instance" do + build(:event).should be_valid + end + end + + describe 'Key' do + it "builds a valid instance" do + build(:key).should be_valid + end + end + + describe 'Milestone' do + it "builds a valid instance" do + build(:milestone).should be_valid + end + end + + describe 'SystemHook' do + it "builds a valid instance" do + build(:system_hook).should be_valid + end + end + + describe 'ProjectHook' do + it "builds a valid instance" do + build(:project_hook).should be_valid + end + end + + describe 'Wiki' do + it "builds a valid instance" do + build(:wiki).should be_valid + end + end + + describe 'Snippet' do + it "builds a valid instance" do + build(:snippet).should be_valid + end + end +end diff --git a/spec/helpers/gitlab_flavored_markdown_spec.rb b/spec/helpers/gitlab_flavored_markdown_spec.rb index e147cb39375..28bd46ecb99 100644 --- a/spec/helpers/gitlab_flavored_markdown_spec.rb +++ b/spec/helpers/gitlab_flavored_markdown_spec.rb @@ -2,7 +2,7 @@ require "spec_helper" describe GitlabMarkdownHelper do before do - @project = Project.find_by_path("gitlabhq") || Factory(:project) + @project = Factory(:project) @commit = @project.repo.commits.first.parents.first @commit = CommitDecorator.decorate(Commit.new(@commit)) @other_project = Factory :project, path: "OtherPath", code: "OtherCode" @@ -157,7 +157,7 @@ describe GitlabMarkdownHelper do gfm("Let @#{user.name} fix the *mess* in #{@commit.id}").should == "Let #{link_to "@#{user.name}", project_team_member_path(@project, member), class: "gfm gfm-team_member "} fix the *mess* in #{link_to @commit.id, project_commit_path(@project, id: @commit.id), title: "Commit: #{@commit.author_name} - #{@commit.title}", class: "gfm gfm-commit "}" end - it "should not trip over other stuff", focus: true do + it "should not trip over other stuff" do gfm("_Please_ *stop* 'helping' and all the other b*$#%' you do.").should == "_Please_ *stop* 'helping' and all the other b*$#%' you do." end diff --git a/spec/models/system_hook_spec.rb b/spec/models/system_hook_spec.rb index 56d76ed08cf..fe2a5836fe7 100644 --- a/spec/models/system_hook_spec.rb +++ b/spec/models/system_hook_spec.rb @@ -10,13 +10,12 @@ describe SystemHook do end it "project_create hook" do - user = Factory :user with_resque do - project = Factory :project_without_owner, owner: user + project = Factory :project end WebMock.should have_requested(:post, @system_hook.url).with(body: /project_create/).once end - + it "project_destroy hook" do project = Factory :project with_resque do @@ -31,7 +30,7 @@ describe SystemHook do end WebMock.should have_requested(:post, @system_hook.url).with(body: /user_create/).once end - + it "user_destroy hook" do user = Factory :user with_resque do @@ -39,7 +38,7 @@ describe SystemHook do end WebMock.should have_requested(:post, @system_hook.url).with(body: /user_destroy/).once end - + it "project_create hook" do user = Factory :user project = Factory :project @@ -48,7 +47,7 @@ describe SystemHook do end WebMock.should have_requested(:post, @system_hook.url).with(body: /user_add_to_team/).once end - + it "project_destroy hook" do user = Factory :user project = Factory :project diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index fc5ba14659a..d381b3f1e2e 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -27,6 +27,7 @@ RSpec.configure do |config| config.include LoginHelpers, type: :request config.include GitoliteStub + config.include FactoryGirl::Syntax::Methods # 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/monkeypatch.rb b/spec/support/monkeypatch.rb deleted file mode 100644 index 04bbb6fb680..00000000000 --- a/spec/support/monkeypatch.rb +++ /dev/null @@ -1,21 +0,0 @@ -# Stubbing Project <-> git host path -# create project using Factory only -class Project - def path_to_repo - File.join(Rails.root, "tmp", "tests", path) - end - - def satellite - @satellite ||= FakeSatellite.new - end -end - -class FakeSatellite - def exists? - true - end - - def create - true - end -end diff --git a/spec/support/stubbed_repository.rb b/spec/support/stubbed_repository.rb new file mode 100644 index 00000000000..fa6c4c9a9b6 --- /dev/null +++ b/spec/support/stubbed_repository.rb @@ -0,0 +1,60 @@ +# Stubs out all Git repository access done by models so that specs can run +# against fake repositories without Grit complaining that they don't exist. +module StubbedRepository + extend ActiveSupport::Concern + + included do + # If a class defines the method we want to stub directly, rather than + # inheriting it from a module (as is the case in UsersProject), that method + # will overwrite our stub, so use alias_method to ensure it's our stub + # getting called. + + alias_method :update_repository, :fake_update_repository + alias_method :destroy_repository, :fake_destroy_repository + alias_method :repository_delete_key, :fake_repository_delete_key + alias_method :path_to_repo, :fake_path_to_repo + alias_method :satellite, :fake_satellite + end + + def fake_update_repository + true + end + + def fake_destroy_repository + true + end + + def fake_repository_delete_key + true + end + + def fake_path_to_repo + if new_record? + # There are a couple Project specs that expect the Project's path to be + # in the returned path, so let's patronize them. + File.join(Rails.root, 'tmp', 'tests', path) + else + # For everything else, just give it the path to one of our real seeded + # repos. + File.join(Rails.root, 'tmp', 'tests', 'gitlabhq_1') + end + end + + def fake_satellite + FakeSatellite.new + end + + class FakeSatellite + def exists? + true + end + + def create + true + end + end +end + +[Project, Key, ProtectedBranch, UsersProject].each do |c| + c.send(:include, StubbedRepository) +end From 0bc909405852135d7f98440193830eba664ea122 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 28 Aug 2012 06:57:00 -0400 Subject: [PATCH 4/9] Add deploy_key and personal_key factories --- spec/factories.rb | 8 ++++++++ spec/factories_spec.rb | 8 ++++++++ 2 files changed, 16 insertions(+) diff --git a/spec/factories.rb b/spec/factories.rb index eb3882a6bee..82fd0db2ea4 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -76,6 +76,14 @@ FactoryGirl.define do factory :key do title key { File.read(File.join(Rails.root, "db", "pkey.example")) } + + factory :deploy_key do + project + end + + factory :personal_key do + user + end end factory :milestone do diff --git a/spec/factories_spec.rb b/spec/factories_spec.rb index 126a5b2309f..5760aad423b 100644 --- a/spec/factories_spec.rb +++ b/spec/factories_spec.rb @@ -49,6 +49,14 @@ describe "Factories" do it "builds a valid instance" do build(:key).should be_valid end + + it "builds a valid deploy key instance" do + build(:deploy_key).should be_valid + end + + it "builds a valid personal key instance" do + build(:personal_key).should be_valid + end end describe 'Milestone' do From 77d06454ededc3beef09db709829ccb687ccc045 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 28 Aug 2012 07:01:27 -0400 Subject: [PATCH 5/9] Simple model spec changes made possible by new factories --- spec/models/issue_spec.rb | 37 ++++++++----------------------- spec/models/key_spec.rb | 17 +++++--------- spec/models/merge_request_spec.rb | 24 +++++--------------- spec/models/milestone_spec.rb | 7 ++---- spec/models/note_spec.rb | 21 +++++++----------- spec/models/user_spec.rb | 26 +++++----------------- 6 files changed, 37 insertions(+), 95 deletions(-) diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index e9cbd72589a..133f073451f 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -19,11 +19,7 @@ describe Issue do it { Issue.should respond_to :opened } end - subject { Factory.create(:issue, - author: Factory(:user), - assignee: Factory(:user), - project: Factory.create(:project)) } - it { should be_valid } + subject { Factory.create(:issue) } describe '#is_being_reassigned?' do it 'returns true if the issue assignee has changed' do @@ -41,11 +37,7 @@ describe Issue do subject.is_being_closed?.should be_true end it 'returns false if the closed attribute has changed and is now false' do - issue = Factory.create(:issue, - closed: true, - author: Factory(:user), - assignee: Factory(:user), - project: Factory.create(:project)) + issue = Factory.create(:closed_issue) issue.closed = false issue.is_being_closed?.should be_false end @@ -57,11 +49,7 @@ describe Issue do describe '#is_being_reopened?' do it 'returns true if the closed attribute has changed and is now false' do - issue = Factory.create(:issue, - closed: true, - author: Factory(:user), - assignee: Factory(:user), - project: Factory.create(:project)) + issue = Factory.create(:closed_issue) issue.closed = false issue.is_being_reopened?.should be_true end @@ -75,40 +63,33 @@ describe Issue do end describe "plus 1" do - let(:project) { Factory(:project) } - subject { - Factory.create(:issue, - author: Factory(:user), - assignee: Factory(:user), - project: project) - } + subject { Factory.create(:issue) } it "with no notes has a 0/0 score" do subject.upvotes.should == 0 end it "should recognize non-+1 notes" do - subject.notes << Factory(:note, note: "No +1 here", project: Factory(:project, path: 'plusone', code: 'plusone')) + subject.notes << Factory(:note, note: "No +1 here") subject.should have(1).note subject.notes.first.upvote?.should be_false subject.upvotes.should == 0 end it "should recognize a single +1 note" do - subject.notes << Factory(:note, note: "+1 This is awesome", project: Factory(:project, path: 'plusone', code: 'plusone')) + subject.notes << Factory(:note, note: "+1 This is awesome") subject.upvotes.should == 1 end it "should recognize a multiple +1 notes" do - subject.notes << Factory(:note, note: "+1 This is awesome", project: Factory(:project, path: 'plusone', code: 'plusone')) - subject.notes << Factory(:note, note: "+1 I want this", project: Factory(:project, path: 'plustwo', code: 'plustwo')) + subject.notes << Factory(:note, note: "+1 This is awesome") + subject.notes << Factory(:note, note: "+1 I want this") subject.upvotes.should == 2 end end describe ".search" do - let!(:issue) { Factory.create(:issue, title: "Searchable issue", - project: Factory.create(:project)) } + let!(:issue) { Factory.create(:issue, title: "Searchable issue") } it "matches by title" do Issue.search('able').all.should == [issue] diff --git a/spec/models/key_spec.rb b/spec/models/key_spec.rb index 0f9b31778df..ea58fbd291e 100644 --- a/spec/models/key_spec.rb +++ b/spec/models/key_spec.rb @@ -17,20 +17,15 @@ describe Key do context "validation of uniqueness" do context "as a deploy key" do - let(:project) { Factory.create(:project, path: 'alpha', code: 'alpha') } - let(:another_project) { Factory.create(:project, path: 'beta', code: 'beta') } - - before do - deploy_key = Factory.create(:key, project: project) - end + let!(:deploy_key) { create(:deploy_key) } it "does not accept the same key twice for a project" do - key = Factory.new(:key, project: project) + key = build(:key, project: deploy_key.project) key.should_not be_valid end it "does accept the same key for another project" do - key = Factory.new(:key, project: another_project) + key = build(:key, project_id: 0) key.should be_valid end end @@ -39,12 +34,12 @@ describe Key do let(:user) { Factory.create(:user) } it "accepts the key once" do - Factory.new(:key, user: user).should be_valid + build(:key, user: user).should be_valid end it "does not accepts the key twice" do - Factory.create(:key, user: user) - Factory.new(:key, user: user).should_not be_valid + create(:key, user: user) + build(:key, user: user).should_not be_valid end end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index c7ad08a1e06..f4b93eea966 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -20,46 +20,34 @@ describe MergeRequest do it { MergeRequest.should respond_to :opened } end - it { Factory.create(:merge_request, - author: Factory(:user), - assignee: Factory(:user), - project: Factory.create(:project)).should be_valid } - describe "plus 1" do - let(:project) { Factory(:project) } - subject { - Factory.create(:merge_request, - author: Factory(:user), - assignee: Factory(:user), - project: project) - } + subject { Factory.create(:merge_request) } it "with no notes has a 0/0 score" do subject.upvotes.should == 0 end it "should recognize non-+1 notes" do - subject.notes << Factory(:note, note: "No +1 here", project: Factory(:project, path: 'plusone', code: 'plusone')) + subject.notes << Factory(:note, note: "No +1 here") subject.should have(1).note subject.notes.first.upvote?.should be_false subject.upvotes.should == 0 end it "should recognize a single +1 note" do - subject.notes << Factory(:note, note: "+1 This is awesome", project: Factory(:project, path: 'plusone', code: 'plusone')) + subject.notes << Factory(:note, note: "+1 This is awesome") subject.upvotes.should == 1 end it "should recognize a multiple +1 notes" do - subject.notes << Factory(:note, note: "+1 This is awesome", project: Factory(:project, path: 'plusone', code: 'plusone')) - subject.notes << Factory(:note, note: "+1 I want this", project: Factory(:project, path: 'plustwo', code: 'plustwo')) + subject.notes << Factory(:note, note: "+1 This is awesome") + subject.notes << Factory(:note, note: "+1 I want this") subject.upvotes.should == 2 end end describe ".search" do - let!(:issue) { Factory.create(:issue, title: "Searchable issue", - project: Factory.create(:project)) } + let!(:issue) { Factory.create(:issue, title: "Searchable issue") } it "matches by title" do Issue.search('able').all.should == [issue] diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb index e9acc4e2815..ed805a243b5 100644 --- a/spec/models/milestone_spec.rb +++ b/spec/models/milestone_spec.rb @@ -25,11 +25,8 @@ describe Milestone do it { should validate_presence_of(:project_id) } end - let(:project) { Factory :project } - let(:milestone) { Factory :milestone, project: project } - let(:issue) { Factory :issue, project: project } - - it { milestone.should be_valid } + let(:milestone) { Factory :milestone } + let(:issue) { Factory :issue } describe "#percent_complete" do it "should not count open issues" do diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index c97b23cb4fa..89e50479762 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -1,9 +1,6 @@ require 'spec_helper' describe Note do - let(:project) { Factory :project } - let!(:commit) { project.commit } - describe "Associations" do it { should belong_to(:project) } end @@ -13,8 +10,6 @@ describe Note do it { should validate_presence_of(:project) } end - it { Factory.create(:note, - project: project).should be_valid } describe "Scopes" do it "should have a today named scope that returns ..." do Note.today.where_values.should == ["created_at >= '#{Date.today}'"] @@ -25,26 +20,27 @@ describe Note do let(:project) { Factory(:project) } it "recognizes a neutral note" do - note = Factory(:note, project: project, note: "This is not a +1 note") + note = Factory(:note, note: "This is not a +1 note") note.should_not be_upvote end it "recognizes a +1 note" do - note = Factory(:note, project: project, note: "+1 for this") + note = Factory(:note, note: "+1 for this") note.should be_upvote end it "recognizes a -1 note as no vote" do - note = Factory(:note, project: project, note: "-1 for this") + note = Factory(:note, note: "-1 for this") note.should_not be_upvote end end - describe "Commit notes" do + let(:project) { create(:project) } + let(:commit) { project.commit } + describe "Commit notes" do before do @note = Factory :note, - project: project, noteable_id: commit.id, noteable_type: "Commit" end @@ -58,7 +54,6 @@ describe Note do describe "Pre-line commit notes" do before do @note = Factory :note, - project: project, noteable_id: commit.id, noteable_type: "Commit", line_code: "0_16_1" @@ -91,8 +86,8 @@ describe Note do describe :authorization do before do - @p1 = project - @p2 = Factory :project, code: "alien", path: "gitlabhq_1" + @p1 = create(:project) + @p2 = Factory :project @u1 = Factory :user @u2 = Factory :user @u3 = Factory :user diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 265dcef1e77..ebc45fa4710 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -3,11 +3,12 @@ require 'spec_helper' describe User do describe "Associations" do it { should have_many(:projects) } - it { should have_many(:users_projects) } - it { should have_many(:issues) } - it { should have_many(:assigned_issues) } - it { should have_many(:merge_requests) } - it { should have_many(:assigned_merge_requests) } + it { should have_many(:users_projects).dependent(:destroy) } + it { should have_many(:issues).dependent(:destroy) } + it { should have_many(:assigned_issues).dependent(:destroy) } + it { should have_many(:merge_requests).dependent(:destroy) } + it { should have_many(:assigned_merge_requests).dependent(:destroy) } + it { should have_many(:notes).dependent(:destroy) } end describe "Respond to" do @@ -49,21 +50,6 @@ describe User do user = Factory(:user) user.authentication_token.should_not == "" end - - describe "dependent" do - before do - @user = Factory :user - @note = Factory :note, - author: @user, - project: Factory(:project) - end - - it "should destroy all notes with user" do - Note.find_by_id(@note.id).should_not be_nil - @user.destroy - Note.find_by_id(@note.id).should be_nil - end - end end # == Schema Information # From a39cfb54616fe156ba43ac3df2b771c3a11d2879 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 28 Aug 2012 20:57:54 -0400 Subject: [PATCH 6/9] Simplify StubbedRepository after GitHost changes from master --- spec/support/stubbed_repository.rb | 29 +++++++---------------------- 1 file changed, 7 insertions(+), 22 deletions(-) diff --git a/spec/support/stubbed_repository.rb b/spec/support/stubbed_repository.rb index fa6c4c9a9b6..637754bac5b 100644 --- a/spec/support/stubbed_repository.rb +++ b/spec/support/stubbed_repository.rb @@ -9,34 +9,19 @@ module StubbedRepository # will overwrite our stub, so use alias_method to ensure it's our stub # getting called. - alias_method :update_repository, :fake_update_repository - alias_method :destroy_repository, :fake_destroy_repository - alias_method :repository_delete_key, :fake_repository_delete_key - alias_method :path_to_repo, :fake_path_to_repo - alias_method :satellite, :fake_satellite - end - - def fake_update_repository - true - end - - def fake_destroy_repository - true - end - - def fake_repository_delete_key - true + alias_method :path_to_repo, :fake_path_to_repo + alias_method :satellite, :fake_satellite end def fake_path_to_repo - if new_record? - # There are a couple Project specs that expect the Project's path to be - # in the returned path, so let's patronize them. + if new_record? || path == 'newproject' + # There are a couple Project specs and features that expect the Project's + # path to be in the returned path, so let's patronize them. File.join(Rails.root, 'tmp', 'tests', path) else # For everything else, just give it the path to one of our real seeded # repos. - File.join(Rails.root, 'tmp', 'tests', 'gitlabhq_1') + File.join(Rails.root, 'tmp', 'tests', 'gitlabhq_0') end end @@ -55,6 +40,6 @@ module StubbedRepository end end -[Project, Key, ProtectedBranch, UsersProject].each do |c| +[Project, ProtectedBranch, UsersProject].each do |c| c.send(:include, StubbedRepository) end From a3a63eeb92f5cc660dc3f03e2d7249e4b5f04acf Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 28 Aug 2012 21:13:22 -0400 Subject: [PATCH 7/9] Remove all instances to 'gitlabhq_x' seed repositories from specs and features We now need only one seed repo! Also cleans up the seeding script. --- db/fixtures/test/001_repo.rb | 32 ++++++++++++-------- features/step_definitions/dashboard_steps.rb | 8 ++--- spec/models/project_spec.rb | 2 +- spec/requests/admin/admin_projects_spec.rb | 2 +- spec/requests/atom/dashboard_issues_spec.rb | 8 ++--- spec/support/stubbed_repository.rb | 4 +-- 6 files changed, 28 insertions(+), 28 deletions(-) diff --git a/db/fixtures/test/001_repo.rb b/db/fixtures/test/001_repo.rb index ebf005a1467..67d4e7bfbde 100644 --- a/db/fixtures/test/001_repo.rb +++ b/db/fixtures/test/001_repo.rb @@ -1,15 +1,23 @@ -# create tmp dir if not exist -tmp_dir = File.join(Rails.root, "tmp") -Dir.mkdir(tmp_dir) unless File.exists?(tmp_dir) +require 'fileutils' -# Create dir for test repo -repo_dir = File.join(Rails.root, "tmp", "tests") -Dir.mkdir(repo_dir) unless File.exists?(repo_dir) +print "Unpacking seed repository..." -`cp spec/seed_project.tar.gz tmp/tests/` -Dir.chdir(repo_dir) -`tar -xf seed_project.tar.gz` -3.times do |i| -`cp -r gitlabhq/ gitlabhq_#{i}/` -puts "Unpacked seed repo - tmp/tests/gitlabhq_#{i}" +SEED_REPO = 'seed_project.tar.gz' +REPO_PATH = File.join(Rails.root, 'tmp', 'repositories') + +# Make whatever directories we need to make +FileUtils.mkdir_p(REPO_PATH) + +# Copy the archive to the repo path +FileUtils.cp(File.join(Rails.root, 'spec', SEED_REPO), REPO_PATH) + +# chdir to the repo path +FileUtils.cd(REPO_PATH) do + # Extract the archive + `tar -xf #{SEED_REPO}` + + # Remove the copy + FileUtils.rm(SEED_REPO) end + +puts ' done.' diff --git a/features/step_definitions/dashboard_steps.rb b/features/step_definitions/dashboard_steps.rb index d910ec90d19..a4edd224dc0 100644 --- a/features/step_definitions/dashboard_steps.rb +++ b/features/step_definitions/dashboard_steps.rb @@ -106,13 +106,9 @@ Given /^I have assigned issues$/ do end Given /^I have authored merge requests$/ do - project1 = Factory :project, - :path => "gitlabhq_1", - :code => "gitlabhq_1" + project1 = Factory :project - project2 = Factory :project, - :path => "gitlabhq_2", - :code => "gitlabhq_2" + project2 = Factory :project project1.add_access(@user, :read, :write) project2.add_access(@user, :read, :write) diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index af193295ee3..faaa9a917f4 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -85,7 +85,7 @@ describe Project do it "should return path to repo" do project = Project.new(path: "somewhere") - project.path_to_repo.should == File.join(Rails.root, "tmp", "tests", "somewhere") + project.path_to_repo.should == File.join(Rails.root, "tmp", "repositories", "somewhere") end it "returns the full web URL for this repo" do diff --git a/spec/requests/admin/admin_projects_spec.rb b/spec/requests/admin/admin_projects_spec.rb index 0ce66f5f868..2edfb59231d 100644 --- a/spec/requests/admin/admin_projects_spec.rb +++ b/spec/requests/admin/admin_projects_spec.rb @@ -87,7 +87,7 @@ describe "Admin::Projects" do visit new_admin_project_path fill_in 'project_name', with: 'NewProject' fill_in 'project_code', with: 'NPR' - fill_in 'project_path', with: 'gitlabhq_1' + fill_in 'project_path', with: 'newproject' expect { click_button "Create project" }.to change { Project.count }.by(1) @project = Project.last end diff --git a/spec/requests/atom/dashboard_issues_spec.rb b/spec/requests/atom/dashboard_issues_spec.rb index 1d208c70b12..79a9b8ef996 100644 --- a/spec/requests/atom/dashboard_issues_spec.rb +++ b/spec/requests/atom/dashboard_issues_spec.rb @@ -6,13 +6,9 @@ describe "User Issues Dashboard" do login_as :user - @project1 = Factory :project, - path: "gitlabhq_0", - code: "TEST1" + @project1 = Factory :project - @project2 = Factory :project, - path: "gitlabhq_1", - code: "TEST2" + @project2 = Factory :project @project1.add_access(@user, :read, :write) @project2.add_access(@user, :read, :write) diff --git a/spec/support/stubbed_repository.rb b/spec/support/stubbed_repository.rb index 637754bac5b..8dd4f40e084 100644 --- a/spec/support/stubbed_repository.rb +++ b/spec/support/stubbed_repository.rb @@ -17,11 +17,11 @@ module StubbedRepository if new_record? || path == 'newproject' # There are a couple Project specs and features that expect the Project's # path to be in the returned path, so let's patronize them. - File.join(Rails.root, 'tmp', 'tests', path) + File.join(Rails.root, 'tmp', 'repositories', path) else # For everything else, just give it the path to one of our real seeded # repos. - File.join(Rails.root, 'tmp', 'tests', 'gitlabhq_0') + File.join(Rails.root, 'tmp', 'repositories', 'gitlabhq') end end From 39ff033d1ab536c4e493510ee99e392b5fd25a07 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 28 Aug 2012 21:15:21 -0400 Subject: [PATCH 8/9] Remove pkey.example --- db/pkey.example | 3 --- spec/factories.rb | 8 +++++++- 2 files changed, 7 insertions(+), 4 deletions(-) delete mode 100644 db/pkey.example diff --git a/db/pkey.example b/db/pkey.example deleted file mode 100644 index ae045772430..00000000000 --- a/db/pkey.example +++ /dev/null @@ -1,3 +0,0 @@ -AAAAB3NzaC1yc2EAAAABJQAAAIEAiPWx6WM4lhHNedGfBpPJNPpZ7yKu+dnn1SJejgt4 -596k6YjzGGphH2TUxwKzxcKDKKezwkpfnxPkSMkuEspGRt/aZZ9wa++Oi7Qkr8prgHc4 -soW6NUlfDzpvZK2H5E7eQaSeP3SAwGmQKUFHCddNaP0L+hM7zhFNzjFvpaMgJw0= diff --git a/spec/factories.rb b/spec/factories.rb index 82fd0db2ea4..929f9295947 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -75,7 +75,13 @@ FactoryGirl.define do factory :key do title - key { File.read(File.join(Rails.root, "db", "pkey.example")) } + key do + """ + AAAAB3NzaC1yc2EAAAABJQAAAIEAiPWx6WM4lhHNedGfBpPJNPpZ7yKu+dnn1SJejgt4 + 596k6YjzGGphH2TUxwKzxcKDKKezwkpfnxPkSMkuEspGRt/aZZ9wa++Oi7Qkr8prgHc4 + soW6NUlfDzpvZK2H5E7eQaSeP3SAwGmQKUFHCddNaP0L+hM7zhFNzjFvpaMgJw0= + """ + end factory :deploy_key do project From 1dda08486b7751a677870486d4b576e6dd6071e7 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Wed, 29 Aug 2012 01:55:41 -0400 Subject: [PATCH 9/9] Only include StubbedRepository in Project model --- spec/support/stubbed_repository.rb | 20 +++----------------- 1 file changed, 3 insertions(+), 17 deletions(-) diff --git a/spec/support/stubbed_repository.rb b/spec/support/stubbed_repository.rb index 8dd4f40e084..90491e430b4 100644 --- a/spec/support/stubbed_repository.rb +++ b/spec/support/stubbed_repository.rb @@ -1,19 +1,7 @@ # Stubs out all Git repository access done by models so that specs can run # against fake repositories without Grit complaining that they don't exist. module StubbedRepository - extend ActiveSupport::Concern - - included do - # If a class defines the method we want to stub directly, rather than - # inheriting it from a module (as is the case in UsersProject), that method - # will overwrite our stub, so use alias_method to ensure it's our stub - # getting called. - - alias_method :path_to_repo, :fake_path_to_repo - alias_method :satellite, :fake_satellite - end - - def fake_path_to_repo + def path_to_repo if new_record? || path == 'newproject' # There are a couple Project specs and features that expect the Project's # path to be in the returned path, so let's patronize them. @@ -25,7 +13,7 @@ module StubbedRepository end end - def fake_satellite + def satellite FakeSatellite.new end @@ -40,6 +28,4 @@ module StubbedRepository end end -[Project, ProtectedBranch, UsersProject].each do |c| - c.send(:include, StubbedRepository) -end +Project.send(:include, StubbedRepository)