Merge remote-tracking branch 'upstream/master' into qa/ee-4698-backport

* upstream/master:
  Just don't expand if it's already expanded
  fix bug in webpack_helper in which force_same_domain argument was ignored breaking local rspec tests
  Fix rubocop offenses introduced in !16623
  Prefer local variables over given/let
  Try to restore page's state
  Add more views check
  Respect the latest changes from master
  Add a scenario for adding secret variables

9 files changed, 139 insertions, 4 deletions

diff --git a/app/helpers/webpack_helper.rb b/app/helpers/webpack_helper.rb
index 94887c2cbd2..77433acb92a 100644
--- a/app/helpers/webpack_helper.rb
+++ b/app/helpers/webpack_helper.rb
@@ -2,7 +2,7 @@ require 'webpack/rails/manifest'
 
 module WebpackHelper
   def webpack_bundle_tag(bundle, force_same_domain: false)
-    javascript_include_tag(*gitlab_webpack_asset_paths(bundle, force_same_domain: true))
+    javascript_include_tag(*gitlab_webpack_asset_paths(bundle, force_same_domain: force_same_domain))
   end
 
   # override webpack-rails gem helper until changes can make it upstream
diff --git a/qa/qa.rb b/qa/qa.rb
index 69f4dc86d83..deac77036c5 100644
--- a/qa/qa.rb
+++ b/qa/qa.rb
@@ -29,6 +29,7 @@ module QA
       autoload :Group, 'qa/factory/resource/group'
       autoload :Project, 'qa/factory/resource/project'
       autoload :DeployKey, 'qa/factory/resource/deploy_key'
+      autoload :SecretVariable, 'qa/factory/resource/secret_variable'
       autoload :Runner, 'qa/factory/resource/runner'
       autoload :PersonalAccessToken, 'qa/factory/resource/personal_access_token'
     end
@@ -119,6 +120,7 @@ module QA
         autoload :Repository, 'qa/page/project/settings/repository'
         autoload :CICD, 'qa/page/project/settings/ci_cd'
         autoload :DeployKeys, 'qa/page/project/settings/deploy_keys'
+        autoload :SecretVariables, 'qa/page/project/settings/secret_variables'
         autoload :Runners, 'qa/page/project/settings/runners'
       end
     end
diff --git a/qa/qa/factory/resource/secret_variable.rb b/qa/qa/factory/resource/secret_variable.rb
new file mode 100644
index 00000000000..54ef4d8d964
--- /dev/null
+++ b/qa/qa/factory/resource/secret_variable.rb
@@ -0,0 +1,41 @@
+module QA
+  module Factory
+    module Resource
+      class SecretVariable < Factory::Base
+        attr_accessor :key, :value
+
+        product :key do
+          Page::Project::Settings::CICD.act do
+            expand_secret_variables(&:variable_key)
+          end
+        end
+
+        product :value do
+          Page::Project::Settings::CICD.act do
+            expand_secret_variables(&:variable_value)
+          end
+        end
+
+        dependency Factory::Resource::Project, as: :project do |project|
+          project.name = 'project-with-secret-variables'
+          project.description = 'project for adding secret variable test'
+        end
+
+        def fabricate!
+          project.visit!
+
+          Page::Menu::Side.act { click_ci_cd_settings }
+
+          Page::Project::Settings::CICD.perform do |setting|
+            setting.expand_secret_variables do |page|
+              page.fill_variable_key(key)
+              page.fill_variable_value(value)
+
+              page.add_variable
+            end
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/qa/qa/page/menu/side.rb b/qa/qa/page/menu/side.rb
index aeddc77e4c6..77d1689884f 100644
--- a/qa/qa/page/menu/side.rb
+++ b/qa/qa/page/menu/side.rb
@@ -9,6 +9,10 @@ module QA
           element :top_level_items, '.sidebar-top-level-items'
         end
 
+        view 'app/assets/javascripts/fly_out_nav.js' do
+          element :fly_out, "classList.add('fly-out-list')"
+        end
+
         def click_repository_settings
           hover_settings do
             within_submenu do
diff --git a/qa/qa/page/project/settings/ci_cd.rb b/qa/qa/page/project/settings/ci_cd.rb
index 5270dde7411..99be21bbe89 100644
--- a/qa/qa/page/project/settings/ci_cd.rb
+++ b/qa/qa/page/project/settings/ci_cd.rb
@@ -7,6 +7,7 @@ module QA
 
           view 'app/views/projects/settings/ci_cd/show.html.haml' do
             element :runners_settings, 'Runners settings'
