From 054f2f98eda60682fd796a1f66681accc6f7ce1c Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Wed, 11 Nov 2015 17:14:47 +0100
Subject: Faster way of obtaining latest event update time

Instead of using MAX(events.updated_at) we can simply sort the events in
descending order by the "id" column and grab the first row. In other
words, instead of this:

    SELECT max(events.updated_at) AS max_id
    FROM events
    LEFT OUTER JOIN projects   ON projects.id   = events.project_id
    LEFT OUTER JOIN namespaces ON namespaces.id = projects.namespace_id
    WHERE events.author_id IS NOT NULL
    AND events.project_id IN (13083);

we can use this:

    SELECT events.updated_at AS max_id
    FROM events
    LEFT OUTER JOIN projects   ON projects.id   = events.project_id
    LEFT OUTER JOIN namespaces ON namespaces.id = projects.namespace_id
    WHERE events.author_id IS NOT NULL
    AND events.project_id IN (13083)
    ORDER BY events.id DESC
    LIMIT 1;

This has the benefit that on PostgreSQL a backwards index scan can be
used, which due to the "LIMIT 1" will at most process only a single row.
This in turn greatly speeds up the process of grabbing the latest update
time. This can be confirmed by looking at the query plans. The first
query produces the following plan:

    Aggregate  (cost=43779.84..43779.85 rows=1 width=12) (actual time=2142.462..2142.462 rows=1 loops=1)
      ->  Index Scan using index_events_on_project_id on events  (cost=0.43..43704.69 rows=30060 width=12) (actual time=0.033..2138.086 rows=32769 loops=1)
            Index Cond: (project_id = 13083)
            Filter: (author_id IS NOT NULL)
    Planning time: 1.248 ms
    Execution time: 2142.548 ms

The second query in turn produces the following plan:

    Limit  (cost=0.43..41.65 rows=1 width=16) (actual time=1.394..1.394 rows=1 loops=1)
      ->  Index Scan Backward using events_pkey on events  (cost=0.43..1238907.96 rows=30060 width=16) (actual time=1.394..1.394 rows=1 loops=1)
            Filter: ((author_id IS NOT NULL) AND (project_id = 13083))
            Rows Removed by Filter: 2104
    Planning time: 0.166 ms
    Execution time: 1.408 ms

According to the above plans the 2nd query is around 1500 times faster.
However, re-running the first query produces timings of around 80 ms,
making the 2nd query "only" around 55 times faster.
---
 spec/models/event_spec.rb | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

(limited to 'spec/models')

diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb
index 0f32f162a10..f46c50ed6f3 100644
--- a/spec/models/event_spec.rb
+++ b/spec/models/event_spec.rb
@@ -64,4 +64,25 @@ describe Event do
     it { expect(@event.branch_name).to eq("master") }
     it { expect(@event.author).to eq(@user) }
   end
+
+  describe '.latest_update_time' do
+    describe 'when events are present' do
+      let(:time) { Time.utc(2015, 1, 1) }
+
+      before do
+        create(:closed_issue_event, updated_at: time)
+        create(:closed_issue_event, updated_at: time + 5)
+      end
+
+      it 'returns the latest update time' do
+        expect(Event.latest_update_time).to eq(time + 5)
+      end
+    end
+
+    describe 'when no events exist' do
+      it 'returns nil' do
+        expect(Event.latest_update_time).to be_nil
+      end
+    end
+  end
 end
-- 
cgit v1.2.1


From 5fcd9986b86812786c90a0b8d3461db79ce71051 Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Mon, 16 Nov 2015 14:28:02 +0100
Subject: Refactor getting user groups/projects/contributions

This new setup no longer loads any IDs into memory using "pluck",
instead using SQL UNIONs to merge the various datasets together. This
results in greatly improved query performance as well as a reduction of
memory usage.

The old setup was in particular problematic when requesting the
authorized projects _including_ public/internal projects as this would
result in roughly 65000 project IDs being loaded into memory. These IDs
would in turn be passed to other queries.
---
 spec/models/user_spec.rb | 52 ++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 48 insertions(+), 4 deletions(-)

(limited to 'spec/models')

diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 7d716c23120..71160f8dfef 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -686,7 +686,7 @@ describe User do
     end
   end
 
-  describe "#contributed_projects_ids" do
+  describe "#contributed_projects" do
     subject { create(:user) }
     let!(:project1) { create(:project) }
     let!(:project2) { create(:project, forked_from_project: project3) }
@@ -701,15 +701,15 @@ describe User do
     end
 
     it "includes IDs for projects the user has pushed to" do
-      expect(subject.contributed_projects_ids).to include(project1.id)
+      expect(subject.contributed_projects).to include(project1)
     end
 
     it "includes IDs for projects the user has had merge requests merged into" do
-      expect(subject.contributed_projects_ids).to include(project3.id)
+      expect(subject.contributed_projects).to include(project3)
     end
 
     it "doesn't include IDs for unrelated projects" do
-      expect(subject.contributed_projects_ids).not_to include(project2.id)
+      expect(subject.contributed_projects).not_to include(project2)
     end
   end
 
