Skip to content

Commit

Permalink
Add model to track merges made while detecting conflicts
Browse files Browse the repository at this point in the history
  • Loading branch information
mikeweaver committed Dec 27, 2015
1 parent 7e4a97a commit 24ede13
Show file tree
Hide file tree
Showing 7 changed files with 149 additions and 3 deletions.
35 changes: 35 additions & 0 deletions app/models/merge.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
class Merge < ActiveRecord::Base
fields do
timestamps
end

belongs_to :source_branch, class_name: Branch, required: true
belongs_to :target_branch, class_name: Branch, required: true
has_one :repository, through: :source_branch

validates :source_branch, :target_branch, presence: true
validates :source_branch, uniqueness: { scope: :target_branch, message: "Merges must be unique" }
validates_each :source_branch do |record, attr, value|
# specifically ignore nil branches, those will be caught by a different validator
if (value.present? && record.target_branch.present?)
if value.id == record.target_branch.id
record.errors.add(attr, 'Branch cannot merge with itself')
end
if value.repository.id != record.target_branch.repository.id
record.errors.add(attr, 'Branches from different repositories cannot be merged')
end
end
end

scope :by_target_user, lambda { |user|
joins(:target_branch).where(
'(branches.author_id = ?)',
user.id)
}

scope :created_after, lambda { |after_date|
where('created_at >= ?', after_date)
}

scope :from_repository, lambda { |repository_name| joins(:repository).where("repositories.name = ?", repository_name) }
end
16 changes: 16 additions & 0 deletions db/migrate/20151227045135_hobo_migration_14.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
class HoboMigration14 < ActiveRecord::Migration
def self.up
create_table :merges do |t|
t.datetime :created_at
t.datetime :updated_at
t.integer :source_branch_id
t.integer :target_branch_id
end
add_index :merges, [:source_branch_id]
add_index :merges, [:target_branch_id]
end

def self.down
drop_table :merges
end
end
12 changes: 11 additions & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema.define(version: 20151220025330) do
ActiveRecord::Schema.define(version: 20151227045135) do

create_table "branches", force: :cascade do |t|
t.datetime "git_tested_at"
Expand Down Expand Up @@ -39,6 +39,16 @@
add_index "conflicts", ["branch_a_id"], name: "index_conflicts_on_branch_a_id"
add_index "conflicts", ["branch_b_id"], name: "index_conflicts_on_branch_b_id"

create_table "merges", force: :cascade do |t|
t.datetime "created_at"
t.datetime "updated_at"
t.integer "source_branch_id"
t.integer "target_branch_id"
end

add_index "merges", ["source_branch_id"], name: "index_merges_on_source_branch_id"
add_index "merges", ["target_branch_id"], name: "index_merges_on_target_branch_id"

create_table "notification_suppressions", force: :cascade do |t|
t.datetime "suppress_until"
t.datetime "created_at"
Expand Down
7 changes: 6 additions & 1 deletion lib/conflict_detector.rb
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,9 @@ def get_conflicts(target_branch, source_branches)
Rails.logger.info("MERGED: #{source_branch.name} can be merged into #{target_branch.name} without conflicts")
if should_push_merged_branch?(target_branch, source_branch)
Rails.logger.info("PUSHING: #{target_branch.name} to origin")
@git.push
if @git.push
Merge.create!(source_branch: source_branch, target_branch: target_branch)
end
end
else
if should_ignore_conflicts?(conflict.conflicting_files)
Expand Down Expand Up @@ -148,6 +150,9 @@ def exclude_tested_branches(branch_to_test, branches_to_test, tested_pairs)
end

def test_branches(untested_branches, start_time)
# destroy record of previous merges
Merge.destroy_all

