From b74b7309a5121be50fe6c07c85b61e258c113ce6 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Thu, 25 May 2017 22:14:22 +0800
Subject: [PATCH] Add tests for CI_ENVIRONMENT_URL

---
 app/models/ci/build.rb       | 11 ++++---
 spec/models/ci/build_spec.rb | 59 ++++++++++++++++++++++++++++++++----
 2 files changed, 60 insertions(+), 10 deletions(-)

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 82c7ca943ed..e50ea0e566a 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -499,10 +499,13 @@ module Ci
     end
 
     def persisted_environment_variables
-      persisted_environment.predefined_variables <<
-        { key: 'CI_ENVIRONMENT_URL',
-          value: expanded_environment_url,
-          public: true }
+      variables = persisted_environment.predefined_variables
+
+      variables << { key: 'CI_ENVIRONMENT_URL',
+                     value: expanded_environment_url,
+                     public: true } if environment_url
+
+      variables
     end
 
     def legacy_variables
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index e971b4bc3f9..5b2c1472974 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -20,6 +20,7 @@ describe Ci::Build, :models do
   it { is_expected.to validate_presence_of(:ref) }
   it { is_expected.to respond_to(:has_trace?) }
   it { is_expected.to respond_to(:trace) }
+  it { is_expected.to validate_length_of(:external_url).is_at_most(255) }
 
   describe '#actionize' do
     context 'when build is a created' do
@@ -427,6 +428,30 @@ describe Ci::Build, :models do
       end
     end
 
+    describe '#expanded_environment_url' do
+      subject { build.expanded_environment_url }
+
+      context 'when environment uses $CI_COMMIT_REF_NAME' do
+        let(:build) do
+          create(:ci_build,
+                 ref: 'master',
+                 environment_url: 'http://review/$CI_COMMIT_REF_NAME')
+        end
+
+        it { is_expected.to eq('http://review/master') }
+      end
+
+      context 'when environment uses yaml_variables containing symbol keys' do
+        let(:build) do
+          create(:ci_build,
+                 yaml_variables: [{ key: :APP_HOST, value: 'host' }],
+                 environment_url: 'http://review/$APP_HOST')
+        end
+
+        it { is_expected.to eq('http://review/host') }
+      end
+    end
+
     describe '#starts_environment?' do
       subject { build.starts_environment? }
 
@@ -1176,11 +1201,6 @@ describe Ci::Build, :models do
     end
 
     context 'when build has an environment' do
-      before do
-        build.update(environment: 'production')
-        create(:environment, project: build.project, name: 'production', slug: 'prod-slug')
-      end
-
       let(:environment_variables) do
         [
           { key: 'CI_ENVIRONMENT_NAME', value: 'production', public: true },
@@ -1188,7 +1208,34 @@ describe Ci::Build, :models do
         ]
       end
 
-      it { environment_variables.each { |v| is_expected.to include(v) } }
+      before do
+        build.update(environment: 'production')
+        create(:environment, project: build.project, name: 'production', slug: 'prod-slug')
+      end
+
+      context 'when no URL was set' do
+        it { environment_variables.each { |v| is_expected.to include(v) } }
+
+        it 'does not have CI_ENVIRONMENT_URL' do
+          keys = subject.map { |var| var[:key] }
+
+          expect(keys).to include('CI_ENVIRONMENT_NAME', 'CI_ENVIRONMENT_SLUG')
+          expect(keys).not_to include('CI_ENVIRONMENT_URL')
+        end
+      end
+
+      context 'when an URL was set' do
+        before do
+          build.update(environment_url: 'http://host/$CI_JOB_NAME')
+
+          environment_variables <<
+            { key: 'CI_ENVIRONMENT_URL',
+              value: 'http://host/test',
+              public: true }
+        end
+
+        it { environment_variables.each { |v| is_expected.to include(v) } }
+      end
     end
 
     context 'when build started manually' do