Merge remote-tracking branch 'ce/8-5-stable' into ce-8-5-stable-to-ee-8-5-stable-ee

features/steps/project/project_milestone.rb

+class Spinach::Features::ProjectMilestone < Spinach::FeatureSteps
+  include SharedAuthentication
+  include SharedProject
+  include SharedPaths
+
+  step 'milestone has issue "Bugfix1" with labels: "bug", "feature"' do
+    project = Project.find_by(name: "Shop")
+    milestone = project.milestones.find_by(title: 'v2.2')
+    issue = create(:issue, title: "Bugfix1", project: project, milestone: milestone)
+    issue.labels << project.labels.find_by(title: 'bug')
+    issue.labels << project.labels.find_by(title: 'feature')
+  end
+
+  step 'milestone has issue "Bugfix2" with labels: "bug", "enhancement"' do
+    project = Project.find_by(name: "Shop")
+    milestone = project.milestones.find_by(title: 'v2.2')
+    issue = create(:issue, title: "Bugfix2", project: project, milestone: milestone)
+    issue.labels << project.labels.find_by(title: 'bug')
+    issue.labels << project.labels.find_by(title: 'enhancement')
+  end
+
+  step 'project "Shop" has milestone "v2.2"' do
+    project = Project.find_by(name: "Shop")
+    milestone = create(:milestone,
+                       title: "v2.2",
+                       project: project,
+                       description: "# Description header"
+                      )
+    3.times { create(:issue, project: project, milestone: milestone) }
+  end
+
+  step 'I should see the list of labels' do
+    expect(page).to have_selector('ul.manage-labels-list')
+  end
+
+  step 'I should see the labels "bug", "enhancement" and "feature"' do
+    page.within('#tab-issues') do
+      expect(page).to have_content 'bug'
+      expect(page).to have_content 'enhancement'
+      expect(page).to have_content 'feature'
+    end
+  end
+
+  step 'I click link "v2.2"' do
+    click_link "v2.2"
+  end
+
+  step 'I click link "Labels"' do
+    page.within('.nav-links') do
+      page.find(:xpath, "//a[@href='#tab-labels']").click
+    end
+  end
+end

features/steps/shared/builds.rb

@@ -42,6 +42,10 @@ module SharedBuilds
     @build.update_attributes(artifacts_metadata: gzip)
   end
+  step 'recent build has a build trace' do
+    @build.trace = 'build trace'
+  end
+
   step 'download of build artifacts archive starts' do
     expect(page.response_headers['Content-Type']).to eq 'application/zip'
     expect(page.response_headers['Content-Transfer-Encoding']).to eq 'binary'

lib/api/builds.rb

@@ -115,13 +115,33 @@ class Builds < Grape::API
         authorize_update_builds!
         build = get_build(params[:build_id])
-        return forbidden!('Build is not retryable') unless build && build.retryable?
+        return not_found!(build) unless build
+        return forbidden!('Build is not retryable') unless build.retryable?
         build = Ci::Build.retry(build)
         present build, with: Entities::Build,
                        user_can_download_artifacts: can?(current_user, :read_build, user_project)
       end
+
+      # Erase build (remove artifacts and build trace)
+      #
+      # Parameters:
+      #   id (required) - the id of a project
+      #   build_id (required) - the id of a build
+      # example Request:
+      #  post  /projects/:id/build/:build_id/erase
+      post ':id/builds/:build_id/erase' do
+        authorize_update_builds!
+
+        build = get_build(params[:build_id])
+        return not_found!(build) unless build
+        return forbidden!('Build is not erasable!') unless build.erasable?
+
+        build.erase(erased_by: current_user)
+        present build, with: Entities::Build,
+                       user_can_download_artifacts: can?(current_user, :download_build_artifacts, user_project)
+      end
     end
     helpers do

lib/ci/api/builds.rb

@@ -38,6 +38,8 @@ class Builds < Grape::API
           authenticate_runner!
           update_runner_last_contact
           build = Ci::Build.where(runner_id: current_runner.id).running.find(params[:id])
+          forbidden!('Build has been erased!') if build.erased?
+
           build.update_attributes(trace: params[:trace]) if params[:trace]
           case params[:state].to_s