+            element :secret_variables, 'Secret variables'
           end
 
           def expand_runners_settings(&block)
@@ -14,6 +15,12 @@ module QA
               Settings::Runners.perform(&block)
             end
           end
+
+          def expand_secret_variables(&block)
+            expand_section('Secret variables') do
+              Settings::SecretVariables.perform(&block)
+            end
+          end
         end
       end
     end
diff --git a/qa/qa/page/project/settings/common.rb b/qa/qa/page/project/settings/common.rb
index 1d5c919ba4a..5bed873b5df 100644
--- a/qa/qa/page/project/settings/common.rb
+++ b/qa/qa/page/project/settings/common.rb
@@ -17,7 +17,7 @@ module QA
           def expand_section(name)
             page.within('#content-body') do
               page.within('section', text: name) do
-                click_button('Expand')
+                click_button 'Expand' unless first('button', text: 'Collapse')
 
                 yield if block_given?
               end
diff --git a/qa/qa/page/project/settings/secret_variables.rb b/qa/qa/page/project/settings/secret_variables.rb
new file mode 100644
index 00000000000..e3bfbfcf080
--- /dev/null
+++ b/qa/qa/page/project/settings/secret_variables.rb
@@ -0,0 +1,57 @@
+module QA
+  module Page
+    module Project
+      module Settings
+        class SecretVariables < Page::Base
+          include Common
+
+          view 'app/views/ci/variables/_table.html.haml' do
+            element :variable_key, '.variable-key'
+            element :variable_value, '.variable-value'
+          end
+
+          view 'app/views/ci/variables/_index.html.haml' do
+            element :add_new_variable, 'btn_text: "Add new variable"'
+          end
+
+          view 'app/assets/javascripts/behaviors/secret_values.js' do
+            element :reveal_value, 'Reveal value'
+            element :hide_value, 'Hide value'
+          end
+
+          def fill_variable_key(key)
+            fill_in 'variable_key', with: key
+          end
+
+          def fill_variable_value(value)
+            fill_in 'variable_value', with: value
+          end
+
+          def add_variable
+            click_on 'Add new variable'
+          end
+
+          def variable_key
+            page.find('.variable-key').text
+          end
+
+          def variable_value
+            reveal_value do
+              page.find('.variable-value').text
+            end
+          end
+
+          private
+
+          def reveal_value
+            click_button('Reveal value')
+
+            yield.tap do
+              click_button('Hide value')
+            end
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/qa/qa/specs/features/project/add_secret_variable_spec.rb b/qa/qa/specs/features/project/add_secret_variable_spec.rb
new file mode 100644
index 00000000000..36422a92afc
--- /dev/null
+++ b/qa/qa/specs/features/project/add_secret_variable_spec.rb
@@ -0,0 +1,19 @@
+module QA
+  feature 'secret variables support', :core do
+    scenario 'user adds a secret variable' do
+      Runtime::Browser.visit(:gitlab, Page::Main::Login)
+      Page::Main::Login.act { sign_in_using_credentials }
+
+      variable_key = 'VARIABLE_KEY'
+      variable_value = 'variable value'
+
+      variable = Factory::Resource::SecretVariable.fabricate! do |resource|
+        resource.key = variable_key
+        resource.value = variable_value
+      end
+
+      expect(variable.key).to eq(variable_key)
+      expect(variable.value).to eq(variable_value)
+    end
+  end
+end
diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb
index e4562e4d43e..3db04d99855 100644
--- a/spec/lib/gitlab/git/repository_spec.rb
+++ b/spec/lib/gitlab/git/repository_spec.rb
@@ -1998,8 +1998,13 @@ describe Gitlab::Git::Repository, seed_helper: true do
       let(:project) { create(:project) }
       let(:imported_repo) { project.repository.raw }
 
-      before { expect(repository.bundle_to_disk(bundle_path)).to be true }
-      after { FileUtils.rm_rf(bundle_path) }
+      before do
+        expect(repository.bundle_to_disk(bundle_path)).to be true
+      end
+
+      after do
+        FileUtils.rm_rf(bundle_path)
+      end
 
       it 'creates a repo from a bundle file' do
         expect(imported_repo).not_to exist