@@ -758,4 +758,48 @@ describe User do
       expect(subject.recent_push).to eq(nil)
     end
   end
+
+  describe '#authorized_groups' do
+    let!(:user) { create(:user) }
+    let!(:private_group) { create(:group) }
+    let!(:public_group) { create(:group, public: true) }
+
+    before do
+      private_group.add_user(user, Gitlab::Access::MASTER)
+    end
+
+    describe 'excluding public groups' do
+      subject { user.authorized_groups }
+
+      it { is_expected.to eq([private_group]) }
+    end
+
+    describe 'including public groups' do
+      subject { user.authorized_groups(true) }
+
+      it { is_expected.to eq([public_group, private_group]) }
+    end
+  end
+
+  describe '#authorized_projects' do
+    let!(:user) { create(:user) }
+    let!(:private_project) { create(:project, :private) }
+    let!(:public_project) { create(:project, :public) }
+
+    before do
+      private_project.team << [user, Gitlab::Access::MASTER]
+    end
+
+    describe 'excluding public projects' do
+      subject { user.authorized_projects }
+
+      it { is_expected.to eq([private_project]) }
+    end
+
+    describe 'including public projects' do
+      subject { user.authorized_projects(true) }
+
+      it { is_expected.to eq([public_project, private_project]) }
+    end
+  end
 end
-- 
cgit v1.2.1


From 01620dd7e7f3015e31ac0288ef71fcfc4f268a14 Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Wed, 18 Nov 2015 12:25:37 +0100
Subject: Added Event.limit_recent

This will be used to move some querying logic from the users controller
to the Event model (where it belongs).
---
 spec/models/event_spec.rb | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

(limited to 'spec/models')

diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb
index f46c50ed6f3..ae53f7a536b 100644
--- a/spec/models/event_spec.rb
+++ b/spec/models/event_spec.rb
@@ -85,4 +85,21 @@ describe Event do
       end
     end
   end
+
+  describe '.limit_recent' do
+    let!(:event1) { create(:closed_issue_event) }
+    let!(:event2) { create(:closed_issue_event) }
+
+    describe 'without an explicit limit' do
+      subject { Event.limit_recent }
+
+      it { is_expected.to eq([event2, event1]) }
+    end
+
+    describe 'with an explicit limit' do
+      subject { Event.limit_recent(1) }
+
+      it { is_expected.to eq([event2]) }
+    end
+  end
 end
-- 
cgit v1.2.1


From a74d6d204366c862657a545d999cb33dfde300dd Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Wed, 18 Nov 2015 12:27:21 +0100
Subject: Group methods for filtering public/visible groups

These methods will be used to get a list of groups, optionally
restricted to only those visible to a given user.
---
 spec/models/group_spec.rb | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

(limited to 'spec/models')

diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index bbfc5535eec..6f166b5ab75 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -38,6 +38,33 @@ describe Group do
     it { is_expected.not_to validate_presence_of :owner }
   end
 
+  describe '.public_and_given_groups' do
+    let!(:public_group) { create(:group, public: true) }
+
+    subject { described_class.public_and_given_groups([group.id]) }
+
+    it { is_expected.to eq([public_group, group]) }
+  end
+
+  describe '.visible_to_user' do
+    let!(:group) { create(:group) }
+    let!(:user)  { create(:user) }
+
+    subject { described_class.visible_to_user(user) }
+
+    describe 'when the user has access to a group' do
+      before do
+        group.add_user(user, Gitlab::Access::MASTER)
+      end
+
+      it { is_expected.to eq([group]) }
+    end
+
+    describe 'when the user does not have access to any groups' do
+      it { is_expected.to eq([]) }
+    end
+  end
+
   describe '#to_reference' do
     it 'returns a String reference to the object' do
       expect(group.to_reference).to eq "@#{group.name}"
-- 
cgit v1.2.1


From a4fc8112df3cf6cb344cfba65f5df46c7a99bef7 Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Wed, 18 Nov 2015 12:29:45 +0100
Subject: Added Project.visible_to_user

This method can be used to filter projects to those visible to a given
user.
---
 spec/models/project_spec.rb | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

(limited to 'spec/models')

diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 8d7e6e76766..c42e8870f8c 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -464,4 +464,23 @@ describe Project do
       end
     end
   end
+
+  describe '.visible_to_user' do
+    let!(:project) { create(:project, :private) }
+    let!(:user)    { create(:user) }
+
+    subject { described_class.visible_to_user(user) }
+
+    describe 'when a user has access to a project' do
+      before do
+        project.team.add_user(user, Gitlab::Access::MASTER)
+      end
+
+      it { is_expected.to eq([project]) }
+    end
+
+    describe 'when a user does not have access to any projects' do
+      it { is_expected.to eq([]) }
+    end
+  end
 end
-- 
cgit v1.2.1


From e116a356b8ac07bd3a935c40ceb274d67d808c83 Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Wed, 18 Nov 2015 12:30:24 +0100
Subject: Refactor User#authorized_groups/projects

These methods no longer include public groups/projects (that don't
belong to the actual user) as this is handled by the various finder
classes now. This also removes the need for passing extra arguments.

