diff options
29 files changed, 626 insertions, 318 deletions
@@ -2,6 +2,7 @@ v5.1 - Registration token and runner token are named differently - Redirect to previous page after sign-in - Dont show archived projects + - Separate Commit logic from Build logic in prep for Parallel Builds v5.0.1 - Update rails to 4.0.5 @@ -78,7 +79,7 @@ v2.2.0 - rescue build timeout correctly - badge helper with markdown & html - increased test coverage to 85% - + v2.1.0 - Removed horizontal scroll for build trace - new status badges diff --git a/app/controllers/builds_controller.rb b/app/controllers/builds_controller.rb index 79054a2..25b921e 100644 --- a/app/controllers/builds_controller.rb +++ b/app/controllers/builds_controller.rb @@ -20,7 +20,7 @@ class BuildsController < ApplicationController raise ActiveRecord::RecordNotFound unless @build - @builds = project.builds.where(sha: @build.sha).order('id DESC') + @builds = @project.commits.find_by_sha(@build.sha).builds.order('id DESC') @builds = @builds.where("id not in (?)", @build.id).page(params[:page]).per(20) respond_to do |format| @@ -35,7 +35,7 @@ class BuildsController < ApplicationController build = project.builds.create( sha: @build.sha, before_sha: @build.before_sha, - push_data: @build.push_data, + push_data: @build.commit.push_data, ref: @build.ref ) @@ -65,6 +65,6 @@ class BuildsController < ApplicationController end def build_by_sha - project.builds.where(sha: params[:id]).last + @project.commits.find_by_sha(sha: params[:id]).try(:last_build) end end diff --git a/app/helpers/builds_helper.rb b/app/helpers/builds_helper.rb index 9980e75..0c1e264 100644 --- a/app/helpers/builds_helper.rb +++ b/app/helpers/builds_helper.rb @@ -1,6 +1,6 @@ module BuildsHelper def build_ref_link build - if build.gitlab? + if build.commit.gitlab? gitlab_ref_link build.project, build.ref else build.ref @@ -8,13 +8,13 @@ module BuildsHelper end def build_compare_link build - if build.gitlab? - gitlab_compare_link build.project, build.short_before_sha, build.short_sha + if build.commit.gitlab? + gitlab_compare_link build.project, build.commit.short_before_sha, build.short_sha end end def build_commit_link build - if build.gitlab? + if build.commit.gitlab? gitlab_commit_link build.project, build.short_sha else build.short_sha diff --git a/app/models/build.rb b/app/models/build.rb index 47ca196..2377d50 100644 --- a/app/models/build.rb +++ b/app/models/build.rb @@ -3,41 +3,30 @@ # Table name: builds # # id :integer not null, primary key -# project_id :integer -# ref :string(255) # status :string(255) # finished_at :datetime # trace :text # created_at :datetime # updated_at :datetime -# sha :string(255) # started_at :datetime # tmp_file :string(255) -# before_sha :string(255) -# push_data :text # runner_id :integer +# commit_id :integer # class Build < ActiveRecord::Base - belongs_to :project + belongs_to :commit belongs_to :runner - serialize :push_data + attr_accessible :status, :finished_at, :trace, :started_at, :runner_id, :commit_id - attr_accessible :project_id, :ref, :sha, :before_sha, - :status, :finished_at, :trace, :started_at, :push_data, :runner_id, :project_name - - validates :before_sha, presence: true - validates :sha, presence: true - validates :ref, presence: true + validates :commit, presence: true validates :status, presence: true - validate :valid_commit_sha scope :running, ->() { where(status: "running") } scope :pending, ->() { where(status: "pending") } scope :success, ->() { where(status: "success") } scope :failed, ->() { where(status: "failed") } - scope :uniq_sha, ->() { select('DISTINCT(sha)') } def self.last_month where('created_at > ?', Date.today - 1.month) @@ -97,44 +86,6 @@ class Build < ActiveRecord::Base state :canceled, value: 'canceled' end - def valid_commit_sha - if self.sha =~ /\A00000000/ - self.errors.add(:sha, " cant be 00000000 (branch removal)") - end - end - - def compare? - gitlab? && before_sha - end - - def gitlab? - project.gitlab? - end - - def ci_skip? - !!(git_commit_message =~ /(\[ci skip\])/) - end - - def git_author_name - commit_data[:author][:name] if commit_data && commit_data[:author] - end - - def git_author_email - commit_data[:author][:email] if commit_data && commit_data[:author] - end - - def git_commit_message - commit_data[:message] if commit_data - end - - def short_before_sha - before_sha[0..8] - end - - def short_sha - sha[0..8] - end - def trace_html html = Ansi2html::convert(trace) if trace.present? html ||= '' @@ -156,47 +107,53 @@ class Build < ActiveRecord::Base project.scripts end - def commit_data - push_data[:commits].each do |commit| - return commit if commit[:id] == sha - end - rescue - nil + def timeout + project.timeout end - # Build a clone-able repo url - # using http and basic auth - def repo_url - auth = "gitlab-ci-token:#{project.token}@" - url = project.gitlab_url + ".git" - url.sub(/^https?:\/\//) do |prefix| - prefix + auth + def duration + if started_at && finished_at + finished_at - started_at + elsif started_at + Time.now - started_at end end - def timeout - project.timeout + + # The following methods are provided for Grape::Entity and end up being + # useful everywhere else to reduce the changes needed for parallel builds. + def ref + commit.ref + end + + def sha + commit.sha + end + + def short_sha + commit.short_sha end + def before_sha + commit.before_sha end + def allow_git_fetch - project.allow_git_fetch + commit.allow_git_fetch end - def project_name - project.name + def project + commit.project end - def project_recipients - recipients = project.email_recipients.split(' ') - recipients << git_author_email if project.email_add_committer? - recipients.uniq + def project_id + commit.project_id end - def duration - if started_at && finished_at - finished_at - started_at - elsif started_at - Time.now - started_at - end + def project_name + commit.project_name + end + + def repo_url + commit.repo_url end end diff --git a/app/models/commit.rb b/app/models/commit.rb new file mode 100644 index 0000000..665f136 --- /dev/null +++ b/app/models/commit.rb @@ -0,0 +1,97 @@ +# == Schema Information +# +# Table name: commits +# +# id :integer not null, primary key +# project_id :integer +# ref :string(255) +# sha :string(255) +# before_sha :string(255) +# push_data :text +# created_at :datetime +# updated_at :datetime +# + +class Commit < ActiveRecord::Base + belongs_to :project + has_many :builds + + serialize :push_data + + validates_presence_of :ref, :sha, :before_sha, :push_data + validate :valid_commit_sha + + def last_build + builds.last + end + + def valid_commit_sha + if self.sha =~ /\A00000000/ + self.errors.add(:sha, " cant be 00000000 (branch removal)") + end + end + + def compare? + gitlab? && before_sha + end + + def gitlab? + project.gitlab? + end + + def ci_skip? + !!(git_commit_message =~ /(\[ci skip\])/) + end + + def git_author_name + commit_data[:author][:name] if commit_data && commit_data[:author] + end + + def git_author_email + commit_data[:author][:email] if commit_data && commit_data[:author] + end + + def git_commit_message + commit_data[:message] if commit_data + end + + def short_before_sha + before_sha[0..8] + end + + def short_sha + sha[0..8] + end + + def commit_data + push_data[:commits].each do |commit| + return commit if commit[:id] == sha + end + rescue + nil + end + + # Build a clone-able repo url + # using http and basic auth + def repo_url + auth = "gitlab-ci-token:#{project.token}@" + url = project.gitlab_url + ".git" + url.sub(/^https?:\/\//) do |prefix| + prefix + auth + end + end + + def allow_git_fetch + project.allow_git_fetch + end + + def project_name + project.name + end + + def project_recipients + recipients = project.email_recipients.split(' ') + recipients << git_author_email if project.email_add_committer? + recipients.uniq + end +end diff --git a/app/models/project.rb b/app/models/project.rb index 5a8f052..ec25158 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -28,7 +28,7 @@ class Project < ActiveRecord::Base :public, :ssh_url_to_repo, :gitlab_id, :allow_git_fetch, :email_recipients, :email_add_committer, :email_only_broken_builds - has_many :builds, dependent: :destroy + has_many :commits, dependent: :destroy has_many :runner_projects, dependent: :destroy has_many :runners, through: :runner_projects has_many :web_hooks, dependent: :destroy @@ -123,6 +123,10 @@ ls -la broken? || success? end + def builds + Build.where(commit_id: commits).references(:commits) + end + def last_build builds.last end @@ -135,8 +139,14 @@ ls -la status end + def last_build_for_ref(ref) + commits_for_ref = commits.where(ref: ref) + Build.where(commit_id: commits_for_ref).last + end + def last_build_for_sha(sha) - builds.where(sha: sha).order('id DESC').limit(1).first + commit = commits.find_by_sha(sha) + commit.last_build unless commit.nil? end def tracked_refs @@ -160,10 +170,10 @@ ls -la web_hooks.any? end - # onlu check for toggling build status within same ref. + # only check for toggling build status within same ref. def last_build_changed_status? - ref = last_build.ref - last_builds = builds.where(ref: ref).order('id DESC').limit(2) + commits_for_ref = commit.where(ref: last_build.ref) + last_builds = Build.where(commit_id: commits_for_ref).order('id DESC').limit(2) return false if last_builds.size < 2 return last_builds[0].status != last_builds[1].status end diff --git a/app/services/create_build_service.rb b/app/services/create_build_service.rb index a5b2484..b3a4edf 100644 --- a/app/services/create_build_service.rb +++ b/app/services/create_build_service.rb @@ -1,28 +1,12 @@ class CreateBuildService def execute(project, params) - before_sha = params[:before] - sha = params[:after] - ref = params[:ref] + commit = Commit.where(project: project).where(sha: params[:after]).first + commit ||= CreateCommitService.new.execute(project, params) - if ref && ref.include?('refs/heads/') - ref = ref.scan(/heads\/(.*)$/).flatten[0] + if commit.persisted? + commit.builds.create + else + commit.builds.new end - - data = { - ref: ref, - sha: sha, - before_sha: before_sha, - push_data: { - before: before_sha, - after: sha, - ref: ref, - user_name: params[:user_name], - repository: params[:repository], - commits: params[:commits], - total_commits_count: params[:total_commits_count] - } - } - - project.builds.create(data) end end diff --git a/app/services/create_commit_service.rb b/app/services/create_commit_service.rb new file mode 100644 index 0000000..5480802 --- /dev/null +++ b/app/services/create_commit_service.rb @@ -0,0 +1,28 @@ +class CreateCommitService + def execute(project, params) + before_sha = params[:before] + sha = params[:after] + ref = params[:ref] + + if ref && ref.include?('refs/heads/') + ref = ref.scan(/heads\/(.*)$/).flatten[0] + end + + data = { + ref: ref, + sha: sha, + before_sha: before_sha, + push_data: { + before: before_sha, + after: sha, + ref: ref, + user_name: params[:user_name], + repository: params[:repository], + commits: params[:commits], + total_commits_count: params[:total_commits_count] + } + } + + project.commits.create(data) + end +end diff --git a/app/services/image_for_build_service.rb b/app/services/image_for_build_service.rb index 425a020..79bf2ee 100644 --- a/app/services/image_for_build_service.rb +++ b/app/services/image_for_build_service.rb @@ -6,7 +6,7 @@ class ImageForBuildService image_name = image_for_build(build) elsif params[:ref] # Look for last build per branch - build = project.builds.where(ref: params[:ref]).last + build = project.last_build_for_ref(params[:ref]) image_name = image_for_build(build) else image_name = 'unknown.png' diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index acd2e54..db52c02 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -7,7 +7,7 @@ # class NotificationService def build_ended(build) - build.project_recipients.each do |recipient| + build.commit.project_recipients.each do |recipient| case build.status.to_sym when :success mailer.build_success_email(build.id, recipient) diff --git a/app/services/web_hook_service.rb b/app/services/web_hook_service.rb index 3e61a99..94962a1 100644 --- a/app/services/web_hook_service.rb +++ b/app/services/web_hook_service.rb @@ -27,7 +27,7 @@ class WebHookService ref: build.ref, sha: build.sha, before_sha: build.before_sha, - push_data: build.push_data + push_data: build.commit.push_data }) end end diff --git a/app/views/builds/_build.html.haml b/app/views/builds/_build.html.haml index f97cd28..54f46df 100644 --- a/app/views/builds/_build.html.haml +++ b/app/views/builds/_build.html.haml @@ -7,7 +7,7 @@ %strong #{build.short_sha} %td.build-message - %span= truncate(build.git_commit_message, length: 50) + %span= truncate(build.commit.git_commit_message, length: 50) %td.build-branch - unless @ref diff --git a/app/views/builds/show.html.haml b/app/views/builds/show.html.haml index 85b37a0..f7e1182 100644 --- a/app/views/builds/show.html.haml +++ b/app/views/builds/show.html.haml @@ -76,7 +76,7 @@ .pull-right %small #{build_commit_link @build} - - if @build.compare? + - if @build.commit.compare? %p %span.attr-name Compare: #{build_compare_link @build} @@ -85,10 +85,10 @@ #{build_ref_link @build} %p %span.attr-name Author: - #{@build.git_author_name} + #{@build.commit.git_author_name} %p %span.attr-name Message: - #{@build.git_commit_message} + #{@build.commit.git_commit_message} - if @builds.present? .build-widget diff --git a/app/views/charts/_overall.haml b/app/views/charts/_overall.haml index 36e1616..34d83ab 100644 --- a/app/views/charts/_overall.haml +++ b/app/views/charts/_overall.haml @@ -18,4 +18,4 @@ %p Commits covered: %strong - = @project.builds.uniq_sha.count
\ No newline at end of file + = @project.commits.count diff --git a/db/migrate/20140823225019_create_commits_from_builds.rb b/db/migrate/20140823225019_create_commits_from_builds.rb new file mode 100644 index 0000000..97252a0 --- /dev/null +++ b/db/migrate/20140823225019_create_commits_from_builds.rb @@ -0,0 +1,80 @@ +class CreateCommitsFromBuilds < ActiveRecord::Migration + def change + create_table :commits do |t| + t.integer :project_id + t.string :ref, nil: false + t.string :sha, nil: false + t.string :before_sha, nil: false + t.text :push_data, nil: false + + t.timestamps + end + + reversible do |migration| + migration.up { init_commits } + migration.down {} + end + + add_column :builds, :commit_id, :integer + + reversible do |migration| + migration.up { associate_builds_with_commit } + migration.down { dissociate_builds_with_commit } + end + + # Remove commit data from builds + # -- don't use change_table, its not very reversible + remove_column :builds, :project_id, :integer + remove_column :builds, :ref, :string + remove_column :builds, :sha, :string + remove_column :builds, :before_sha, :string + remove_column :builds, :push_data, :text + end + + private + + def init_commits + Commit.reset_column_information + + # Create one Commit for each unique sha value + shas = Build.pluck(:sha).uniq + shas.each do |sha| + # Get latest build for a particular commit + build = Build.where(sha: sha).order('created_at desc').first + attributes = { + project_id: build.project_id, + ref: build.ref, + sha: build.sha, + before_sha: build.before_sha, + push_data: build.push_data, + # Original commit status matches the latest build status + status: build.status + } + + Commit.create!(attributes) + end + end + + def associate_builds_with_commit + Build.reset_column_information + Build.find_each do |build| + build.commit_id = Commit.find_by_sha(build.sha).id + build.save! + end + end + + def dissociate_builds_with_commit + Build.reset_column_information + Build.find_each do |build| + # Don't assume an assocation exists + commit = Commit.find(build.commit_id) + + build.project_id = commit.project_id + build.ref = commit.ref + build.sha = commit.sha + build.before_sha = commit.before_sha + build.push_data = commit.push_data + build.save! + end + end +end diff --git a/db/schema.rb b/db/schema.rb index dcb3569..a3edfda 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,30 +11,35 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20140506091853) do +ActiveRecord::Schema.define(version: 20140823225019) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" create_table "builds", force: true do |t| - t.integer "project_id" - t.string "ref" t.string "status" t.datetime "finished_at" t.text "trace" t.datetime "created_at" t.datetime "updated_at" - t.string "sha" t.datetime "started_at" t.string "tmp_file" - t.string "before_sha" - t.text "push_data" t.integer "runner_id" + t.integer "commit_id" end - add_index "builds", ["project_id"], name: "index_builds_on_project_id", using: :btree add_index "builds", ["runner_id"], name: "index_builds_on_runner_id", using: :btree + create_table "commits", force: true do |t| + t.integer "project_id" + t.string "ref" + t.string "sha" + t.string "before_sha" + t.text "push_data" + t.datetime "created_at" + t.datetime "updated_at" + end + create_table "projects", force: true do |t| t.string "name", null: false t.integer "timeout", default: 1800, null: false diff --git a/lib/api/builds.rb b/lib/api/builds.rb index 6ecc248..f96c1d4 100644 --- a/lib/api/builds.rb +++ b/lib/api/builds.rb @@ -14,9 +14,12 @@ module API required_attributes! [:token] ActiveRecord::Base.transaction do - builds = Build.all - builds = builds.where(project_id: current_runner.projects) unless current_runner.shared? - build = builds.first_pending + build = if current_runner.shared? + Build.first_pending + else + commits = Commit.where(project_id: current_runner.projects) + Build.where(commit_id: commits).first_pending + end not_found! and return unless build diff --git a/spec/factories/builds.rb b/spec/factories/builds.rb index d90d16d..7e222b2 100644 --- a/spec/factories/builds.rb +++ b/spec/factories/builds.rb @@ -3,28 +3,21 @@ # Table name: builds # # id :integer not null, primary key -# project_id :integer -# ref :string(255) # status :string(255) # finished_at :datetime # trace :text # created_at :datetime # updated_at :datetime -# sha :string(255) # started_at :datetime # tmp_file :string(255) -# before_sha :string(255) -# push_data :text # runner_id :integer +# commit_id :integer # # Read about factories at https://github.com/thoughtbot/factory_girl FactoryGirl.define do factory :build do - ref 'master' - before_sha '76de212e80737a608d939f648d959671fb0a0142' - sha '97de212e80737a608d939f648d959671fb0a0142' started_at 'Di 29. Okt 09:51:28 CET 2013' finished_at 'Di 29. Okt 09:53:28 CET 2013' end diff --git a/spec/factories/commits.rb b/spec/factories/commits.rb new file mode 100644 index 0000000..c05dad8 --- /dev/null +++ b/spec/factories/commits.rb @@ -0,0 +1,31 @@ +# == Schema Information +# +# Table name: commits +# +# id :integer not null, primary key +# project_id :integer +# ref :string(255) +# sha :string(255) +# before_sha :string(255) +# status :string(255) +# push_data :text +# created_at :datetime +# updated_at :datetime +# + +# Read about factories at https://github.com/thoughtbot/factory_girl + +FactoryGirl.define do + factory :commit do + ref 'master' + before_sha '76de212e80737a608d939f648d959671fb0a0142' + sha '97de212e80737a608d939f648d959671fb0a0142' + push_data do + { + ref: 'refs/heads/master', + before_sha: '76de212e80737a608d939f648d959671fb0a0142', + after_sha: '97de212e80737a608d939f648d959671fb0a0142' + } + end + end +end diff --git a/spec/features/builds_spec.rb b/spec/features/builds_spec.rb index 10cd6a1..2c59e14 100644 --- a/spec/features/builds_spec.rb +++ b/spec/features/builds_spec.rb @@ -4,7 +4,8 @@ describe "Builds" do before do login_as :user @project = FactoryGirl.create :project - @build = FactoryGirl.create :build, project: @project + @commit = FactoryGirl.create :commit, project: @project + @build = FactoryGirl.create :build, commit: @commit end describe "GET /:project/builds" do @@ -12,8 +13,8 @@ describe "Builds" do visit project_build_path(@project, @build) end - it { page.should have_content @build.sha[0..7] } - it { page.should have_content @build.git_commit_message } - it { page.should have_content @build.git_author_name } + it { page.should have_content @commit.sha[0..7] } + it { page.should have_content @commit.git_commit_message } + it { page.should have_content @commit.git_author_name } end end diff --git a/spec/lib/charts_spec.rb b/spec/lib/charts_spec.rb index f45a578..236cfc2 100644 --- a/spec/lib/charts_spec.rb +++ b/spec/lib/charts_spec.rb @@ -5,7 +5,8 @@ describe "Charts" do context "build_times" do before do @project = FactoryGirl.create(:project) - FactoryGirl.create(:build, :project_id => @project.id) + @commit = FactoryGirl.create(:commit, project: @project) + FactoryGirl.create(:build, commit: @commit) end it 'should return build times in minutes' do diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index 9e932ab..ae67c33 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -3,58 +3,43 @@ # Table name: builds # # id :integer not null, primary key -# project_id :integer -# ref :string(255) # status :string(255) # finished_at :datetime # trace :text # created_at :datetime # updated_at :datetime -# sha :string(255) # started_at :datetime # tmp_file :string(255) -# before_sha :string(255) -# push_data :text # runner_id :integer +# commit_id :integer # require 'spec_helper' describe Build do let(:project) { FactoryGirl.create :project } - let(:build) { FactoryGirl.create :build } - let(:build_with_project) { FactoryGirl.create :build, project: project } + let(:commit) { FactoryGirl.create :commit, project: project } + let(:build) { FactoryGirl.create :build, commit: commit } - it { should belong_to(:project) } - it { should validate_presence_of :before_sha } - it { should validate_presence_of :sha } - it { should validate_presence_of :ref } + it { should belong_to(:commit) } it { should validate_presence_of :status } it { should respond_to :success? } it { should respond_to :failed? } it { should respond_to :running? } it { should respond_to :pending? } - it { should respond_to :git_author_name } - it { should respond_to :git_author_email } - it { should respond_to :short_sha } it { should respond_to :trace_html } - it { should allow_mass_assignment_of(:project_id) } - it { should allow_mass_assignment_of(:ref) } - it { should allow_mass_assignment_of(:sha) } - it { should allow_mass_assignment_of(:before_sha) } + it { should allow_mass_assignment_of(:commit_id) } it { should allow_mass_assignment_of(:status) } + it { should allow_mass_assignment_of(:started_at) } it { should allow_mass_assignment_of(:finished_at) } it { should allow_mass_assignment_of(:trace) } - it { should allow_mass_assignment_of(:started_at) } - it { should allow_mass_assignment_of(:push_data) } it { should allow_mass_assignment_of(:runner_id) } - it { should allow_mass_assignment_of(:project_name) } describe :first_pending do - let(:first) { FactoryGirl.create :build, status: 'pending', created_at: Date.yesterday } - let(:second) { FactoryGirl.create :build, status: 'pending' } + let(:first) { FactoryGirl.create :build, commit: commit, status: 'pending', created_at: Date.yesterday } + let(:second) { FactoryGirl.create :build, commit: commit, status: 'pending' } before { first; second } subject { Build.first_pending } @@ -76,73 +61,6 @@ describe Build do end end - describe :ci_skip? do - let(:project) { FactoryGirl.create(:project) } - let(:build) { FactoryGirl.create(:build, project: project) } - - it 'true if commit message contains [ci skip]' do - build.stub(:git_commit_message) { 'Small typo [ci skip]' } - build.ci_skip?.should == true - end - - it 'false if commit message does not contain [ci skip]' do - build.ci_skip?.should == false - end - end - - describe :project_recipients do - - context 'always sending notification' do - it 'should return git_author_email as only recipient when no additional recipients are given' do - project = FactoryGirl.create :project, - email_add_committer: true, - email_recipients: '' - build = FactoryGirl.create :build, - status: :success, - project: project - expected = 'git_author_email' - build.stub(:git_author_email) { expected } - build.project_recipients.should == [expected] - end - - it 'should return git_author_email and additional recipients' do - project = FactoryGirl.create :project, - email_add_committer: true, - email_recipients: 'rec1 rec2' - build = FactoryGirl.create :build, - status: :success, - project: project - expected = 'git_author_email' - build.stub(:git_author_email) { expected } - build.project_recipients.should == ['rec1', 'rec2', expected] - end - - it 'should return recipients' do - project = FactoryGirl.create :project, - email_add_committer: false, - email_recipients: 'rec1 rec2' - build = FactoryGirl.create :build, - status: :success, - project: project - expected = 'git_author_email' - build.stub(:git_author_email) { expected } - build.project_recipients.should == ['rec1', 'rec2'] - end - - it 'should return unique recipients only' do - project = FactoryGirl.create :project, - email_add_committer: true, - email_recipients: 'rec1 rec1 rec2' - build = FactoryGirl.create :build, - status: :success, - project: project - expected = 'rec2' - build.stub(:git_author_email) { expected } - build.project_recipients.should == ['rec1', 'rec2'] - end - end - end - describe :started? do subject { build.started? } @@ -209,17 +127,6 @@ describe Build do end end - describe :valid_commit_sha do - context 'build.sha can not start with 00000000' do - before do - build.sha = '0' * 32 - build.valid_commit_sha - end - - it('build errors should not be empty') { build.errors.should_not be_empty } - end - end - describe :trace do subject { build.trace_html } @@ -234,91 +141,94 @@ describe Build do end end - describe :compare? do - subject { build_with_project.compare? } + describe :commands do + subject { build.commands } - context 'if project.gitlab_url and build.before_sha are not nil' do - it { should be_true } - end + it { should eq(commit.project.scripts) } end - describe :short_sha do - subject { build.short_before_sha } + describe :timeout do + subject { build.timeout } - it { should have(9).items } - it { build.before_sha.should start_with(subject) } + it { should eq(commit.project.timeout) } end - describe :short_sha do - subject { build.short_sha } + describe :duration do + subject { build.duration } + + it { should eq(120.0) } + + context 'if the building process has not started yet' do + before do + build.started_at = nil + build.finished_at = nil + end - it { should have(9).items } - it { build.sha.should start_with(subject) } + it { should be_nil } + end + + context 'if the building process has started' do + before do + build.started_at = Time.now - 1.minute + build.finished_at = nil + end + + it { should be_a(Float) } + it { should > 0.0 } + end end - describe :repo_url do - subject { build_with_project.repo_url } - - it { should be_a(String) } - it { should end_with(".git") } - it { should start_with(project.gitlab_url[0..6]) } - it { should include(project.token) } - it { should include('gitlab-ci-token') } - it { should include(project.gitlab_url[7..-1]) } + describe :ref do + subject { build.ref } + + it { should eq(commit.ref) } end - describe :gitlab? do - subject { build_with_project.gitlab? } + describe :sha do + subject { build.sha } - it { should eq(project.gitlab?) } + it { should eq(commit.sha) } end - describe :commands do - subject { build_with_project.commands } + describe :short_sha do + subject { build.short_sha } - it { should eq(project.scripts) } + it { should eq(commit.short_sha) } end - describe :timeout do - subject { build_with_project.timeout } + describe :before_sha do + subject { build.before_sha } - it { should eq(project.timeout) } + it { should eq(commit.before_sha) } end describe :allow_git_fetch do - subject { build_with_project.allow_git_fetch } + subject { build.allow_git_fetch } - it { should eq(project.allow_git_fetch) } + it { should eq(commit.allow_git_fetch) } end - describe :name do - subject { build_with_project.project_name } + describe :project do + subject { build.project } - it { should eq(project.name) } + it { should eq(commit.project) } end - describe :duration do - subject { build.duration } + describe :project_id do + subject { build.project_id } - it { should eq(120.0) } + it { should eq(commit.project_id) } + end - context 'if the building process has not started yet' do - before do - build.started_at = nil - build.finished_at = nil - end + describe :project_name do + subject { build.project_name } - it { should be_nil } - end + it { should eq(commit.project_name) } + end - context 'if the building process has started' do - before do - build.started_at = Time.now - 1.minute - build.finished_at = nil - end + describe :repo_url do + subject { build.repo_url } - it { should be_a(Float) } - it { should > 0.0 } - end + it { should eq(commit.repo_url) } end end diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb new file mode 100644 index 0000000..380a9b1 --- /dev/null +++ b/spec/models/commit_spec.rb @@ -0,0 +1,172 @@ +# == Schema Information +# +# Table name: commits +# +# id :integer not null, primary key +# project_id :integer +# ref :string(255) +# sha :string(255) +# before_sha :string(255) +# push_data :text +# created_at :datetime +# updated_at :datetime +# + +require 'spec_helper' + +describe Commit do + let(:project) { FactoryGirl.create :project } + let(:commit) { FactoryGirl.create :commit } + let(:commit_with_project) { FactoryGirl.create :commit, project: project } + + it { should belong_to(:project) } + it { should have_many(:builds) } + it { should validate_presence_of :before_sha } + it { should validate_presence_of :sha } + it { should validate_presence_of :ref } + it { should validate_presence_of :push_data } + + it { should respond_to :git_author_name } + it { should respond_to :git_author_email } + it { should respond_to :short_sha } + + it { should allow_mass_assignment_of(:project_id) } + it { should allow_mass_assignment_of(:ref) } + it { should allow_mass_assignment_of(:sha) } + it { should allow_mass_assignment_of(:before_sha) } + it { should allow_mass_assignment_of(:push_data) } + it { should allow_mass_assignment_of(:status) } + it { should allow_mass_assignment_of(:project_name) } + + describe :last_build do + subject { commit.last_build } + before do + @first = FactoryGirl.create :build, commit: commit, created_at: Date.yesterday + @second = FactoryGirl.create :build, commit: commit + end + + it { should be_a(Build) } + it('returns with the most recently created build') { should eq(@second) } + end + + describe :ci_skip? do + let(:project) { FactoryGirl.create(:project) } + let(:commit) { FactoryGirl.create(:commit, project: project) } + + it 'true if commit message contains [ci skip]' do + commit.stub(:git_commit_message) { 'Small typo [ci skip]' } + commit.ci_skip?.should == true + end + + it 'false if commit message does not contain [ci skip]' do + commit.ci_skip?.should == false + end + end + + describe :project_recipients do + + context 'always sending notification' do + it 'should return git_author_email as only recipient when no additional recipients are given' do + project = FactoryGirl.create :project, + email_add_committer: true, + email_recipients: '' + commit = FactoryGirl.create :commit, project: project + expected = 'git_author_email' + commit.stub(:git_author_email) { expected } + commit.project_recipients.should == [expected] + end + + it 'should return git_author_email and additional recipients' do + project = FactoryGirl.create :project, + email_add_committer: true, + email_recipients: 'rec1 rec2' + commit = FactoryGirl.create :commit, project: project + expected = 'git_author_email' + commit.stub(:git_author_email) { expected } + commit.project_recipients.should == ['rec1', 'rec2', expected] + end + + it 'should return recipients' do + project = FactoryGirl.create :project, + email_add_committer: false, + email_recipients: 'rec1 rec2' + commit = FactoryGirl.create :commit, project: project + expected = 'git_author_email' + commit.stub(:git_author_email) { expected } + commit.project_recipients.should == ['rec1', 'rec2'] + end + + it 'should return unique recipients only' do + project = FactoryGirl.create :project, + email_add_committer: true, + email_recipients: 'rec1 rec1 rec2' + commit = FactoryGirl.create :commit, project: project + expected = 'rec2' + commit.stub(:git_author_email) { expected } + commit.project_recipients.should == ['rec1', 'rec2'] + end + end + end + + describe :valid_commit_sha do + context 'commit.sha can not start with 00000000' do + before do + commit.sha = '0' * 32 + commit.valid_commit_sha + end + + it('commit errors should not be empty') { commit.errors.should_not be_empty } + end + end + + describe :compare? do + subject { commit_with_project.compare? } + + context 'if project.gitlab_url and commit.before_sha are not nil' do + it { should be_true } + end + end + + describe :short_sha do + subject { commit.short_before_sha } + + it { should have(9).items } + it { commit.before_sha.should start_with(subject) } + end + + describe :short_sha do + subject { commit.short_sha } + + it { should have(9).items } + it { commit.sha.should start_with(subject) } + end + + describe :repo_url do + subject { commit_with_project.repo_url } + + it { should be_a(String) } + it { should end_with(".git") } + it { should start_with(project.gitlab_url[0..6]) } + it { should include(project.token) } + it { should include('gitlab-ci-token') } + it { should include(project.gitlab_url[7..-1]) } + end + + describe :gitlab? do + subject { commit_with_project.gitlab? } + + it { should eq(project.gitlab?) } + end + + describe :allow_git_fetch do + subject { commit_with_project.allow_git_fetch } + + it { should eq(project.allow_git_fetch) } + end + + describe :name do + subject { commit_with_project.project_name } + + it { should eq(project.name) } + end +end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index fc06414..0e29b5c 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -27,7 +27,7 @@ require 'spec_helper' describe Project do subject { FactoryGirl.build :project } - it { should have_many(:builds) } + it { should have_many(:commits) } it { should validate_presence_of :name } it { should validate_presence_of :scripts } @@ -49,8 +49,11 @@ describe Project do context :valid_project do let(:project) { FactoryGirl.create :project } - context :project_with_build do - before { FactoryGirl.create(:build, project: project) } + context :project_with_commit_and_builds do + before do + commit = FactoryGirl.create(:commit, project: project) + FactoryGirl.create(:build, commit: commit) + end it { project.status.should == 'pending' } it { project.last_build.should be_kind_of(Build) } diff --git a/spec/requests/builds_spec.rb b/spec/requests/builds_spec.rb index 26b21b2..0fe2313 100644 --- a/spec/requests/builds_spec.rb +++ b/spec/requests/builds_spec.rb @@ -13,7 +13,8 @@ describe API::API do describe "POST /builds/register" do it "should start a build" do - build = FactoryGirl.create(:build, project_id: project.id, status: 'pending' ) + commit = FactoryGirl.create(:commit, project: project) + build = FactoryGirl.create(:build, commit: commit, status: 'pending' ) post api("/builds/register"), token: runner.token @@ -29,7 +30,8 @@ describe API::API do end describe "PUT /builds/:id" do - let(:build) { FactoryGirl.create(:build, project_id: project.id, runner_id: runner.id) } + let(:commit) { FactoryGirl.create(:commit, project: project)} + let(:build) { FactoryGirl.create(:build, commit: commit, runner_id: runner.id) } it "should update a running build" do build.run! diff --git a/spec/services/create_commit_service_spec.rb b/spec/services/create_commit_service_spec.rb new file mode 100644 index 0000000..bb2d2d9 --- /dev/null +++ b/spec/services/create_commit_service_spec.rb @@ -0,0 +1,25 @@ +require 'spec_helper' + +describe CreateCommitService do + let(:service) { CreateCommitService.new } + let(:project) { FactoryGirl.create(:project) } + + describe :execute do + context 'valid params' do + let(:commit) { service.execute(project, ref: 'refs/heads/master', before: '00000000', after: '31das312') } + + it { commit.should be_kind_of(Commit) } + it { commit.should be_valid } + it { commit.should be_persisted } + it { commit.should == project.commits.last } + end + + context 'without params' do + let(:commit) { service.execute(project, {}) } + + it { commit.should be_kind_of(Commit) } + it { commit.should_not be_valid } + it { commit.should_not be_persisted } + end + end +end diff --git a/spec/services/image_for_build_service_spec.rb b/spec/services/image_for_build_service_spec.rb index 2a65c48..2c9da1f 100644 --- a/spec/services/image_for_build_service_spec.rb +++ b/spec/services/image_for_build_service_spec.rb @@ -3,7 +3,8 @@ require 'spec_helper' describe ImageForBuildService do let(:service) { ImageForBuildService.new } let(:project) { FactoryGirl.create(:project) } - let(:build) { FactoryGirl.create(:build, project: project, ref: 'master') } + let(:commit) { FactoryGirl.create(:commit, project: project, ref: 'master') } + let(:build) { FactoryGirl.create(:build, commit: commit) } describe :execute do before { build } diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index fff9c87..413523c 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -6,11 +6,12 @@ describe NotificationService do describe 'Builds' do describe 'failed build' do - let(:project) { FactoryGirl.create(:project)} - let(:build) { FactoryGirl.create(:build, :status => :failed, :project => project) } + let(:project) { FactoryGirl.create(:project) } + let(:commit) { FactoryGirl.create(:commit, project: project) } + let(:build) { FactoryGirl.create(:build, status: :failed, commit: commit) } it do - should_email(build.git_author_email) + should_email(commit.git_author_email) notification.build_ended(build) end @@ -21,10 +22,11 @@ describe NotificationService do end describe 'successfull build' do - let(:project) { FactoryGirl.create(:project)} - let(:build) { FactoryGirl.create(:build, :status => :success, :project => project) } + let(:project) { FactoryGirl.create(:project) } + let(:commit) { FactoryGirl.create(:commit, project: project) } + let(:build) { FactoryGirl.create(:build, status: :success, commit: commit) } it do - should_email(build.git_author_email) + should_email(commit.git_author_email) notification.build_ended(build) end @@ -35,11 +37,12 @@ describe NotificationService do end describe 'successfull build and project has email_recipients' do - let(:project) { FactoryGirl.create(:project, :email_recipients => "jeroen@example.com")} - let(:build) { FactoryGirl.create(:build, :status => :success, :project => project) } + let(:project) { FactoryGirl.create(:project, email_recipients: "jeroen@example.com") } + let(:commit) { FactoryGirl.create(:commit, project: project) } + let(:build) { FactoryGirl.create(:build, status: :success, commit: commit) } it do - should_email(build.git_author_email) + should_email(commit.git_author_email) should_email("jeroen@example.com") notification.build_ended(build) end diff --git a/spec/services/web_hook_service_spec.rb b/spec/services/web_hook_service_spec.rb index 699c126..bc63fd8 100644 --- a/spec/services/web_hook_service_spec.rb +++ b/spec/services/web_hook_service_spec.rb @@ -2,7 +2,8 @@ require 'spec_helper' describe WebHookService do let (:project) { FactoryGirl.create :project } - let (:build) { FactoryGirl.create :build, project: project } + let (:commit) { FactoryGirl.create :commit, project: project } + let (:build) { FactoryGirl.create :build, commit: commit } let (:hook) { FactoryGirl.create :web_hook, project: project } describe :execute do |