Address feedback

7 files changed, 132 insertions, 120 deletions

diff --git a/spec/features/projects/view_on_env_spec.rb b/spec/features/projects/view_on_env_spec.rb
index 552705661a9..ce5c5f21167 100644
--- a/spec/features/projects/view_on_env_spec.rb
+++ b/spec/features/projects/view_on_env_spec.rb
@@ -5,11 +5,11 @@ describe 'View on environment', js: true do
 
   let(:branch_name) { 'feature' }
   let(:file_path) { 'files/ruby/feature.rb' }
-  let(:project) { create(:project) }
+  let(:project) { create(:project, :repository) }
   let(:user) { project.creator }
 
   before do
-    project.team << [user, :master]
+    project.add_master(user)
   end
 
   context 'when the branch has a route map' do
@@ -24,7 +24,7 @@ describe 'View on environment', js: true do
       Files::CreateService.new(
         project,
         user,
-        source_branch: branch_name,
+        start_branch: branch_name,
         target_branch: branch_name,
         commit_message: "Add .gitlab/route-map.yml",
         file_path: '.gitlab/route-map.yml',
@@ -35,7 +35,7 @@ describe 'View on environment', js: true do
       Files::UpdateService.new(
         project,
         user,
-        source_branch: branch_name,
+        start_branch: branch_name,
         target_branch: branch_name,
         commit_message: "Update feature",
         file_path: file_path,
diff --git a/spec/finders/environments_finder_spec.rb b/spec/finders/environments_finder_spec.rb
new file mode 100644
index 00000000000..6e7e899abed
--- /dev/null
+++ b/spec/finders/environments_finder_spec.rb
@@ -0,0 +1,110 @@
+require 'spec_helper'
+
+describe EnvironmentsFinder do
+  describe '#execute' do
+    let(:project) { create(:project, :repository) }
+    let(:user) { project.creator }
+    let(:environment) { create(:environment, project: project) }
+
+    before do
+      project.add_master(user)
+    end
+
+    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(EnvironmentsFinder.new(project, user, ref: 'master', commit: project.commit, with_tags: true).execute)
+          .to contain_exactly(environment)
+      end
+
+      it 'does not return environment when no with_tags is set' do
+        expect(EnvironmentsFinder.new(project, user, ref: 'master', commit: project.commit).execute)
+          .to be_empty
+      end
+
+      it 'does not return environment when commit is not part of deployment' do
+        expect(EnvironmentsFinder.new(project, user, ref: 'master', commit: project.commit('feature')).execute)
+          .to be_empty
+      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(EnvironmentsFinder.new(project, user, ref: 'master', commit: project.commit).execute)
+          .to contain_exactly(environment)
+      end
+
+      it 'does not environment when ref is different' do
+        expect(EnvironmentsFinder.new(project, user, ref: 'feature', commit: project.commit).execute)
+          .to be_empty
+      end
+
+      it 'does not return environment when commit is not part of deployment' do
+        expect(EnvironmentsFinder.new(project, user, ref: 'master', commit: project.commit('feature')).execute)
+          .to be_empty
+      end
+
+      it 'returns environment when commit constraint is not set' do
+        expect(EnvironmentsFinder.new(project, user, ref: 'master').execute)
+          .to contain_exactly(environment)
+      end
+    end
+
+    context 'commit deployment' do
+      before do
+        create(:deployment, environment: environment, ref: 'master', sha: project.commit.id)
+      end
+
+      it 'returns environment' do
+        expect(EnvironmentsFinder.new(project, user, commit: project.commit).execute)
+          .to contain_exactly(environment)
+      end
+    end
+
+    context 'recently updated' do
+      context 'when last deployment to environment is the most recent one' do
+        before do
+          create(:deployment, environment: environment, ref: 'feature')
+        end
+
+        it 'finds recently updated environment' do
+          expect(EnvironmentsFinder.new(project, user, ref: 'feature', recently_updated: true).execute)
+            .to contain_exactly(environment)
+        end
+      end
+
+      context 'when last deployment to environment is not the most recent' do
+        before do
+          create(:deployment, environment: environment, ref: 'feature')
+          create(:deployment, environment: environment, ref: 'master')
+        end
+
+        it 'does not find environment' do
+          expect(EnvironmentsFinder.new(project, user, ref: 'feature', recently_updated: true).execute)
+            .to be_empty
+        end
+      end
+
+      context 'when there are two environments that deploy to the same branch' do
+        let(:second_environment) { create(:environment, project: project) }
+
+        before do
+          create(:deployment, environment: environment, ref: 'feature')
+          create(:deployment, environment: second_environment, ref: 'feature')
+        end
+
+        it 'finds both environments' do
+          expect(EnvironmentsFinder.new(project, user, ref: 'feature', recently_updated: true).execute)
+            .to contain_exactly(environment, second_environment)
+        end
+      end
+    end
+  end
+end
diff --git a/spec/helpers/commits_helper_spec.rb b/spec/helpers/commits_helper_spec.rb
index 85c663030d8..a2c008790f9 100644
--- a/spec/helpers/commits_helper_spec.rb
+++ b/spec/helpers/commits_helper_spec.rb
@@ -27,7 +27,7 @@ describe CommitsHelper do
     end
   end
 
-  describe '#view_on_environment_btn' do
+  describe '#view_on_environment_button' do
     let(:project) { create(:empty_project) }
     let(:environment) { create(:environment, external_url: 'http://example.com') }
     let(:path) { 'source/file.html' }
@@ -38,7 +38,7 @@ describe CommitsHelper do
     end
 
     it 'returns a link tag linking to the file in the environment' do
-      html = helper.view_on_environment_btn(sha, path, environment)
+      html = helper.view_on_environment_button(sha, path, environment)
       node = Nokogiri::HTML.parse(html).at_css('a')
 
       expect(node[:title]).to eq('View on example.com')
diff --git a/spec/lib/gitlab/route_map_spec.rb b/spec/lib/gitlab/route_map_spec.rb
index d56380bec50..459fa8a63a9 100644
--- a/spec/lib/gitlab/route_map_spec.rb
+++ b/spec/lib/gitlab/route_map_spec.rb
@@ -54,7 +54,8 @@ describe Gitlab::RouteMap, lib: true do
     context 'when all is good' do
       it 'returns a route map' do
         route_map = described_class.new(YAML.dump([{ 'source' => '/index\.html/', 'public' => 'index.html' }]))
-        expect(route_map.map).to eq([{ source: /^index\.html$/, public: 'index.html' }])
+        
+        expect(route_map.public_path_for_source_path('index.html')).to eq('index.html')
       end
     end
   end
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 32ed1e96749..e1e99300489 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -1005,10 +1005,16 @@ describe MergeRequest, models: true do
     end
   end
 
-  describe "#environments" do
+  describe "#environments_for" do
     let(:project)       { create(:project, :repository) }
+    let(:user)          { project.creator }
     let(:merge_request) { create(:merge_request, source_project: project) }
 
+    before do
+      merge_request.source_project.add_master(user)
+      merge_request.target_project.add_master(user)
+    end
+
     context 'with multiple environments' do
       let(:environments) { create_list(:environment, 3, project: project) }
 
@@ -1018,7 +1024,7 @@ describe MergeRequest, models: true do
       end
 
       it 'selects deployed environments' do
-        expect(merge_request.environments).to contain_exactly(environments.first)
+        expect(merge_request.environments_for(user)).to contain_exactly(environments.first)
       end
     end
 
@@ -1042,7 +1048,7 @@ describe MergeRequest, models: true do
       end
 
       it 'selects deployed environments' do
-        expect(merge_request.environments).to contain_exactly(source_environment)
+        expect(merge_request.environments_for(user)).to contain_exactly(source_environment)
       end
 
       context 'with environments on target project' do
@@ -1053,7 +1059,7 @@ describe MergeRequest, models: true do
         end
 
         it 'selects deployed environments' do
-          expect(merge_request.environments).to contain_exactly(source_environment, target_environment)
+          expect(merge_request.environments_for(user)).to contain_exactly(source_environment, target_environment)
         end
       end
     end
@@ -1064,7 +1070,7 @@ describe MergeRequest, models: true do
       end
 
       it 'returns an empty array' do
-        expect(merge_request.environments).to be_empty
+        expect(merge_request.environments_for(user)).to be_empty
       end
     end
   end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 2280d0f554a..2129bcbd74d 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -1716,111 +1716,6 @@ describe Project, models: true do
     end
   end
 
-  describe '#environments_for' do
-    let(:project) { create(:project, :repository) }
-    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(ref: 'master', commit: 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(ref: 'master', commit: project.commit))
-          .to be_empty
-      end
-
-      it 'does not return environment when commit is not part of deployment' do
-        expect(project.environments_for(ref: 'master', commit: project.commit('feature')))
-          .to be_empty
-      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(ref: 'master', commit: project.commit))
-          .to contain_exactly(environment)
-      end
-
-      it 'does not environment when ref is different' do
-        expect(project.environments_for(ref: 'feature', commit: project.commit))
-          .to be_empty
-      end
-
-      it 'does not return environment when commit is not part of deployment' do
-        expect(project.environments_for(ref: 'master', commit: project.commit('feature')))
-          .to be_empty
-      end
-
-      it 'returns environment when commit constraint is not set' do
-        expect(project.environments_for(ref: 'master'))
-          .to contain_exactly(environment)
-      end
-    end
-
-    context 'commit deployment' do
-      before do
-        create(:deployment, environment: environment, ref: 'master', sha: project.commit.id)
-      end
-
-      it 'returns environment' do
-        expect(project.environments_for(commit: project.commit))
-          .to contain_exactly(environment)
-      end
-    end
-  end
-
-  describe '#environments_recently_updated_on_branch' do
-    let(:project) { create(:project, :repository) }
-    let(:environment) { create(:environment, project: project) }
-
-    context 'when last deployment to environment is the most recent one' do
-      before do
-        create(:deployment, environment: environment, ref: 'feature')
-      end
-
-      it 'finds recently updated environment' do
-        expect(project.environments_recently_updated_on_branch('feature'))
-          .to contain_exactly(environment)
-      end
-    end
-
-    context 'when last deployment to environment is not the most recent' do
-      before do
-        create(:deployment, environment: environment, ref: 'feature')
-        create(:deployment, environment: environment, ref: 'master')
-      end
-
-      it 'does not find environment' do
-        expect(project.environments_recently_updated_on_branch('feature'))
-          .to be_empty
-      end
-    end
-
-    context 'when there are two environments that deploy to the same branch' do
-      let(:second_environment) { create(:environment, project: project) }
-
-      before do
-        create(:deployment, environment: environment, ref: 'feature')
-        create(:deployment, environment: second_environment, ref: 'feature')
-      end
-
-      it 'finds both environments' do
-        expect(project.environments_recently_updated_on_branch('feature'))
-          .to contain_exactly(environment, second_environment)
-      end
-    end
-  end
-
   describe '#deployment_variables' do
     context 'when project has no deployment service' do
       let(:project) { create(:empty_project) }
@@ -1879,7 +1774,7 @@ describe Project, models: true do
     end
 
     before do
-      project.repository.commit_file(User.last, '.gitlab/route-map.yml', route_map, 'Add .gitlab/route-map.yml', 'master', false)
+      project.repository.commit_file(User.last, '.gitlab/route-map.yml', route_map, message: 'Add .gitlab/route-map.yml', branch_name: 'master', update: false)
     end
 
     context 'when there is a .gitlab/route-map.yml at the commit' do
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index 7f97319aa2f..9bfa6409607 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -1785,7 +1785,7 @@ describe Repository, models: true do
 
   describe '#gitlab_ci_yml_for' do
     before do
-      repository.commit_file(User.last, '.gitlab-ci.yml', 'CONTENT', 'Add .gitlab-ci.yml', 'master', false)
+      repository.commit_file(User.last, '.gitlab-ci.yml', 'CONTENT', message: 'Add .gitlab-ci.yml', branch_name: 'master', update: false)
     end
 
     context 'when there is a .gitlab-ci.yml at the commit' do
@@ -1803,7 +1803,7 @@ describe Repository, models: true do
 
   describe '#route_map_for' do
     before do
-      repository.commit_file(User.last, '.gitlab/route-map.yml', 'CONTENT', 'Add .gitlab/route-map.yml', 'master', false)
+      repository.commit_file(User.last, '.gitlab/route-map.yml', 'CONTENT', message: 'Add .gitlab/route-map.yml', branch_name: 'master', update: false)
     end
 
     context 'when there is a .gitlab/route-map.yml at the commit' do