Note that memoizing was removed _explicitly_. For whatever reason doing
so messes up the users controller to a point where it claims a certain
user does _not_ have access to certain groups/projects when it does have
access. Existing code shouldn't be affected as these methods are only
called in ways that they'd run queries anyway (e.g. a combination of
"any?" and "each" which would run 2 queries regardless of memoizing).
---
 spec/models/user_spec.rb | 26 ++++----------------------
 1 file changed, 4 insertions(+), 22 deletions(-)

(limited to 'spec/models')

diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 71160f8dfef..4631b12faf1 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -762,44 +762,26 @@ describe User do
   describe '#authorized_groups' do
     let!(:user) { create(:user) }
     let!(:private_group) { create(:group) }
-    let!(:public_group) { create(:group, public: true) }
 
     before do
       private_group.add_user(user, Gitlab::Access::MASTER)
     end
 
-    describe 'excluding public groups' do
-      subject { user.authorized_groups }
+    subject { user.authorized_groups }
 
-      it { is_expected.to eq([private_group]) }
-    end
-
-    describe 'including public groups' do
-      subject { user.authorized_groups(true) }
-
-      it { is_expected.to eq([public_group, private_group]) }
-    end
+    it { is_expected.to eq([private_group]) }
   end
 
   describe '#authorized_projects' do
     let!(:user) { create(:user) }
     let!(:private_project) { create(:project, :private) }
-    let!(:public_project) { create(:project, :public) }
 
     before do
       private_project.team << [user, Gitlab::Access::MASTER]
     end
 
-    describe 'excluding public projects' do
-      subject { user.authorized_projects }
+    subject { user.authorized_projects }
 
-      it { is_expected.to eq([private_project]) }
-    end
-
-    describe 'including public projects' do
-      subject { user.authorized_projects(true) }
-
-      it { is_expected.to eq([public_project, private_project]) }
-    end
+    it { is_expected.to eq([private_project]) }
   end
 end
-- 
cgit v1.2.1


From fd2c0fe446c7f761b845c91307ef8110d869e8e8 Mon Sep 17 00:00:00 2001
From: Valery Sizov <vsv2711@gmail.com>
Date: Wed, 11 Nov 2015 15:12:51 +0200
Subject: award emoji

---
 spec/models/project_spec.rb | 11 -----------
 1 file changed, 11 deletions(-)

(limited to 'spec/models')

diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 8d7e6e76766..3c537220106 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -345,17 +345,6 @@ describe Project do
       expect(project1.star_count).to eq(0)
       expect(project2.star_count).to eq(0)
     end
-
-    it 'is decremented when an upvoter account is deleted' do
-      user = create :user
-      project = create :project, :public
-      user.toggle_star(project)
-      project.reload
-      expect(project.star_count).to eq(1)
-      user.destroy
-      project.reload
-      expect(project.star_count).to eq(0)
-    end
   end
 
   describe :avatar_type do
-- 
cgit v1.2.1


From 23c5473cc0bd9b9034c5671fbea1356b0fb531e5 Mon Sep 17 00:00:00 2001
From: Valery Sizov <vsv2711@gmail.com>
Date: Thu, 19 Nov 2015 13:20:09 +0200
Subject: added spinach tests

---
 spec/models/note_spec.rb | 75 ------------------------------------------------
 1 file changed, 75 deletions(-)

(limited to 'spec/models')

diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb
index 75564839dcf..6e37dae07d0 100644
--- a/spec/models/note_spec.rb
+++ b/spec/models/note_spec.rb
@@ -32,77 +32,6 @@ describe Note do
     it { is_expected.to validate_presence_of(:project) }
   end
 
-  describe '#votable?' do
-    it 'is true for issue notes' do
-      note = build(:note_on_issue)
-      expect(note).to be_votable
-    end
-
-    it 'is true for merge request notes' do
-      note = build(:note_on_merge_request)
-      expect(note).to be_votable
-    end
-
-    it 'is false for merge request diff notes' do
-      note = build(:note_on_merge_request_diff)
-      expect(note).not_to be_votable
-    end
-
-    it 'is false for commit notes' do
-      note = build(:note_on_commit)
-      expect(note).not_to be_votable
-    end
-
-    it 'is false for commit diff notes' do
-      note = build(:note_on_commit_diff)
-      expect(note).not_to be_votable
-    end
-  end
-
-  describe 'voting score' do
-    it 'recognizes a neutral note' do
-      note = build(:votable_note, note: 'This is not a +1 note')
-      expect(note).not_to be_upvote
-      expect(note).not_to be_downvote
-    end
-
-    it 'recognizes a neutral emoji note' do
-      note = build(:votable_note, note: "I would :+1: this, but I don't want to")
-      expect(note).not_to be_upvote
-      expect(note).not_to be_downvote
-    end
-
-    it 'recognizes a +1 note' do
-      note = build(:votable_note, note: '+1 for this')
-      expect(note).to be_upvote
-    end
-
-    it 'recognizes a +1 emoji as a vote' do
-      note = build(:votable_note, note: ':+1: for this')
-      expect(note).to be_upvote
-    end
-
-    it 'recognizes a thumbsup emoji as a vote' do
-      note = build(:votable_note, note: ':thumbsup: for this')
-      expect(note).to be_upvote
-    end
-
-    it 'recognizes a -1 note' do
-      note = build(:votable_note, note: '-1 for this')
-      expect(note).to be_downvote
-    end
-
-    it 'recognizes a -1 emoji as a vote' do
-      note = build(:votable_note, note: ':-1: for this')
-      expect(note).to be_downvote
-    end
-
-    it 'recognizes a thumbsdown emoji as a vote' do
-      note = build(:votable_note, note: ':thumbsdown: for this')
-      expect(note).to be_downvote
-    end
-  end
-
   describe "Commit notes" do
     let!(:note) { create(:note_on_commit, note: "+1 from me") }
     let!(:commit) { note.noteable }
