Skip to content
This repository has been archived by the owner on May 10, 2018. It is now read-only.

[WIP] Only denormalize a build if it's the most recent #318

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions lib/travis/model/build.rb
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,10 @@ def pull_request?
event_type == 'pull_request'
end

def last_build_on_default_branch?
repository.last_build_on_default_branch == self
end

# COMPAT: used in http api v1, deprecate as soon as v1 gets retired
def result
state.try(:to_sym) == :passed ? 0 : 1
Expand Down
2 changes: 1 addition & 1 deletion lib/travis/model/build/denormalize.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ def denormalize(event, *args)
}

def denormalize?(event)
DENORMALIZE.key?(event)
DENORMALIZE.key?(event) && last_build_on_default_branch?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@henrikhodne does this mean we only denormalize builds on the default branch? This is going to be confusing in the UI as a pusher event will come through for a PR build and update the side bar, but then if you refresh you won't see that update as the build was not denormalized.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@joshk: Where does travis-web update the sidebar? Looking at the receive method, it doesn't seem to do much when a build:started event comes in.

end

def denormalize_attributes_for(event)
Expand Down
4 changes: 4 additions & 0 deletions lib/travis/model/repository.rb
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,10 @@ def last_build_on(branch)
builds.pushes.last_build_on(branch: branch)
end

def last_build_on_default_branch
builds.pushes.last_build_on(branch: default_branch)
end

def build_status(branch)
builds.pushes.last_state_on(state: [:passed, :failed, :errored], branch: branch)
end
Expand Down
98 changes: 63 additions & 35 deletions spec/travis/model/build/denormalize_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,54 +6,82 @@
let(:build) { Factory(:build, state: :started, duration: 30) }

describe 'on build:started' do
before :each do
build.denormalize(:start)
build.reload
end
describe 'if the build is the most recent' do
before :each do
build.stubs(:last_build_on_default_branch?).returns(true)
build.denormalize(:start)
build.reload
end

it 'denormalizes last_build_id to its repository' do
build.repository.last_build_id.should == build.id
end
it 'denormalizes last_build_id to its repository' do
build.repository.last_build_id.should == build.id
end

it 'denormalizes last_build_state to its repository' do
build.repository.last_build_state.should == 'started'
end
it 'denormalizes last_build_state to its repository' do
build.repository.last_build_state.should == 'started'
end

it 'denormalizes last_build_number to its repository' do
build.repository.last_build_number.should == build.number
end
it 'denormalizes last_build_number to its repository' do
build.repository.last_build_number.should == build.number
end

it 'denormalizes last_build_duration to its repository' do
build.repository.last_build_duration.should == build.duration
end
it 'denormalizes last_build_duration to its repository' do
build.repository.last_build_duration.should == build.duration
end

it 'denormalizes last_build_started_at to its repository' do
build.repository.last_build_started_at.should == build.started_at
end
it 'denormalizes last_build_started_at to its repository' do
build.repository.last_build_started_at.should == build.started_at
end

it 'denormalizes last_build_finished_at to its repository' do
build.repository.last_build_finished_at.should == build.finished_at
it 'denormalizes last_build_finished_at to its repository' do
build.repository.last_build_finished_at.should == build.finished_at
end
end
describe 'if the build is not the most recent' do
before :each do
build.stubs(:most_recent_buid?).returns(true)
build.denormalize(:start)
build.reload
end
it 'does not denormalize' do
build.repository.expects(:update_attributes!).never
build.expects(:denormalize_attributes_for).never
end
end
end

describe 'on build:finished' do
before :each do
build.update_attributes(state: :errored)
build.denormalize(:finish)
build.reload
end
describe 'if the build is the most recent' do
before :each do
build.stubs(:last_build_on_default_branch?).returns(:true)
build.update_attributes(state: :errored)
build.denormalize(:finish)
build.reload
end

it 'denormalizes last_build_state to its repository' do
build.repository.last_build_state.should == 'errored'
end
it 'denormalizes last_build_state to its repository' do
build.repository.last_build_state.should == 'errored'
end

it 'denormalizes last_build_duration to its repository' do
build.repository.last_build_duration.should == build.duration
end
it 'denormalizes last_build_duration to its repository' do
build.repository.last_build_duration.should == build.duration
end

it 'denormalizes last_build_finished_at to its repository' do
build.repository.last_build_finished_at.should == build.finished_at
it 'denormalizes last_build_finished_at to its repository' do
build.repository.last_build_finished_at.should == build.finished_at
end
end
describe 'if the build is not the most recent' do
before :each do
build.stubs(:last_build_on_default_branch?).returns(false)
build.update_attributes(state: :errored)
build.denormalize(:finish)
build.reload
end
it 'does not denormalize' do
build.repository.expects(:update_attributes!).never
build.expects(:denormalize_attributes_for).never
end
end
end
end

18 changes: 18 additions & 0 deletions spec/travis/model/build_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,24 @@
end
end

describe 'last_build_on_default_branch?' do
it 'returns the most recently created build on the default branch' do
build1 = Factory(:build, state: 'passed', started_at: 3.minutes.ago, finished_at: 2.minutes.ago)
build2 = Factory(:build, state: 'failed', started_at: 10.minutes.ago, finished_at: 9.minutes.ago)

build1.last_build_on_default_branch?.should be_false
build2.last_build_on_default_branch?.should be_true
end

it 'ignores builds not on the default branch' do
build1 = Factory(:build, state: 'failed')
build2 = Factory(:build, state: 'passed', commit: Factory(:commit, branch: 'blah'))

build1.last_build_on_default_branch?.should be_true
build2.last_build_on_default_branch?.should be_false
end
end

describe 'on_branch' do
it 'returns builds that are on any of the given branches' do
Factory(:build, commit: Factory(:commit, branch: 'master'))
Expand Down
12 changes: 12 additions & 0 deletions spec/travis/model/repository_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,18 @@
end
end

describe 'last_build_on_default_branch' do
let(:repo) { Factory(:repository) }

it 'returns the last created build for the repostiory on the default branch' do
build1 = Factory(:build, repository: repo, commit: Factory(:commit, branch: 'blah'))
build2 = Factory(:build, repository: repo)
build3 = Factory(:build, repository: repo, started_at: 20.minutes.ago, finished_at: 11.minutes.ago)

repo.last_build_on_default_branch.should == build3
end
end

describe 'api_url' do
let(:repo) { Repository.new(owner_name: 'travis-ci', name: 'travis-ci') }

Expand Down