@@ -99,6 +101,7 @@ class Builds < Grape::API
           not_found! unless build
           authenticate_build_token!(build)
           forbidden!('Build is not running!') unless build.running?
+          forbidden!('Build has been erased!') if build.erased?
           artifacts_upload_path = ArtifactUploader.artifacts_upload_path
           artifacts = uploaded_file(:file, artifacts_upload_path)
@@ -143,7 +146,7 @@ class Builds < Grape::API
           present_file!(artifacts_file.path, artifacts_file.filename)
         end
-        # Remove the artifacts file from build
+        # Remove the artifacts file from build - Runners only
         #
         # Parameters:
         #   id (required) - The ID of a build
@@ -156,6 +159,7 @@ class Builds < Grape::API
           build = Ci::Build.find_by_id(params[:id])
           not_found! unless build
           authenticate_build_token!(build)
+
           build.remove_artifacts_file!
           build.remove_artifacts_metadata!
         end

spec/controllers/commit_controller_spec.rb

@@ -143,4 +143,53 @@
       expect(assigns(:tags)).to include("v1.1.0")
     end
   end
+
+  describe '#revert' do
+    context 'when target branch is not provided' do
+      it 'should render the 404 page' do
+        post(:revert,
+            namespace_id: project.namespace.to_param,
+            project_id: project.to_param,
+            id: commit.id)
+
+        expect(response).not_to be_success
+        expect(response.status).to eq(404)
+      end
+    end
+
+    context 'when the revert was successful' do
+      it 'should redirect to the commits page' do
+        post(:revert,
+            namespace_id: project.namespace.to_param,
+            project_id: project.to_param,
+            target_branch: 'master',
+            id: commit.id)
+
+        expect(response).to redirect_to namespace_project_commits_path(project.namespace, project, 'master')
+        expect(flash[:notice]).to eq('The commit has been successfully reverted.')
+      end
+    end
+
+    context 'when the revert failed' do
+      before do
+        post(:revert,
+            namespace_id: project.namespace.to_param,
+            project_id: project.to_param,
+            target_branch: 'master',
+            id: commit.id)
+      end
+
+      it 'should redirect to the commit page' do
+        # Reverting a commit that has been already reverted.
+        post(:revert,
+            namespace_id: project.namespace.to_param,
+            project_id: project.to_param,
+            target_branch: 'master',
+            id: commit.id)
+
+        expect(response).to redirect_to namespace_project_commit_path(project.namespace, project, commit.id)
+        expect(flash[:alert]).to match('Sorry, we cannot revert this commit automatically.')
+      end
+    end
+  end
 end

spec/factories/ci/builds.rb

@@ -54,7 +54,7 @@
     end
     factory :ci_build_with_trace do
-      after(:create) do  |build, evaluator|
+      after(:create) do |build, evaluator|
         build.trace = 'BUILD TRACE'
       end
     end
@@ -62,14 +62,13 @@
     trait :artifacts do
       after(:create) do |build, _|
         build.artifacts_file =
-          fixture_file_upload(Rails.root +
-                              'spec/fixtures/ci_build_artifacts.zip',
-                              'application/zip')
+          fixture_file_upload(Rails.root.join('spec/fixtures/ci_build_artifacts.zip'),
+                             'application/zip')
         build.artifacts_metadata =
-          fixture_file_upload(Rails.root +
-                              'spec/fixtures/ci_build_artifacts_metadata.gz',
-                              'application/x-gzip')
+          fixture_file_upload(Rails.root.join('spec/fixtures/ci_build_artifacts_metadata.gz'),
+                             'application/x-gzip')
+        
         build.save!
       end
     end

spec/factories/merge_requests.rb

@@ -24,6 +24,7 @@
 #  merge_params              :text
 #  merge_when_build_succeeds :boolean          default(FALSE), not null
 #  merge_user_id             :integer
+#  merge_commit_sha          :string
 #
 FactoryGirl.define do

spec/lib/ci/status_spec.rb

@@ -2,36 +2,40 @@
 describe Ci::Status do
   describe '.get_status' do