@@ -139,10 +68,6 @@ describe Note do
     it "should be recognized by #for_commit_diff_line?" do
       expect(note).to be_for_commit_diff_line
     end
-
-    it "should not be votable" do
-      expect(note).not_to be_votable
-    end
   end
 
   describe 'authorization' do
-- 
cgit v1.2.1


From 671a49cfd53230b57acf579a609bab958e066982 Mon Sep 17 00:00:00 2001
From: Valery Sizov <vsv2711@gmail.com>
Date: Thu, 19 Nov 2015 14:41:05 +0200
Subject: added specs

---
 spec/models/note_spec.rb | 13 +++++++++++++
 1 file changed, 13 insertions(+)

(limited to 'spec/models')

diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb
index 6e37dae07d0..86b6c27a182 100644
--- a/spec/models/note_spec.rb
+++ b/spec/models/note_spec.rb
@@ -129,4 +129,17 @@ describe Note do
 
     it { expect(Note.search('wow')).to include(note) }
   end
+
+  describe :grouped_awards do
+    before do
+      create :note, note: "smile"
+      create :note, note: "smile"
+    end
+
+    it "returns grouped array of notes" do
+      grouped_array = Note.grouped_awards
+      expect(Note.grouped_awards.first.first).to eq("smile")
+      expect(Note.grouped_awards.first.last).to match_array(Note.all)
+    end
+  end
 end
-- 
cgit v1.2.1


From 22bbb379ae976d94d7ddd7addb7384e0b79ddfd9 Mon Sep 17 00:00:00 2001
From: Valery Sizov <vsv2711@gmail.com>
Date: Thu, 19 Nov 2015 23:00:30 +0200
Subject: fox tests

---
 spec/models/note_spec.rb | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

(limited to 'spec/models')

diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb
index 86b6c27a182..f347f537550 100644
--- a/spec/models/note_spec.rb
+++ b/spec/models/note_spec.rb
@@ -132,12 +132,11 @@ describe Note do
 
   describe :grouped_awards do
     before do
-      create :note, note: "smile"
-      create :note, note: "smile"
+      create :note, note: "smile", is_award: true
+      create :note, note: "smile", is_award: true
     end
 
     it "returns grouped array of notes" do
-      grouped_array = Note.grouped_awards
       expect(Note.grouped_awards.first.first).to eq("smile")
       expect(Note.grouped_awards.first.last).to match_array(Note.all)
     end
-- 
cgit v1.2.1


From d496a6b919abd32252f54e59d5607956005cfb15 Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Fri, 20 Nov 2015 23:43:10 +0100
Subject: Handle removed source projects in MR CI commits

When calling MergeRequest#ci_commit the code would previously raise an
error if the source project no longer existed (e.g. because the user
removed their fork).

See #3599 for more information.
---
 spec/models/merge_request_spec.rb | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

(limited to 'spec/models')

diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 90af75ff0e3..567c911425c 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -193,4 +193,29 @@ describe MergeRequest do
   it_behaves_like 'a Taskable' do
     subject { create :merge_request, :simple }
   end
+
+  describe '#ci_commit' do
+    describe 'when the source project exists' do
+      it 'returns the latest commit' do
+        commit    = double(:commit, id: '123abc')
+        ci_commit = double(:ci_commit)
+
+        allow(subject).to receive(:last_commit).and_return(commit)
+
+        expect(subject.source_project).to receive(:ci_commit).
+          with('123abc').
+          and_return(ci_commit)
+
+        expect(subject.ci_commit).to eq(ci_commit)
+      end
+    end
+
+    describe 'when the source project does not exist' do
+      it 'returns nil' do
+        allow(subject).to receive(:source_project).and_return(nil)
+
+        expect(subject.ci_commit).to be_nil
+      end
+    end
+  end
 end
-- 
cgit v1.2.1


From b9df1a63550c78396d43b661bd24d2745604f6fc Mon Sep 17 00:00:00 2001
From: Jose Corcuera <jzcorcuera@gmail.com>
Date: Thu, 26 Nov 2015 10:16:50 -0500
Subject: Strip attributes for Milestone and Issuable. #3428

---
 spec/models/concerns/strip_attribute_spec.rb | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 spec/models/concerns/strip_attribute_spec.rb

