Add latest changes from gitlab-org/gitlab@master

GITLAB_SHELL_VERSION

-10.2.0
+10.3.0

app/assets/javascripts/diffs/components/app.vue

@@ -144,6 +144,9 @@ export default {
     isLimitedContainer() {
       return !this.showTreeList && !this.isParallelView && !this.isFluidLayout;
     },
+    shouldSetDiscussions() {
+      return this.isNotesFetched && !this.assignedDiscussions && !this.isLoading;
+    },
   },
   watch: {
     diffViewType() {
@@ -160,6 +163,11 @@ export default {
     },
     isLoading: 'adjustView',
     showTreeList: 'adjustView',
+    shouldSetDiscussions(newVal) {
+      if (newVal) {
+        this.setDiscussions();
+      }
+    },
   },
   mounted() {
     this.setBaseConfig({
@@ -214,26 +222,28 @@ export default {
     isLatestVersion() {
       return window.location.search.indexOf('diff_id') === -1;
     },
+    startDiffRendering() {
+      requestIdleCallback(
+        () => {
+          this.startRenderDiffsQueue();
+        },
+        { timeout: 1000 },
+      );
+    },
     fetchData(toggleTree = true) {
       if (this.isLatestVersion() && this.glFeatures.diffsBatchLoad) {
         this.fetchDiffFilesMeta()
           .then(() => {
             if (toggleTree) this.hideTreeListIfJustOneFile();
+
+            this.startDiffRendering();
           })
           .catch(() => {
             createFlash(__('Something went wrong on our end. Please try again!'));
           });
 
         this.fetchDiffFilesBatch()
-          .then(() => {
-            requestIdleCallback(
-              () => {
-                this.setDiscussions();
-                this.startRenderDiffsQueue();
-              },
-              { timeout: 1000 },
-            );
-          })
+          .then(() => this.startDiffRendering())
           .catch(() => {
             createFlash(__('Something went wrong on our end. Please try again!'));
           });
@@ -246,7 +256,6 @@ export default {
 
             requestIdleCallback(
               () => {
-                this.setDiscussions();
                 this.startRenderDiffsQueue();
               },
               { timeout: 1000 },
@@ -262,7 +271,7 @@ export default {
       }
     },
     setDiscussions() {
-      if (this.isNotesFetched && !this.assignedDiscussions && !this.isLoading) {
+      if (this.shouldSetDiscussions) {
         this.assignedDiscussions = true;
 
         requestIdleCallback(

app/assets/javascripts/diffs/store/actions.js

@@ -119,7 +119,7 @@ export const fetchDiffFilesMeta = ({ commit, state }) => {
     .get(state.endpointMetadata)
     .then(({ data }) => {
       const strippedData = { ...data };
-      strippedData.diff_files = [];
+      delete strippedData.diff_files;
       commit(types.SET_LOADING, false);
       commit(types.SET_MERGE_REQUEST_DIFFS, data.merge_request_diffs || []);
       commit(types.SET_DIFF_DATA, strippedData);

app/assets/javascripts/diffs/store/mutations.js

@@ -39,7 +39,16 @@ export default {
   },
 
   [types.SET_DIFF_DATA](state, data) {
-    prepareDiffData(data);
+    if (
+      !(
+        gon &&
+        gon.features &&
+        gon.features.diffsBatchLoad &&
+        window.location.search.indexOf('diff_id') === -1
+      )
+    ) {
+      prepareDiffData(data);
+    }
 
     Object.assign(state, {
       ...convertObjectPropsToCamelCase(data),

app/assets/javascripts/pipelines/components/test_reports/test_summary.vue

@@ -28,7 +28,9 @@ export default {
       return this.report.name || __('Summary');
     },
     successPercentage() {
-      return Math.round((this.report.success_count / this.report.total_count) * 100) || 0;
+      // Returns a full number when the decimals equal .00.
+      // Otherwise returns a float to two decimal points
+      return Number(((this.report.success_count / this.report.total_count) * 100 || 0).toFixed(2));
     },
     formattedDuration() {
       return formatTime(secondsToMilliseconds(this.report.total_time));

app/models/issue/metrics.rb

@@ -3,6 +3,12 @@
 class Issue::Metrics < ApplicationRecord
   belongs_to :issue
 
+  scope :for_issues, ->(issues) { where(issue: issues) }
+  scope :with_first_mention_not_earlier_than, -> (timestamp) {
+    where(first_mentioned_in_commit_at: nil)
+      .or(where(arel_table['first_mentioned_in_commit_at'].gteq(timestamp)))
+  }
+
   def record!
     if issue.milestone_id.present? && self.first_associated_with_milestone_at.blank?
       self.first_associated_with_milestone_at = Time.now

app/workers/process_commit_worker.rb

@@ -55,16 +55,15 @@ class ProcessCommitWorker
     end
   end
 
-  # rubocop: disable CodeReuse/ActiveRecord
   def update_issue_metrics(commit, author)
     mentioned_issues = commit.all_references(author).issues
 
     return if mentioned_issues.empty?
 
-    Issue::Metrics.where(issue_id: mentioned_issues.map(&:id), first_mentioned_in_commit_at: nil)
+    Issue::Metrics.for_issues(mentioned_issues)
+      .with_first_mention_not_earlier_than(commit.committed_date)
       .update_all(first_mentioned_in_commit_at: commit.committed_date)
   end
-  # rubocop: enable CodeReuse/ActiveRecord
 
   def build_commit(project, hash)
     date_suffix = '_date'

changelogs/unreleased/16050-improve-first-mentioned-issue-metric-calc.yml

+---
+title: Adjust issue metrics first_mentioned_in_commit_at calculation
+merge_request: 20923
+author:
+type: fixed

changelogs/unreleased/37363-junit-xml-summary-incorrectly-shows-100-success-rate.yml

+---
+title: Junit success percentage no longer displays 100% if there are failures
+merge_request: 20835
+author:
+type: fixed

changelogs/unreleased/37371-update-gitlab-shell.yml

+---
+title: Update GitLab Shell to v10.3.0
+merge_request: 21151
+author:
+type: other

spec/frontend/pipelines/test_reports/test_summary_spec.js

@@ -79,4 +79,25 @@ describe('Test reports summary', () => {
       expect(duration().text()).toBe('00:00:00');
     });
   });
+
+  describe('success percentage calculation', () => {
+    it.each`
+      name                                     | successCount | totalCount | result
+      ${'displays 0 when there are no tests'}  | ${0}         | ${0}       | ${'0'}
+      ${'displays whole number when possible'} | ${10}        | ${50}      | ${'20'}
+      ${'rounds to 0.01'}                      | ${1}         | ${16604}   | ${'0.01'}
+      ${'correctly rounds to 50'}              | ${8302}      | ${16604}   | ${'50'}
+      ${'rounds down for large close numbers'} | ${16603}     | ${16604}   | ${'99.99'}
+      ${'correctly displays 100'}              | ${16604}     | ${16604}   | ${'100'}
+    `('$name', ({ successCount, totalCount, result }) => {
+      createComponent({
+        report: {
+          success_count: successCount,
+          total_count: totalCount,
+        },
+      });
+
+      expect(successRate().text()).toBe(`${result}% success rate`);
+    });
+  });
 });

spec/javascripts/diffs/components/app_spec.js

@@ -77,16 +77,17 @@ describe('diffs/components/app', () => {
       spyOn(wrapper.vm, 'startRenderDiffsQueue');
     });
 
-    it('calls fetchDiffFiles if diffsBatchLoad is not enabled', () => {
+    it('calls fetchDiffFiles if diffsBatchLoad is not enabled', done => {
       wrapper.vm.glFeatures.diffsBatchLoad = false;
       wrapper.vm.fetchData(false);
 
       expect(wrapper.vm.fetchDiffFiles).toHaveBeenCalled();
-      wrapper.vm.$nextTick(() => {
-        expect(wrapper.vm.setDiscussions).toHaveBeenCalled();
+      setTimeout(() => {
         expect(wrapper.vm.startRenderDiffsQueue).toHaveBeenCalled();
         expect(wrapper.vm.fetchDiffFilesMeta).not.toHaveBeenCalled();
         expect(wrapper.vm.fetchDiffFilesBatch).not.toHaveBeenCalled();
+
+        done();
       });
     });
 
@@ -97,7 +98,6 @@ describe('diffs/components/app', () => {
 
       expect(wrapper.vm.fetchDiffFiles).toHaveBeenCalled();
       wrapper.vm.$nextTick(() => {
-        expect(wrapper.vm.setDiscussions).toHaveBeenCalled();
         expect(wrapper.vm.startRenderDiffsQueue).toHaveBeenCalled();
         expect(wrapper.vm.fetchDiffFilesMeta).not.toHaveBeenCalled();
         expect(wrapper.vm.fetchDiffFilesBatch).not.toHaveBeenCalled();
@@ -110,7 +110,6 @@ describe('diffs/components/app', () => {
 
       expect(wrapper.vm.fetchDiffFiles).not.toHaveBeenCalled();
       wrapper.vm.$nextTick(() => {
-        expect(wrapper.vm.setDiscussions).toHaveBeenCalled();
         expect(wrapper.vm.startRenderDiffsQueue).toHaveBeenCalled();
         expect(wrapper.vm.fetchDiffFilesMeta).toHaveBeenCalled();
         expect(wrapper.vm.fetchDiffFilesBatch).toHaveBeenCalled();

spec/javascripts/diffs/store/actions_spec.js

@@ -186,7 +186,7 @@ describe('DiffsStoreActions', () => {
           { type: types.SET_LOADING, payload: true },
           { type: types.SET_LOADING, payload: false },
           { type: types.SET_MERGE_REQUEST_DIFFS, payload: [] },
-          { type: types.SET_DIFF_DATA, payload: { data, diff_files: [] } },
+          { type: types.SET_DIFF_DATA, payload: { data } },
         ],
         [],
         () => {

spec/lib/gitlab/auth_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-describe Gitlab::Auth do
+describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
   let(:gl_auth) { described_class }
   set(:project) { create(:project) }
 

spec/models/issue/metrics_spec.rb

@@ -7,6 +7,33 @@ describe Issue::Metrics do
 
   subject { create(:issue, project: project) }
 
+  describe '.for_issues' do
+    subject(:scope) { described_class.for_issues([issue1, issue2]) }
+
+    let(:issue1) { create(:issue) }
+    let(:issue2) { create(:issue) }
+
+    it 'returns metrics associated with given issues' do
+      create(:issue)
+
+      expect(scope).to match_array([issue1.metrics, issue2.metrics])
+    end
+  end
+
+  describe '.with_first_mention_not_earlier_than' do
+    subject(:scope) { described_class.with_first_mention_not_earlier_than(timestamp) }
+
+    let(:timestamp) { DateTime.now }
+
+    it 'returns metrics without mentioning in commit or with mentioning after given timestamp' do
+      issue1 = create(:issue)
+      issue2 = create(:issue).tap { |i| i.metrics.update!(first_mentioned_in_commit_at: timestamp + 1.day) }
+      create(:issue).tap { |i| i.metrics.update!(first_mentioned_in_commit_at: timestamp - 1.day) }
+
+      expect(scope).to match_array([issue1.metrics, issue2.metrics])
+    end
+  end
+
   describe "when recording the default set of issue metrics on issue save" do
     context "milestones" do
       it "records the first time an issue is associated with a milestone" do

spec/workers/process_commit_worker_spec.rb

@@ -129,21 +129,54 @@ describe ProcessCommitWorker do
   end
 
   describe '#update_issue_metrics' do
-    it 'updates any existing issue metrics' do
-      allow(commit).to receive(:safe_message).and_return("Closes #{issue.to_reference}")
+    context 'when commit has issue reference' do
+      subject(:update_metrics_and_reload) do
+        -> {
+          worker.update_issue_metrics(commit, user)
+          issue.metrics.reload
+        }
+      end
+
+      before do
+        allow(commit).to receive(:safe_message).and_return("Closes #{issue.to_reference}")
+      end
 
-      worker.update_issue_metrics(commit, user)
+      context 'when issue has no first_mentioned_in_commit_at set' do
+        it 'updates issue metrics' do
+          expect(update_metrics_and_reload)
+            .to change { issue.metrics.first_mentioned_in_commit_at }.to(commit.committed_date)
+        end
+      end
 
-      metric = Issue::Metrics.first
+      context 'when issue has first_mentioned_in_commit_at earlier than given committed_date' do
+        before do
+          issue.metrics.update(first_mentioned_in_commit_at: commit.committed_date - 1.day)
+        end
 
-      expect(metric.first_mentioned_in_commit_at).to eq(commit.committed_date)
+        it "doesn't update issue metrics" do
+          expect(update_metrics_and_reload).not_to change { issue.metrics.first_mentioned_in_commit_at }
+        end
+      end
+
+      context 'when issue has first_mentioned_in_commit_at later than given committed_date' do
+        before do
+          issue.metrics.update(first_mentioned_in_commit_at: commit.committed_date + 1.day)
+        end
+
+        it "doesn't update issue metrics" do
+          expect(update_metrics_and_reload)
+            .to change { issue.metrics.first_mentioned_in_commit_at }.to(commit.committed_date)
+        end
+      end
     end
 
-    it "doesn't execute any queries with false conditions" do
-      allow(commit).to receive(:safe_message).and_return("Lorem Ipsum")
+    context 'when commit has no issue references' do
+      it "doesn't execute any queries with false conditions" do
+        allow(commit).to receive(:safe_message).and_return("Lorem Ipsum")
 
-      expect { worker.update_issue_metrics(commit, user) }
-        .not_to make_queries_matching(/WHERE (?:1=0|0=1)/)
+        expect { worker.update_issue_metrics(commit, user) }
+          .not_to make_queries_matching(/WHERE (?:1=0|0=1)/)
+      end
     end
   end