-    subject { described_class.get_status(builds) }
-    context 'all builds successful' do
-      let(:builds) { Array.new(2) { create(:ci_build, :success) } }
-      it { is_expected.to eq 'success' }
-    end
-
-    context 'at least one build failed' do
-      let(:builds) { [create(:ci_build, :success), create(:ci_build, :failed)] }
-      it { is_expected.to eq 'failed' }
-    end
-    context 'at least one running' do
-      let(:builds) { [create(:ci_build, :success), create(:ci_build, :running)] }
-      it { is_expected.to eq 'running' }
-    end
-    context 'at least one pending' do
-      let(:builds) { [create(:ci_build, :success), create(:ci_build, :pending)] }
-      it { is_expected.to eq 'running' }
-    end
-    context 'build success and failed but allowed to fail' do
-      let(:builds) { [create(:ci_build, :success), create(:ci_build, :failed, :allowed_to_fail)] }
-      it { is_expected.to eq 'success' }
-    end
-    context 'one build failed but allowed to fail' do
-      let(:builds) { [create(:ci_build, :failed, :allowed_to_fail)] }
-      it { is_expected.to eq 'success' }
+    subject { described_class.get_status(statuses) }
+    
+    [:ci_build, :generic_commit_status].each do |type|
+      context "for #{type}" do
+        context 'all successful' do
+          let(:statuses) { Array.new(2) { create(type, status: :success) } }
+          it { is_expected.to eq 'success' }
+        end
+        context 'at least one failed' do
+          let(:statuses) { [create(type, status: :success), create(type, status: :failed)] }
+          it { is_expected.to eq 'failed' }
+        end
+        context 'at least one running' do
+          let(:statuses) { [create(type, status: :success), create(type, status: :running)] }
+          it { is_expected.to eq 'running' }
+        end
+        context 'at least one pending' do
+          let(:statuses) { [create(type, status: :success), create(type, status: :pending)] }
+          it { is_expected.to eq 'running' }
+        end
+        context 'success and failed but allowed to fail' do
+          let(:statuses) { [create(type, status: :success), create(type, status: :failed, allow_failure: true)] }
+          it { is_expected.to eq 'success' }
+        end
+        context 'one failed but allowed to fail' do
+          let(:statuses) { [create(type, status: :failed, allow_failure: true)] }
+          it { is_expected.to eq 'success' }
+        end
+      end
     end
   end
 end

spec/models/build_spec.rb

@@ -346,15 +346,14 @@
   describe :artifacts_download_url do
     subject { build.artifacts_download_url }
-    it "should be nil if artifact doesn't exist" do
-      build.update_attributes(artifacts_file: nil)
-      is_expected.to be_nil
+    context 'artifacts file does not exist' do
+      before { build.update_attributes(artifacts_file: nil) }
+      it { is_expected.to be_nil }
     end
-    it 'should not be nil if artifact exist' do
-      gif = fixture_file_upload(Rails.root + 'spec/fixtures/banana_sample.gif', 'image/gif')
-      build.update_attributes(artifacts_file: gif)
-      is_expected.to_not be_nil
+    context 'artifacts file exists' do
+      let(:build) { create(:ci_build, :artifacts) }
+      it { is_expected.to_not be_nil }
     end
   end
@@ -381,11 +380,7 @@
     end
     context 'artifacts archive exists' do
-      before do
-        gif = fixture_file_upload(Rails.root + 'spec/fixtures/banana_sample.gif', 'image/gif')
-        build.update_attributes(artifacts_file: gif)
-      end
-
+      let(:build) { create(:ci_build, :artifacts) }
       it { is_expected.to be_truthy }
     end
   end
@@ -398,16 +393,7 @@
     end
     context 'artifacts archive is a zip file and metadata exists' do
-      before do
-        fixture_dir = Rails.root + 'spec/fixtures/'
-        archive = fixture_file_upload(fixture_dir + 'ci_build_artifacts.zip',
-                                      'application/zip')
-        metadata = fixture_file_upload(fixture_dir + 'ci_build_artifacts_metadata.gz',
-                                       'application/x-gzip')
-        build.update_attributes(artifacts_file: archive)
-        build.update_attributes(artifacts_metadata: metadata)
-      end
-
+      let(:build) { create(:ci_build, :artifacts) }
       it { is_expected.to be_truthy }
     end
   end