(limited to 'spec/models')

diff --git a/spec/models/concerns/strip_attribute_spec.rb b/spec/models/concerns/strip_attribute_spec.rb
new file mode 100644
index 00000000000..6445e29c3ef
--- /dev/null
+++ b/spec/models/concerns/strip_attribute_spec.rb
@@ -0,0 +1,20 @@
+require 'spec_helper'
+
+describe Milestone, "StripAttribute" do
+  let(:milestone) { create(:milestone) }
+
+  describe ".strip_attributes" do
+    it { expect(Milestone).to respond_to(:strip_attributes) }
+    it { expect(Milestone.strip_attrs).to include(:title) }
+  end
+
+  describe "#strip_attributes" do
+    before do
+      milestone.title = '    8.3   '
+      milestone.valid?
+    end
+
+    it { expect(milestone.title).to eq('8.3') }
+  end
+
+end
-- 
cgit v1.2.1


From e92ceb7b57139e985674a44cfe75534c52ed4acd Mon Sep 17 00:00:00 2001
From: Valery Sizov <valery@gitlab.com>
Date: Mon, 30 Nov 2015 16:12:31 +0200
Subject: fix specs

---
 .../ci/project_services/mail_service_spec.rb       | 62 +++++++++-------------
 spec/models/project_services/jira_service_spec.rb  |  6 +--
 2 files changed, 27 insertions(+), 41 deletions(-)

(limited to 'spec/models')

diff --git a/spec/models/ci/project_services/mail_service_spec.rb b/spec/models/ci/project_services/mail_service_spec.rb
index d9b3d34ff15..c03be3ef75f 100644
--- a/spec/models/ci/project_services/mail_service_spec.rb
+++ b/spec/models/ci/project_services/mail_service_spec.rb
@@ -44,13 +44,10 @@ describe Ci::MailService do
       end
 
       it do
-        should_email("git@example.com")
-        mail.execute(build)
-      end
-
-      def should_email(email)
-        expect(Ci::Notify).to receive(:build_fail_email).with(build.id, email)
-        expect(Ci::Notify).not_to receive(:build_success_email).with(build.id, email)
+        perform_enqueued_jobs do
+          expect{ mail.execute(build) }.to change{ ActionMailer::Base.deliveries.size }.by(1)
+          expect(ActionMailer::Base.deliveries.last.to).to eq(["git@example.com"])
+        end
       end
     end
 
@@ -67,13 +64,10 @@ describe Ci::MailService do
       end
 
       it do
-        should_email("git@example.com")
-        mail.execute(build)
-      end
-
-      def should_email(email)
-        expect(Ci::Notify).to receive(:build_success_email).with(build.id, email)
-        expect(Ci::Notify).not_to receive(:build_fail_email).with(build.id, email)
+        perform_enqueued_jobs do
+          expect{ mail.execute(build) }.to change{ ActionMailer::Base.deliveries.size }.by(1)
+          expect(ActionMailer::Base.deliveries.last.to).to eq(["git@example.com"])
+        end
       end
     end
 
@@ -95,14 +89,12 @@ describe Ci::MailService do
       end
 
       it do
-        should_email("git@example.com")
-        should_email("jeroen@example.com")
-        mail.execute(build)
-      end
-
-      def should_email(email)
-        expect(Ci::Notify).to receive(:build_success_email).with(build.id, email)
-        expect(Ci::Notify).not_to receive(:build_fail_email).with(build.id, email)
+        perform_enqueued_jobs do
+          expect{ mail.execute(build) }.to change{ ActionMailer::Base.deliveries.size }.by(2)
+          expect(
+            ActionMailer::Base.deliveries.map(&:to).flatten
+          ).to include("git@example.com", "jeroen@example.com")
+        end
       end
     end
 
@@ -124,14 +116,11 @@ describe Ci::MailService do
       end
 
       it do
-        should_email(commit.git_author_email)
-        should_email("jeroen@example.com")
-        mail.execute(build) if mail.can_execute?(build)
-      end
-
-      def should_email(email)
-        expect(Ci::Notify).not_to receive(:build_success_email).with(build.id, email)
-        expect(Ci::Notify).not_to receive(:build_fail_email).with(build.id, email)
+        perform_enqueued_jobs do
+          expect do
+            mail.execute(build) if mail.can_execute?(build)
+          end.to_not change{ ActionMailer::Base.deliveries.size }
+        end
       end
     end
 
@@ -177,14 +166,11 @@ describe Ci::MailService do
 
       it do
         Ci::Build.retry(build)
-        should_email(commit.git_author_email)
-        should_email("jeroen@example.com")
-        mail.execute(build) if mail.can_execute?(build)
-      end
-
-      def should_email(email)
-        expect(Ci::Notify).not_to receive(:build_success_email).with(build.id, email)
-        expect(Ci::Notify).not_to receive(:build_fail_email).with(build.id, email)
+        perform_enqueued_jobs do
+          expect do
+            mail.execute(build) if mail.can_execute?(build)
+          end.to_not change{ ActionMailer::Base.deliveries.size }
+        end
       end
     end
   end
