Set feature flag via command line

First attempt at allowing a feature flag to be set via the command line
when running tests. This will enable the flag, run the tests, and then
disable the flag.

Using OptionParser meant changing how scenarios get the instance
address, so this also allows the address to be set as a command line
option. It's backwards compatible (you can still provide the address
as the command line option after the scenario)

19 files changed, 259 insertions, 26 deletions

diff --git a/qa/README.md b/qa/README.md
index 735868e7640..7d66f7d5abc 100644
--- a/qa/README.md
+++ b/qa/README.md
@@ -55,16 +55,19 @@ You can also supply specific tests to run as another parameter. For example, to
 run the repository-related specs, you can execute:
 
 ```
-bin/qa Test::Instance::All http://localhost qa/specs/features/repository/
+bin/qa Test::Instance::All http://localhost -- qa/specs/features/browser_ui/3_create/repository
 ```
 
 Since the arguments would be passed to `rspec`, you could use all `rspec`
 options there. For example, passing `--backtrace` and also line number:
 
 ```
-bin/qa Test::Instance::All http://localhost qa/specs/features/project/create_spec.rb:3 --backtrace
+bin/qa Test::Instance::All http://localhost -- qa/specs/features/browser_ui/3_create/merge_request/create_merge_request_spec.rb:6 --backtrace
 ```
 
+Note that the separator `--` is required; all subsequent options will be
+ignored by the QA framework and passed to `rspec`.
+
 ### Overriding the authenticated user
 
 Unless told otherwise, the QA tests will run as the default `root` user seeded
@@ -117,7 +120,7 @@ tests that are expected to fail while a fix is in progress (similar to how
  can be used).
 
 ```
-bin/qa Test::Instance::All http://localhost --tag quarantine
+bin/qa Test::Instance::All http://localhost -- --tag quarantine
 ```
 
 If `quarantine` is used with other tags, tests will only be run if they have at
@@ -128,3 +131,25 @@ For example, suppose one test has `:smoke` and `:quarantine` metadata, and
 another test has `:ldap` and `:quarantine` metadata. If the tests are run with
 `--tag smoke --tag quarantine`, only the first test will run. The test with
 `:ldap` will not run even though it also has `:quarantine`.
