Merge branch 'qa-ml-feature-flag-command-line-ee' into 'master'

[QA-EE] Toggle Gitaly N+1 detector via feature flag and run tests

See merge request gitlab-org/gitlab-ee!10324

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.

qa/qa/ee/scenario/test/integration/group_saml.rb

@@ -7,6 +7,7 @@ module QA
         module Integration
           class GroupSAML < QA::Scenario::Template
             include QA::Scenario::Bootable
+            include QA::Scenario::SharedAttributes
             tags :group_saml
           end
         end

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)

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

qa/qa/runtime/feature.rb

+# 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

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?

qa/qa/scenario/shared_attributes.rb

+# 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

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

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

qa/qa/scenario/test/instance/smoke.rb

@@ -8,6 +8,7 @@ module QA
         #
         class Smoke < Template
           include Bootable
+          include SharedAttributes
 
           tags :smoke
         end

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

qa/qa/support/api.rb

 module QA
   module Support
     module Api
+      HTTP_STATUS_OK = 200
+      HTTP_STATUS_CREATED = 201
+
       def post(url, payload)
         RestClient::Request.execute(
           method: :post,

qa/spec/runtime/feature_spec.rb

+# 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

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/

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

qa/spec/scenario/template_spec.rb

+# 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

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

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

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