Add tests to verify the correctness of returned environments

CHANGELOG

@@ -24,6 +24,7 @@ v 8.12.0 (unreleased)
   - Cycle analytics (first iteration) !5986
   - Remove vendor prefixes for linear-gradient CSS (ClemMakesApps)
   - Move pushes_since_gc from the database to Redis
+  - Limit number of shown environments on Merge Request: show only environments for target_branch, source_branch and tags
   - Add font color contrast to external label in admin area (ClemMakesApps)
   - Change logo animation to CSS (ClemMakesApps)
   - Instructions for enabling Git packfile bitmaps !6104

app/models/merge_request.rb

@@ -672,10 +672,10 @@ class MergeRequest < ActiveRecord::Base
     environments = source_project.environments_for(
       source_branch, diff_head_commit)
-    environments <<= target_project.environments_for(
-      source_branch, diff_head_commit, with_tags: true)
-    environments
+    environments += target_project.environments_for(
+      target_branch, diff_head_commit, with_tags: true)
+    environments.uniq
   end
   def state_human_name

app/models/project.rb

@@ -1304,7 +1304,7 @@ class Project < ActiveRecord::Base
         environment_ids.where(ref: ref)
       end
-    Environment.where(id: environment_ids).map do |environment|
+    Environment.where(id: environment_ids).select do |environment|
       environment.includes_commit?(commit)
     end
   end

app/views/projects/merge_requests/widget/_heading.html.haml

@@ -43,7 +43,7 @@
       = icon("times-circle")
       Could not connect to the CI server. Please check your settings and try again.
-- @merge_request.environments.each do |environment|
+- @merge_request.environments.sort_by(&:name).each do |environment|
   - if can?(current_user, :read_environment, environment)
     .mr-widget-heading
       .ci_widget.ci-success

spec/models/merge_request_spec.rb

@@ -701,16 +701,56 @@ describe MergeRequest, models: true do
     end
   end
-  describe "#environments" do
+  describe '#environments' do
     let(:project)       { create(:project) }
     let(:merge_request) { create(:merge_request, source_project: project) }
-    it 'selects deployed environments' do
-      environments = create_list(:environment, 3, project: project)
-      create(:deployment, environment: environments.first, sha: project.commit('master').id)
-      create(:deployment, environment: environments.second, sha: project.commit('feature').id)
-      expect(merge_request.environments).to eq [environments.first]
+    context 'with multiple environments' do
+      let(:environments) { create_list(:environment, 3, project: project) }
+      before do
+        create(:deployment, environment: environments.first, ref: 'master', sha: project.commit('master').id)
+        create(:deployment, environment: environments.second, ref: 'feature', sha: project.commit('feature').id)
+      end
+
+      it 'selects deployed environments' do
+        expect(merge_request.environments).to contain_exactly(environments.first)
+      end
+    end
+
+    context 'with environments on source project' do
+      let(:source_project) do
+        create(:project) do |fork_project|
+          fork_project.create_forked_project_link(forked_to_project_id: fork_project.id, forked_from_project_id: project.id)
+        end
+      end
+      let(:merge_request) do
+        create(:merge_request,
+               source_project: source_project, source_branch: 'feature',
+               target_project: project)
+      end
+
+      let(:source_environment) { create(:environment, project: source_project) }
+
+      before do
+        create(:deployment, environment: source_environment, ref: 'feature', sha: merge_request.diff_head_sha)
+      end
+
+      it 'selects deployed environments' do
+        expect(merge_request.environments).to contain_exactly(source_environment)
+      end
+
+      context 'with environments on target project' do
+        let(:target_environment) { create(:environment, project: project) }
+
+        before do
+          create(:deployment, environment: target_environment, tag: true, sha: merge_request.diff_head_sha)
+        end
+
+        it 'selects deployed environments' do
+          expect(merge_request.environments).to contain_exactly(source_environment, target_environment)
+        end
+      end
     end
     context 'without a diff_head_commit' do

spec/models/project_spec.rb

@@ -1647,6 +1647,47 @@ describe Project, models: true do
     end
   end
+  describe '#environments_for' do
+    let(:project) { create(:project) }
+    let(:environment) { create(:environment, project: project) }
+
+    context 'tagged deployment' do
+      before do
+        create(:deployment, environment: environment, ref: '1.0', tag: true, sha: project.commit.id)
+      end
+
+      it 'returns environment when with_tags is set' do
+        expect(project.environments_for('master', project.commit, with_tags: true)).to contain_exactly(environment)
+      end
+
+      it 'does not return environment when no with_tags is set' do
+        expect(project.environments_for('master', project.commit)).not_to contain_exactly(environment)
+      end
+
+      it 'does not return environment when commit is not part of deployment' do
+        expect(project.environments_for('master', project.commit('feature'))).not_to contain_exactly(environment)
+      end
+    end
+
+    context 'branch deployment' do
+      before do
+        create(:deployment, environment: environment, ref: 'master', sha: project.commit.id)
+      end
+
+      it 'returns environment when ref is set' do
+        expect(project.environments_for('master', project.commit)).to contain_exactly(environment)
+      end
+
+      it 'does not environment when ref is different' do
+        expect(project.environments_for('feature', project.commit)).not_to contain_exactly(environment)
+      end
+
+      it 'does not return environment when commit is not part of deployment' do
+        expect(project.environments_for('master', project.commit('feature'))).not_to contain_exactly(environment)
+      end
+    end
+  end
+
   def enable_lfs
     allow(Gitlab.config.lfs).to receive(:enabled).and_return(true)
   end