diff --git a/spec/models/project_services/jira_service_spec.rb b/spec/models/project_services/jira_service_spec.rb
index ddd2cce212c..576f5fc79eb 100644
--- a/spec/models/project_services/jira_service_spec.rb
+++ b/spec/models/project_services/jira_service_spec.rb
@@ -94,9 +94,9 @@ describe JiraService do
       end
 
       it 'should be prepopulated with the settings' do
-        expect(@service.properties[:project_url]).to eq('http://jira.sample/projects/project_a')
-        expect(@service.properties[:issues_url]).to eq("http://jira.sample/issues/:id")
-        expect(@service.properties[:new_issue_url]).to eq("http://jira.sample/projects/project_a/issues/new")
+        expect(@service.properties["project_url"]).to eq('http://jira.sample/projects/project_a')
+        expect(@service.properties["issues_url"]).to eq("http://jira.sample/issues/:id")
+        expect(@service.properties["new_issue_url"]).to eq("http://jira.sample/projects/project_a/issues/new")
       end
     end
   end
-- 
cgit v1.2.1


From a7be01cd07430a4302668224947b2ed135c2d7bb Mon Sep 17 00:00:00 2001
From: Douwe Maan <douwe@gitlab.com>
Date: Mon, 30 Nov 2015 21:10:52 +0100
Subject: Render commit range reference with short shas, link to full shas.

---
 spec/models/commit_range_spec.rb | 111 ++++++++++++++++-----------------------
 1 file changed, 45 insertions(+), 66 deletions(-)

(limited to 'spec/models')

diff --git a/spec/models/commit_range_spec.rb b/spec/models/commit_range_spec.rb
index 1031af097bd..58283f06972 100644
--- a/spec/models/commit_range_spec.rb
+++ b/spec/models/commit_range_spec.rb
@@ -7,50 +7,56 @@ describe CommitRange do
     it { is_expected.to include_module(Referable) }
   end
 
-  let(:sha_from) { 'f3f85602' }
-  let(:sha_to)   { 'e86e1013' }
+  let!(:project) { create(:project, :public) }
+  let!(:commit1) { project.commit("HEAD~2") }
+  let!(:commit2) { project.commit }
 
-  let(:range)  { described_class.new("#{sha_from}...#{sha_to}") }
-  let(:range2) { described_class.new("#{sha_from}..#{sha_to}") }
+  let(:sha_from) { commit1.short_id }
+  let(:sha_to)   { commit2.short_id }
+
+  let(:full_sha_from) { commit1.id }
+  let(:full_sha_to)   { commit2.id }
+
+  let(:range)  { described_class.new("#{sha_from}...#{sha_to}", project) }
+  let(:range2) { described_class.new("#{sha_from}..#{sha_to}", project) }
 
   it 'raises ArgumentError when given an invalid range string' do
-    expect { described_class.new("Foo") }.to raise_error(ArgumentError)
+    expect { described_class.new("Foo", project) }.to raise_error(ArgumentError)
   end
 
   describe '#to_s' do
     it 'is correct for three-dot syntax' do
-      expect(range.to_s).to eq "#{sha_from[0..7]}...#{sha_to[0..7]}"
+      expect(range.to_s).to eq "#{full_sha_from}...#{full_sha_to}"
     end
 
     it 'is correct for two-dot syntax' do
-      expect(range2.to_s).to eq "#{sha_from[0..7]}..#{sha_to[0..7]}"
+      expect(range2.to_s).to eq "#{full_sha_from}..#{full_sha_to}"
     end
   end
 
   describe '#to_reference' do
-    let(:project) { double('project', to_reference: 'namespace1/project') }
+    let(:cross) { create(:project) }
 
-    before do
-      range.project = project
+    it 'returns a String reference to the object' do
+      expect(range.to_reference).to eq "#{sha_from}...#{sha_to}"
     end
 
     it 'returns a String reference to the object' do
-      expect(range.to_reference).to eq range.to_s
+      expect(range2.to_reference).to eq "#{sha_from}..#{sha_to}"
     end
 
     it 'supports a cross-project reference' do
-      cross = double('project')
-      expect(range.to_reference(cross)).to eq "#{project.to_reference}@#{range.to_s}"
+      expect(range.to_reference(cross)).to eq "#{project.to_reference}@#{sha_from}...#{sha_to}"
     end
   end
 
   describe '#reference_title' do
     it 'returns the correct String for three-dot ranges' do
-      expect(range.reference_title).to eq "Commits #{sha_from} through #{sha_to}"
+      expect(range.reference_title).to eq "Commits #{full_sha_from} through #{full_sha_to}"
     end
 
     it 'returns the correct String for two-dot ranges' do
-      expect(range2.reference_title).to eq "Commits #{sha_from}^ through #{sha_to}"
+      expect(range2.reference_title).to eq "Commits #{full_sha_from}^ through #{full_sha_to}"
     end
   end
 
@@ -60,11 +66,11 @@ describe CommitRange do
     end
 
     it 'includes the correct values for a three-dot range' do