tested_pairs = []
all_branches = Branch.all
untested_branches.each do |branch|
Expand Down
7 changes: 6 additions & 1 deletion spec/lib/conflict_detector_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -125,11 +125,16 @@ def expect_get_conflicts_equals(
allow_any_instance_of(Git::Git).to receive(:detect_conflicts).and_return(nil)
end
if expected_push_count > 0
expect_any_instance_of(Git::Git).to receive(:push).exactly(expected_push_count).times
expect_any_instance_of(Git::Git).to receive(:push).exactly(expected_push_count).times.and_return(true)
else
expect_any_instance_of(Git::Git).not_to receive(:push)
end
expect(conflict_detector.send(:get_conflicts, target_branch, source_branches)).to match_array(expected_conflict_list)
if expected_push_count > 0
expect(Merge.count).to eq(expected_push_count)
else
expect(Merge.count).to eq(0)
end
end

it 'should include all conflicts when the "ignore files" list is empty' do
Expand Down
71 changes: 71 additions & 0 deletions spec/models/merge_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
require 'spec_helper'

describe 'Merge' do

DIFFERENT_NAME = 'Different Name'

before do
@branches = create_test_branches(author_name: 'Author Name', count: 3)
end

it 'can be created' do
create_test_merge(@branches[0], @branches[1])
expect(Merge.all.size).to eq(1)
end

it 'does not allow duplicate merges' do
create_test_merge(@branches[0], @branches[1])
expect { create_test_merge(@branches[0], @branches[1]) }.to raise_exception(ActiveRecord::RecordInvalid)
end

it 'cannot merge with itself' do
expect { create_test_merge(@branches[0], @branches[0]) }.to raise_exception(ActiveRecord::RecordInvalid)
end

it 'requires two branches' do
expect { create_test_merge(@branches[0], nil) }.to raise_exception(ActiveRecord::RecordInvalid)
expect { create_test_merge(nil, @branches[0]) }.to raise_exception(ActiveRecord::RecordInvalid)
expect { create_test_merge(nil, nil) }.to raise_exception(ActiveRecord::RecordInvalid)
end

it 'does not allow merges between repositories' do
branch_in_different_repository = create_test_branch(repository_name: 'other_repository_name')
expect { create_test_merge(@branches[0], branch_in_different_repository) }.to raise_exception(ActiveRecord::RecordInvalid)
end

context 'with branches from multiple users' do
before do
@other_branches = create_test_branches(author_name: DIFFERENT_NAME, count: 2)
@merge_1 = create_test_merge(@branches[0], @branches[1])
@merge_2 = create_test_merge(@other_branches[0], @other_branches[1])
end

it 'can be looked up by user' do
merges = Merge.by_target_user(User.where(name: DIFFERENT_NAME).first)
expect(merges.size).to eq(1)
expect(merges[0].target_branch.author.name).to eq(DIFFERENT_NAME)
end

it 'can be filtered by status change date' do
merges = Merge.created_after(2.minutes.from_now)
expect(merges.size).to eq(0)

merges = Merge.created_after(2.minutes.ago)
expect(merges.size).to eq(2)
end
end

context 'with branches from multiple repositories' do
it 'returns branches from a repository' do
@merge_1 = create_test_merge(@branches[0], @branches[1])
@merge_2 = create_test_merge(
create_test_branch(repository_name: 'repository_b', name: 'name_a'),
create_test_branch(repository_name: 'repository_b', name: 'name_b'))

merges = Merge.from_repository('repository_b').all
expect(merges.size).to eq(1)
expect(merges[0].id).to eq(@merge_2.id)
end
end
end

4 changes: 4 additions & 0 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,3 +64,7 @@ def create_test_branches(repository_name: 'repository_name', author_name: 'Autho
def create_test_conflict(branch_a, branch_b, tested_at: Time.now, file_list: ['test/file.rb'])
Conflict.create!(branch_a, branch_b, file_list, tested_at)
end

def create_test_merge(source_branch, target_branch)
Merge.create!(source_branch: source_branch, target_branch: target_branch)
end

0 comments on commit 24ede13

Please sign in to comment.