+
+### Running tests with a feature flag enabled
+
+Tests can be run with with a feature flag enabled by using the command-line
+option `--enable-feature FEATURE_FLAG`. For example, to enable the feature flag
+that enforces Gitaly request limits, you would use the command:
+
+```
+bin/qa Test::Instance::All http://localhost --enable-feature gitaly_enforce_requests_limits
+```
+
+This will instruct the QA framework to enable the `gitaly_enforce_requests_limits`
+feature flag ([via the API](https://docs.gitlab.com/ee/api/features.html)), run
+all the tests in the `Test::Instance::All` scenario, and then disable the
+feature flag again.
+
+Note: the QA framework doesn't currently allow you to easily toggle a feature
+flag during a single test, [as you can in unit tests](https://docs.gitlab.com/ee/development/feature_flags.html#specs),
+but [that capability is planned](https://gitlab.com/gitlab-org/quality/team-tasks/issues/77).
+
+Note also that the `--` separator isn't used because `--enable-feature` is a QA
+framework option, not an `rspec` option.
\ No newline at end of file
diff --git a/qa/qa.rb b/qa/qa.rb
index ec8aef31e48..672f4aa928a 100644
--- a/qa/qa.rb
+++ b/qa/qa.rb
@@ -17,6 +17,7 @@ module QA
     autoload :Env, 'qa/runtime/env'
     autoload :Address, 'qa/runtime/address'
     autoload :Path, 'qa/runtime/path'
+    autoload :Feature, 'qa/runtime/feature'
     autoload :Fixtures, 'qa/runtime/fixtures'
     autoload :Logger, 'qa/runtime/logger'
 
@@ -89,6 +90,7 @@ module QA
     autoload :Bootable, 'qa/scenario/bootable'
     autoload :Actable, 'qa/scenario/actable'
     autoload :Template, 'qa/scenario/template'
+    autoload :SharedAttributes, 'qa/scenario/shared_attributes'
 
     ##
     # Test scenario entrypoints.
diff --git a/qa/qa/resource/api_fabricator.rb b/qa/qa/resource/api_fabricator.rb
index 98eebac0880..de04467ff5b 100644
--- a/qa/qa/resource/api_fabricator.rb
+++ b/qa/qa/resource/api_fabricator.rb
@@ -8,9 +8,6 @@ module QA
     module ApiFabricator
       include Capybara::DSL
 
-      HTTP_STATUS_OK = 200
-      HTTP_STATUS_CREATED = 201
-
       ResourceNotFoundError = Class.new(RuntimeError)
       ResourceFabricationFailedError = Class.new(RuntimeError)
       ResourceURLMissingError = Class.new(RuntimeError)
diff --git a/qa/qa/runtime/address.rb b/qa/qa/runtime/address.rb
index ffad3974b02..af0537dc17c 100644
--- a/qa/qa/runtime/address.rb
+++ b/qa/qa/runtime/address.rb
@@ -15,6 +15,13 @@ module QA
           @instance.to_s
         end
       end
+
+      def self.valid?(value)
+        uri = URI.parse(value)
+        uri.is_a?(URI::HTTP) && !uri.host.nil?
+      rescue URI::InvalidURIError
+        false
+      end
     end
   end
 end
diff --git a/qa/qa/runtime/feature.rb b/qa/qa/runtime/feature.rb
new file mode 100644
index 00000000000..1b4ae7adbbe
--- /dev/null
+++ b/qa/qa/runtime/feature.rb
@@ -0,0 +1,36 @@
+# frozen_string_literal: true
+
+module QA
+  module Runtime
+    module Feature
+      extend self
+      extend Support::Api
+
+      SetFeatureError = Class.new(RuntimeError)
+
+      def enable(key)
+        QA::Runtime::Logger.info("Enabling feature: #{key}")
+        set_feature(key, true)
+      end
+
+      def disable(key)
+        QA::Runtime::Logger.info("Disabling feature: #{key}")
+        set_feature(key, false)
+      end
+
+      private
+
+      def api_client
+        @api_client ||= Runtime::API::Client.new(:gitlab)
+      end
+
+      def set_feature(key, value)
+        request = Runtime::API::Request.new(api_client, "/features/#{key}")
+        response = post(request.url, { value: value })
+        unless response.code == QA::Support::Api::HTTP_STATUS_CREATED
+          raise SetFeatureError, "Setting feature flag #{key} to #{value} failed with `#{response}`."
+        end
+      end
+    end
+  end
+end
diff --git a/qa/qa/scenario/bootable.rb b/qa/qa/scenario/bootable.rb
index dd12ea6d492..038418be023 100644
--- a/qa/qa/scenario/bootable.rb
+++ b/qa/qa/scenario/bootable.rb
@@ -23,7 +23,7 @@ module QA
 
           arguments.parse!(argv)
 
-          self.perform(Runtime::Scenario.attributes, *arguments.default_argv)
+          self.perform(Runtime::Scenario.attributes, *argv)
         end
 
         private
@@ -33,7 +33,13 @@ module QA
         end
 
         def options
-          @options ||= []
+          # Scenario options/attributes are global. There's only ever one
+          # scenario at a time, but they can be inherited and we want scenarios
+          # to share the attributes of their ancestors. For example, `Mattermost`
+          # inherits from `Test::Instance::All` but if this were an instance
+          # variable then `Mattermost` wouldn't have access to the attributes
+          # in `All`
+          @@options ||= [] # rubocop:disable Style/ClassVars
         end
 
         def has_attributes?
diff --git a/qa/qa/scenario/shared_attributes.rb b/qa/qa/scenario/shared_attributes.rb
new file mode 100644
index 00000000000..40d5c6b1ff1
--- /dev/null
+++ b/qa/qa/scenario/shared_attributes.rb
@@ -0,0 +1,12 @@
+# frozen_string_literal: true
+
+module QA
+  module Scenario
+    module SharedAttributes
+      include Bootable
+
+      attribute :gitlab_address, '--address URL', 'Address of the instance to test'
+      attribute :enable_feature, '--enable-feature FEATURE_FLAG', 'Enable a feature before running tests'
+    end
+  end
+end
diff --git a/qa/qa/scenario/template.rb b/qa/qa/scenario/template.rb
index cb1a1de6b9a..b8ea26e805e 100644
--- a/qa/qa/scenario/template.rb
+++ b/qa/qa/scenario/template.rb
@@ -18,19 +18,44 @@ module QA
         end
       end
 
-      def perform(address, *rspec_options)
-        Runtime::Scenario.define(:gitlab_address, address)
+      def perform(options, *args)
+        extract_address(:gitlab_address, options, args)
 
         ##
         # Perform before hooks, which are different for CE and EE
         #
         Runtime::Release.perform_before_hooks
 
+        Runtime::Feature.enable(options[:enable_feature]) if options.key?(:enable_feature)
+
         Specs::Runner.perform do |specs|
           specs.tty = true
           specs.tags = self.class.focus
-          specs.options = rspec_options if rspec_options.any?
+          specs.options = args if args.any?
         end
+      ensure
+        Runtime::Feature.disable(options[:enable_feature]) if options.key?(:enable_feature)
+      end
+
+      def extract_option(name, options, args)
+        option = if options.key?(name)
+                   options[name]
+                 else
+                   args.shift
+                 end
+
+        Runtime::Scenario.define(name, option)
+
+        option
+      end
+
+      # For backwards-compatibility, if the gitlab instance address is not
+      # specified as an option parsed by OptionParser, it can be specified as
+      # the first argument
+      def extract_address(name, options, args)
+        address = extract_option(name, options, args)
+
+        raise ::ArgumentError, "The address provided for `#{name}` is not valid: #{address}" unless Runtime::Address.valid?(address)
       end
     end
   end
diff --git a/qa/qa/scenario/test/instance/all.rb b/qa/qa/scenario/test/instance/all.rb
index a07c26431bd..168ac4c09a1 100644
--- a/qa/qa/scenario/test/instance/all.rb
+++ b/qa/qa/scenario/test/instance/all.rb
@@ -8,6 +8,7 @@ module QA
       module Instance
         class All < Template
           include Bootable
+          include SharedAttributes
         end
       end
     end
diff --git a/qa/qa/scenario/test/instance/smoke.rb b/qa/qa/scenario/test/instance/smoke.rb
index a7d2cb27f27..43f0623483e 100644
--- a/qa/qa/scenario/test/instance/smoke.rb
+++ b/qa/qa/scenario/test/instance/smoke.rb
@@ -8,6 +8,7 @@ module QA
         #
         class Smoke < Template
           include Bootable
+          include SharedAttributes
 
           tags :smoke
         end
diff --git a/qa/qa/scenario/test/integration/mattermost.rb b/qa/qa/scenario/test/integration/mattermost.rb
index ece6fba75c9..f5072ee227c 100644
--- a/qa/qa/scenario/test/integration/mattermost.rb
+++ b/qa/qa/scenario/test/integration/mattermost.rb
@@ -9,10 +9,13 @@ module QA
         class Mattermost < Test::Instance::All
           tags :mattermost
 
-          def perform(address, mattermost, *rspec_options)
-            Runtime::Scenario.define(:mattermost_address, mattermost)
+          attribute :mattermost_address, '--mattermost-address URL', 'Address of the Mattermost server'
 
-            super(address, *rspec_options)
+          def perform(options, *args)
+            extract_address(:gitlab_address, options, args)
+            extract_address(:mattermost_address, options, args)
+
+            super(options, *args)
           end
         end
       end
diff --git a/qa/qa/support/api.rb b/qa/qa/support/api.rb
index 229bfb44fa5..31cff5a241c 100644
--- a/qa/qa/support/api.rb
+++ b/qa/qa/support/api.rb
@@ -1,6 +1,9 @@
 module QA
   module Support
     module Api
+      HTTP_STATUS_OK = 200
+      HTTP_STATUS_CREATED = 201
+
       def post(url, payload)
         RestClient::Request.execute(
           method: :post,
diff --git a/qa/spec/runtime/feature_spec.rb b/qa/spec/runtime/feature_spec.rb
new file mode 100644
index 00000000000..192299b7857
--- /dev/null
+++ b/qa/spec/runtime/feature_spec.rb
@@ -0,0 +1,41 @@
+# frozen_string_literal: true
+
+describe QA::Runtime::Feature do
+  let(:api_client) { double('QA::Runtime::API::Client') }
+  let(:request) { Struct.new(:url).new('http://api') }
+  let(:response) { Struct.new(:code).new(201) }
+
+  before do
+    allow(described_class).to receive(:api_client).and_return(api_client)
+  end
+
+  describe '.enable' do
+    it 'enables a feature flag' do
+      expect(QA::Runtime::API::Request)
+        .to receive(:new)
+        .with(api_client, "/features/a-flag")
+        .and_return(request)
+      expect(described_class)
+        .to receive(:post)
+        .with(request.url, { value: true })
+        .and_return(response)
+
+      subject.enable('a-flag')
+    end
+  end
+
+  describe '.disable' do
+    it 'disables a feature flag' do
+      expect(QA::Runtime::API::Request)
+        .to receive(:new)
+        .with(api_client, "/features/a-flag")
+        .and_return(request)
+      expect(described_class)
+        .to receive(:post)
+        .with(request.url, { value: false })
+        .and_return(response)
+
+      subject.disable('a-flag')
+    end
+  end
+end
diff --git a/qa/spec/runtime/scenario_spec.rb b/qa/spec/runtime/scenario_spec.rb
index 7009192bcc0..70fc71ffc02 100644
--- a/qa/spec/runtime/scenario_spec.rb
+++ b/qa/spec/runtime/scenario_spec.rb
@@ -13,6 +13,14 @@ describe QA::Runtime::Scenario do
       .to eq(my_attribute: 'some-value', another_attribute: 'another-value')
   end
 
+  it 'replaces an existing attribute' do
+    subject.define(:my_attribute, 'some-value')
+    subject.define(:my_attribute, 'another-value')
+
+    expect(subject.my_attribute).to eq 'another-value'
+    expect(subject.attributes).to eq(my_attribute: 'another-value')
+  end
+
   it 'raises error when attribute is not known' do
     expect { subject.invalid_accessor }
       .to raise_error ArgumentError, /invalid_accessor/
diff --git a/qa/spec/scenario/bootable_spec.rb b/qa/spec/scenario/bootable_spec.rb
index 273aac7677e..bd89b21f7fb 100644
--- a/qa/spec/scenario/bootable_spec.rb
+++ b/qa/spec/scenario/bootable_spec.rb
@@ -4,14 +4,21 @@ describe QA::Scenario::Bootable do
       .include(described_class)
   end
 
+  before do
+    allow(subject).to receive(:options).and_return([])
+    allow(QA::Runtime::Scenario).to receive(:attributes).and_return({})
+  end
+
   it 'makes it possible to define the scenario attribute' do
     subject.class_eval do
       attribute :something, '--something SOMETHING', 'Some attribute'
       attribute :another, '--another ANOTHER', 'Some other attribute'
     end
 
+    # If we run just this test from the command line it fails unless
+    # we include the command line args that we use to select this test.
     expect(subject).to receive(:perform)
-      .with(something: 'test', another: 'other')
+      .with({ something: 'test', another: 'other' })
 
     subject.launch!(%w[--another other --something test])
   end
diff --git a/qa/spec/scenario/template_spec.rb b/qa/spec/scenario/template_spec.rb
new file mode 100644
index 00000000000..f97fc22daf9
--- /dev/null
+++ b/qa/spec/scenario/template_spec.rb
@@ -0,0 +1,28 @@
+# frozen_string_literal: true
+
+describe QA::Scenario::Template do
+  let(:feature) { spy('Runtime::Feature') }
+  let(:release) { spy('Runtime::Release') }
+
+  before do
+    stub_const('QA::Runtime::Release', release)
+    stub_const('QA::Runtime::Feature', feature)
+    allow(QA::Specs::Runner).to receive(:perform)
+    allow(QA::Runtime::Address).to receive(:valid?).and_return(true)
+  end
+
+  it 'allows a feature to be enabled' do
+    subject.perform({ enable_feature: 'a-feature' })
+
+    expect(feature).to have_received(:enable).with('a-feature')
+  end
+
+  it 'ensures an enabled feature is disabled afterwards' do
+    allow(QA::Specs::Runner).to receive(:perform).and_raise('failed test')
+
+    expect { subject.perform({ enable_feature: 'a-feature' }) }.to raise_error('failed test')
+
+    expect(feature).to have_received(:enable).with('a-feature')
+    expect(feature).to have_received(:disable).with('a-feature')
+  end
+end
diff --git a/qa/spec/scenario/test/integration/github_spec.rb b/qa/spec/scenario/test/integration/github_spec.rb
index c2aeb1ded1d..6112ba7c694 100644
--- a/qa/spec/scenario/test/integration/github_spec.rb
+++ b/qa/spec/scenario/test/integration/github_spec.rb
@@ -12,7 +12,7 @@ describe QA::Scenario::Test::Integration::Github do
       let(:tags) { [:github] }
 
       it 'requires a GitHub access token' do
-        subject.perform('gitlab_address')
+        subject.perform(args)
 
         expect(env).to have_received(:require_github_access_token!)
       end
diff --git a/qa/spec/scenario/test/integration/mattermost_spec.rb b/qa/spec/scenario/test/integration/mattermost_spec.rb
index 59caf2ba2cd..4e75e72f4d2 100644
--- a/qa/spec/scenario/test/integration/mattermost_spec.rb
+++ b/qa/spec/scenario/test/integration/mattermost_spec.rb
@@ -4,14 +4,21 @@ describe QA::Scenario::Test::Integration::Mattermost do
   context '#perform' do
     it_behaves_like 'a QA scenario class' do
       let(:args) { %w[gitlab_address mattermost_address] }
+      let(:args) do
+        {
+          gitlab_address: 'http://gitlab_address',
+          mattermost_address: 'http://mattermost_address'
+        }
+      end
+      let(:named_options) { %w[--address http://gitlab_address --mattermost-address http://mattermost_address] }
       let(:tags) { [:mattermost] }
       let(:options) { ['path1']}
 
       it 'requires a GitHub access token' do
-        subject.perform(*args)
+        subject.perform(args)
 
         expect(attributes).to have_received(:define)
-          .with(:mattermost_address, 'mattermost_address')
+          .with(:mattermost_address, 'http://mattermost_address')
       end
     end
   end
diff --git a/qa/spec/shared_examples/scenario_shared_examples.rb b/qa/spec/shared_examples/scenario_shared_examples.rb
index 5fd55d7d96b..697e6cb39c8 100644
--- a/qa/spec/shared_examples/scenario_shared_examples.rb
+++ b/qa/spec/shared_examples/scenario_shared_examples.rb
@@ -2,19 +2,23 @@
 
 shared_examples 'a QA scenario class' do
   let(:attributes) { spy('Runtime::Scenario') }
-  let(:release) { spy('Runtime::Release') }
   let(:runner) { spy('Specs::Runner') }
+  let(:release) { spy('Runtime::Release') }
+  let(:feature) { spy('Runtime::Feature') }
 
-  let(:args) { ['gitlab_address'] }
+  let(:args) { { gitlab_address: 'http://gitlab_address' } }
+  let(:named_options) { %w[--address http://gitlab_address] }
   let(:tags) { [] }
   let(:options) { %w[path1 path2] }
 
   before do
+    stub_const('QA::Specs::Runner', runner)
     stub_const('QA::Runtime::Release', release)
     stub_const('QA::Runtime::Scenario', attributes)
-    stub_const('QA::Specs::Runner', runner)
+    stub_const('QA::Runtime::Feature', feature)
 
     allow(runner).to receive(:perform).and_yield(runner)
+    allow(QA::Runtime::Address).to receive(:valid?).and_return(true)
   end
 
   it 'responds to perform' do
@@ -22,28 +26,48 @@ shared_examples 'a QA scenario class' do
   end
 
   it 'sets an address of the subject' do
-    subject.perform(*args)
+    subject.perform(args)
 
-    expect(attributes).to have_received(:define).with(:gitlab_address, 'gitlab_address')
+    expect(attributes).to have_received(:define).with(:gitlab_address, 'http://gitlab_address').at_least(:once)
   end
 
   it 'performs before hooks' do
-    subject.perform(*args)
+    subject.perform(args)
 
     expect(release).to have_received(:perform_before_hooks)
   end
 
   it 'sets tags on runner' do
-    subject.perform(*args)
+    subject.perform(args)
 
     expect(runner).to have_received(:tags=).with(tags)
   end
 
   context 'specifying RSpec options' do
     it 'sets options on runner' do
-      subject.perform(*args, *options)
+      subject.perform(args, *options)
 
       expect(runner).to have_received(:options=).with(options)
     end
   end
+
+  context 'with named command-line options' do
+    it 'converts options to attributes' do
+      described_class.launch!(named_options)
+
+      args do |k, v|
+        expect(attributes).to have_received(:define).with(k, v)
+      end
+    end
+
+    it 'raises an error if the option is invalid' do
+      expect { described_class.launch!(['--foo']) }.to raise_error(OptionParser::InvalidOption)
+    end
+
+    it 'passes on options after --' do
+      expect(described_class).to receive(:perform).with(attributes, *%w[--tag quarantine])
+
+      described_class.launch!(named_options.push(*%w[-- --tag quarantine]))
+    end
+  end
 end