-      expect(range.to_param).to eq({ from: sha_from, to: sha_to })
+      expect(range.to_param).to eq({ from: full_sha_from, to: full_sha_to })
     end
 
     it 'includes the correct values for a two-dot range' do
-      expect(range2.to_param).to eq({ from: sha_from + '^', to: sha_to })
+      expect(range2.to_param).to eq({ from: full_sha_from + '^', to: full_sha_to })
     end
   end
 
@@ -79,64 +85,37 @@ describe CommitRange do
   end
 
   describe '#valid_commits?' do
-    context 'without a project' do
-      it 'returns nil' do
-        expect(range.valid_commits?).to be_nil
+    context 'with a valid repo' do
+      before do
+        expect(project).to receive(:valid_repo?).and_return(true)
       end
-    end
 
-    it 'accepts an optional project argument' do
-      project1 = double('project1').as_null_object
-      project2 = double('project2').as_null_object
+      it 'is false when `sha_from` is invalid' do
+        expect(project).to receive(:commit).with(sha_from).and_return(nil)
+        expect(project).to receive(:commit).with(sha_to).and_call_original
 
-      # project1 gets assigned through the accessor, but ignored when not given
-      # as an argument to `valid_commits?`
-      expect(project1).not_to receive(:present?)
-      range.project = project1
+        expect(range).not_to be_valid_commits
+      end
 
-      # project2 gets passed to `valid_commits?`
-      expect(project2).to receive(:present?).and_return(false)
+      it 'is false when `sha_to` is invalid' do
+        expect(project).to receive(:commit).with(sha_from).and_call_original
+        expect(project).to receive(:commit).with(sha_to).and_return(nil)
 
-      range.valid_commits?(project2)
-    end
+        expect(range).not_to be_valid_commits
+      end
 
-    context 'with a project' do
-      let(:project) { double('project', repository: double('repository')) }
-
-      context 'with a valid repo' do
-        before do
-          expect(project).to receive(:valid_repo?).and_return(true)
-          range.project = project
-        end
-
-        it 'is false when `sha_from` is invalid' do
-          expect(project.repository).to receive(:commit).with(sha_from).and_return(false)
-          expect(project.repository).not_to receive(:commit).with(sha_to)
-          expect(range).not_to be_valid_commits
-        end
-
-        it 'is false when `sha_to` is invalid' do
-          expect(project.repository).to receive(:commit).with(sha_from).and_return(true)
-          expect(project.repository).to receive(:commit).with(sha_to).and_return(false)
-          expect(range).not_to be_valid_commits
-        end
-
-        it 'is true when both `sha_from` and `sha_to` are valid' do
-          expect(project.repository).to receive(:commit).with(sha_from).and_return(true)
-          expect(project.repository).to receive(:commit).with(sha_to).and_return(true)
-          expect(range).to be_valid_commits
-        end
+      it 'is true when both `sha_from` and `sha_to` are valid' do
+        expect(range).to be_valid_commits
       end
+    end
 
-      context 'without a valid repo' do
-        before do
-          expect(project).to receive(:valid_repo?).and_return(false)
-          range.project = project
-        end
+    context 'without a valid repo' do
+      before do
+        expect(project).to receive(:valid_repo?).and_return(false)
+      end
 
-        it 'returns false' do
-          expect(range).not_to be_valid_commits
-        end
+      it 'returns false' do
+        expect(range).not_to be_valid_commits
       end
     end
   end
-- 
cgit v1.2.1


From 4a0ebccf6b96184888f2d28b6e97592c6cb239c7 Mon Sep 17 00:00:00 2001
From: Douwe Maan <douwe@gitlab.com>
Date: Mon, 30 Nov 2015 22:43:54 +0100
Subject: Fix specs

---
 spec/models/commit_spec.rb | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

(limited to 'spec/models')

diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb
index 90be9324951..1aaa927c216 100644
--- a/spec/models/commit_spec.rb
+++ b/spec/models/commit_spec.rb
@@ -15,12 +15,12 @@ describe Commit do
 
   describe '#to_reference' do
     it 'returns a String reference to the object' do
-      expect(commit.to_reference).to eq commit.id
+      expect(commit.to_reference).to eq commit.short_id
     end
 
     it 'supports a cross-project reference' do
       cross = double('project')
-      expect(commit.to_reference(cross)).to eq "#{project.to_reference}@#{commit.id}"
+      expect(commit.to_reference(cross)).to eq "#{project.to_reference}@#{commit.short_id}"
     end
   end
 
@@ -77,14 +77,10 @@ eos
     let(:other_issue) { create :issue, project: other_project }
 
     it 'detects issues that this commit is marked as closing' do
-      allow(commit).to receive(:safe_message).and_return("Fixes ##{issue.iid}")
-      expect(commit.closes_issues).to eq([issue])
-    end
-
-    it 'does not detect issues from other projects' do
       ext_ref = "#{other_project.path_with_namespace}##{other_issue.iid}"
-      allow(commit).to receive(:safe_message).and_return("Fixes #{ext_ref}")
-      expect(commit.closes_issues).to be_empty
+      allow(commit).to receive(:safe_message).and_return("Fixes ##{issue.iid} and #{ext_ref}")
+      expect(commit.closes_issues).to include(issue)
+      expect(commit.closes_issues).to include(other_issue)
     end
   end
 