@@ -511,6 +497,103 @@ def create_mr(build, commit, factory: :merge_request, created_at: Time.now)
         expect(@build2.merge_request.id).to eq(@merge_request.id)
       end
     end
+  end
+
+  describe 'build erasable' do
+    shared_examples 'erasable' do
+      it 'should remove artifact file' do
+        expect(build.artifacts_file.exists?).to be_falsy
+      end
+
+      it 'should remove artifact metadata file' do
+        expect(build.artifacts_metadata.exists?).to be_falsy
+      end
+
+      it 'should erase build trace in trace file' do
+        expect(build.trace).to be_empty
+      end
+
+      it 'should set erased to true' do
+        expect(build.erased?).to be true
+      end
+
+      it 'should set erase date' do
+        expect(build.erased_at).to_not be_falsy
+      end
+    end
+
+    context 'build is not erasable' do
+      let!(:build) { create(:ci_build) }
+
+      describe '#erase' do
+        subject { build.erase }
+
+        it { is_expected.to be false }
+      end
+
+      describe '#erasable?' do
+        subject { build.erasable? }
+        it { is_expected.to eq false }
+      end
+    end
+
+    context 'build is erasable' do
+      let!(:build) { create(:ci_build_with_trace, :success, :artifacts) }
+
+      describe '#erase' do
+        before { build.erase(erased_by: user) }
+
+        context 'erased by user' do
+          let!(:user) { create(:user, username: 'eraser') }
+
+          include_examples 'erasable'
+
+          it 'should record user who erased a build' do
+            expect(build.erased_by).to eq user
+          end
+        end
+
+        context 'erased by system' do
+          let(:user) { nil }
+
+          include_examples 'erasable'
+
+          it 'should not set user who erased a build' do
+            expect(build.erased_by).to be_nil
+          end
+        end
+      end
+      describe '#erasable?' do
+        subject { build.erasable? }
+        it { is_expected.to eq true }
+      end
+
+      describe '#erased?' do
+        let!(:build) { create(:ci_build_with_trace, :success, :artifacts) }
+        subject { build.erased? }
+
+        context 'build has not been erased' do
+          it { is_expected.to be false }
+        end
+
+        context 'build has been erased' do
+          before { build.erase }
+
+          it { is_expected.to be true }
+        end
+      end
+
+      context 'metadata and build trace are not available' do
+        let!(:build) { create(:ci_build, :success, :artifacts) }
+        before { build.remove_artifacts_metadata! }
+
+        describe '#erase' do
+          it 'should not raise error' do
+            expect { build.erase }.to_not raise_error
+          end
+        end
+      end
+    end
   end
 end

spec/models/merge_request_spec.rb

@@ -24,6 +24,7 @@
 #  merge_params              :text
 #  merge_when_build_succeeds :boolean          default(FALSE), not null
 #  merge_user_id             :integer
+#  merge_commit_sha          :string
 #
 require 'spec_helper'

spec/models/repository_spec.rb

@@ -5,6 +5,15 @@
   let(:repository) { create(:project).repository }
   let(:user) { create(:user) }
+  let(:commit_options) do
+    author = repository.user_to_committer(user)
+    { message: 'Test message', committer: author, author: author }
+  end
+  let(:merge_commit) do
+    source_sha = repository.find_branch('feature').target
+    merge_commit_id = repository.merge(user, source_sha, 'master', commit_options)
+    repository.commit(merge_commit_id)
+  end
   describe :branch_names_contains do
     subject { repository.branch_names_contains(sample_commit.id) }
@@ -427,6 +436,21 @@
     it { is_expected.not_to include('e56497bb5f03a90a51293fc6d516788730953899') }
   end