@@ -92,7 +88,7 @@ eos
     subject { create(:project).commit }
 
     let(:author) { create(:user, email: subject.author_email) }
-    let(:backref_text) { "commit #{subject.id}" }
+    let(:backref_text) { "commit #{subject.short_id}" }
     let(:set_mentionable_text) do
       ->(txt) { allow(subject).to receive(:safe_message).and_return(txt) }
     end
-- 
cgit v1.2.1


From 62c14ba2edf9ac4b4bb1e8c46c0c60f1b6574909 Mon Sep 17 00:00:00 2001
From: Douwe Maan <douwe@gitlab.com>
Date: Tue, 1 Dec 2015 12:58:45 +0100
Subject: Render commit reference using short sha, but include full sha in
 comment.

---
 spec/models/commit_range_spec.rb | 22 +++++++++++++++++++---
 spec/models/commit_spec.rb       | 17 ++++++++++++++---
 2 files changed, 33 insertions(+), 6 deletions(-)

(limited to 'spec/models')

diff --git a/spec/models/commit_range_spec.rb b/spec/models/commit_range_spec.rb
index 58283f06972..3c1009a2eb0 100644
--- a/spec/models/commit_range_spec.rb
+++ b/spec/models/commit_range_spec.rb
@@ -38,15 +38,31 @@ describe CommitRange do
     let(:cross) { create(:project) }
 
     it 'returns a String reference to the object' do
-      expect(range.to_reference).to eq "#{sha_from}...#{sha_to}"
+      expect(range.to_reference).to eq "#{full_sha_from}...#{full_sha_to}"
     end
 
     it 'returns a String reference to the object' do
-      expect(range2.to_reference).to eq "#{sha_from}..#{sha_to}"
+      expect(range2.to_reference).to eq "#{full_sha_from}..#{full_sha_to}"
     end
 
     it 'supports a cross-project reference' do
-      expect(range.to_reference(cross)).to eq "#{project.to_reference}@#{sha_from}...#{sha_to}"
+      expect(range.to_reference(cross)).to eq "#{project.to_reference}@#{full_sha_from}...#{full_sha_to}"
+    end
+  end
+
+  describe '#reference_link_text' do
+    let(:cross) { create(:project) }
+
+    it 'returns a String reference to the object' do
+      expect(range.reference_link_text).to eq "#{sha_from}...#{sha_to}"
+    end
+
+    it 'returns a String reference to the object' do
+      expect(range2.reference_link_text).to eq "#{sha_from}..#{sha_to}"
+    end
+
+    it 'supports a cross-project reference' do
+      expect(range.reference_link_text(cross)).to eq "#{project.to_reference}@#{sha_from}...#{sha_to}"
     end
   end
 
diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb
index 1aaa927c216..974b52c1833 100644
--- a/spec/models/commit_spec.rb
+++ b/spec/models/commit_spec.rb
@@ -15,12 +15,23 @@ describe Commit do
 
   describe '#to_reference' do
     it 'returns a String reference to the object' do
-      expect(commit.to_reference).to eq commit.short_id
+      expect(commit.to_reference).to eq commit.id
     end
 
     it 'supports a cross-project reference' do
       cross = double('project')
-      expect(commit.to_reference(cross)).to eq "#{project.to_reference}@#{commit.short_id}"
+      expect(commit.to_reference(cross)).to eq "#{project.to_reference}@#{commit.id}"
+    end
+  end
+
+  describe '#reference_link_text' do
+    it 'returns a String reference to the object' do
+      expect(commit.reference_link_text).to eq commit.short_id
+    end
+
+    it 'supports a cross-project reference' do
+      cross = double('project')
+      expect(commit.reference_link_text(cross)).to eq "#{project.to_reference}@#{commit.short_id}"
     end
   end
 
@@ -88,7 +99,7 @@ eos
     subject { create(:project).commit }
 
     let(:author) { create(:user, email: subject.author_email) }
-    let(:backref_text) { "commit #{subject.short_id}" }
+    let(:backref_text) { "commit #{subject.id}" }
     let(:set_mentionable_text) do
       ->(txt) { allow(subject).to receive(:safe_message).and_return(txt) }
     end
-- 
cgit v1.2.1


From f94afdbdb232418da08658516bc469cf8429dadc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= <ruben@gitlab.com>
Date: Wed, 2 Dec 2015 14:40:43 -0500
Subject: Fix broken spec related to MySQL and fractional seconds support.

---
 spec/models/project_spec.rb | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

(limited to 'spec/models')

diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index f80fada45e9..06a02c13bf1 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -444,7 +444,9 @@ describe Project do
 
       before do
         2.times do
-          create(:note_on_commit, project: project2, created_at: date)
+          # Little fix for special issue related to Fractional Seconds support for MySQL.
+          # See: https://github.com/rails/rails/pull/14359/files
+          create(:note_on_commit, project: project2, created_at: date + 1)
         end
       end
 
-- 
cgit v1.2.1