+  describe '#merge' do
+    it 'should merge the code and return the commit id' do
+      expect(merge_commit).to be_present
+      expect(repository.blob_at(merge_commit.id, 'files/ruby/feature.rb')).to be_present
+    end
+  end
+
+  describe '#revert_merge' do
+    it 'should revert the changes' do
+      repository.revert(user, merge_commit, 'master')
+
+      expect(repository.blob_at_branch('master', 'files/ruby/feature.rb')).not_to be_present
+    end
+  end
+
   describe "Elastic search", elastic: true do
     before do
       Repository.__elasticsearch__.create_index!

spec/requests/api/builds_spec.rb

@@ -169,4 +169,34 @@
       end
     end
   end
+
+  describe 'POST /projects/:id/builds/:build_id/erase' do
+    before do
+      post api("/projects/#{project.id}/builds/#{build.id}/erase", user)
+    end
+
+    context 'build is erasable' do
+      let(:build) { create(:ci_build_with_trace, :artifacts, :success, project: project, commit: commit) }
+
+      it 'should erase build content' do
+        expect(response.status).to eq 201
+        expect(build.trace).to be_empty
+        expect(build.artifacts_file.exists?).to be_falsy
+        expect(build.artifacts_metadata.exists?).to be_falsy
+      end
+
+      it 'should update build' do
+        expect(build.reload.erased_at).to be_truthy
+        expect(build.reload.erased_by).to eq user
+      end
+    end
+
+    context 'build is not erasable' do
+      let(:build) { create(:ci_build_with_trace, project: project, commit: commit) }
+
+      it 'should respond with forbidden' do
+        expect(response.status).to eq 403
+      end
+    end
+  end
 end

spec/requests/ci/api/builds_spec.rb

@@ -131,20 +131,28 @@
     end
     describe "PUT /builds/:id" do
-      let(:commit) { FactoryGirl.create(:ci_commit, project: project)}
-      let(:build) { FactoryGirl.create(:ci_build, commit: commit, runner_id: runner.id) }
-      it "should update a running build" do
+      let(:commit) {create(:ci_commit, project: project)}
+      let(:build) { create(:ci_build_with_trace, commit: commit, runner_id: runner.id) }
+      before do
         build.run!
         put ci_api("/builds/#{build.id}"), token: runner.token
+      end
+
+      it "should update a running build" do
         expect(response.status).to eq(200)
       end
-      it 'Should not override trace information when no trace is given' do
-        build.run!
-        build.update!(trace: 'hello_world')
-        put ci_api("/builds/#{build.id}"), token: runner.token
-        expect(build.reload.trace).to eq 'hello_world'
+      it 'should not override trace information when no trace is given' do
+        expect(build.reload.trace).to eq 'BUILD TRACE'
+      end
+
+      context 'build has been erased' do
+        let(:build) { create(:ci_build, runner_id: runner.id, erased_at: Time.now) }
+
+        it 'should respond with forbidden' do
+          expect(response.status).to eq 403
+        end
       end
     end
@@ -191,9 +199,10 @@
           end
         end
-        context 'token is invalid' do
-          it 'should respond with forbidden'do
-            post authorize_url, { token: 'invalid', filesize: 100 }
+        context 'authorization token is invalid' do
+          before { post authorize_url, { token: 'invalid', filesize: 100 } }
+
+          it 'should respond with forbidden' do
             expect(response.status).to eq(403)
           end
         end
@@ -206,6 +215,15 @@
             allow(ArtifactUploader).to receive(:artifacts_upload_path).and_return('/')
           end
+          context 'build has been erased' do
+            let(:build) { create(:ci_build, erased_at: Time.now) }
+            before { upload_artifacts(file_upload, headers_with_token) }
+
+            it 'should respond with forbidden' do
+              expect(response.status).to eq 403
+            end
+          end
+
           context "should post artifact to running build" do
             it "uses regual file post" do
               upload_artifacts(file_upload, headers_with_token, false)
@@ -234,7 +252,9 @@
             let(:stored_artifacts_file) { build.reload.artifacts_file.file }
             let(:stored_metadata_file) { build.reload.artifacts_metadata.file }
-            before { post(post_url, post_data, headers_with_token) }
+            before do
+              post(post_url, post_data, headers_with_token)
+            end
             context 'post data accelerated by workhorse is correct' do
               let(:post_data) do