From 67c38a6573f35333cf9b1a399431b86e8b376443 Mon Sep 17 00:00:00 2001
From: Mark Lapierre <mlapierre@gitlab.com>
Date: Wed, 27 Mar 2019 19:03:03 +0000
Subject: [PATCH] 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)
---
qa/README.md | 31 ++++++++++++--
qa/qa.rb | 2 +
qa/qa/resource/api_fabricator.rb | 3 --
qa/qa/runtime/address.rb | 7 ++++
qa/qa/runtime/feature.rb | 36 ++++++++++++++++
qa/qa/scenario/bootable.rb | 10 ++++-
qa/qa/scenario/shared_attributes.rb | 12 ++++++
qa/qa/scenario/template.rb | 31 ++++++++++++--
qa/qa/scenario/test/instance/all.rb | 1 +
qa/qa/scenario/test/instance/smoke.rb | 1 +
qa/qa/scenario/test/integration/mattermost.rb | 9 ++--
qa/qa/support/api.rb | 3 ++
qa/spec/runtime/feature_spec.rb | 41 +++++++++++++++++++
qa/spec/runtime/scenario_spec.rb | 8 ++++
qa/spec/scenario/bootable_spec.rb | 9 +++-
qa/spec/scenario/template_spec.rb | 28 +++++++++++++
.../scenario/test/integration/github_spec.rb | 2 +-
.../test/integration/mattermost_spec.rb | 11 ++++-
.../scenario_shared_examples.rb | 40 ++++++++++++++----
19 files changed, 259 insertions(+), 26 deletions(-)
create mode 100644 qa/qa/runtime/feature.rb
create mode 100644 qa/qa/scenario/shared_attributes.rb
create mode 100644 qa/spec/runtime/feature_spec.rb
create mode 100644 qa/spec/scenario/template_spec.rb
diff --git a/qa/README.md b/qa/README.md
index 735868e764021..7d66f7d5abc13 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 ec8aef31e48f5..672f4aa928aa9 100644
--- a/qa/qa.rb
+++ b/qa/qa.rb
@@ -17,6 +17,7 @@ module Runtime
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 Scenario
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 98eebac088086..de04467ff5b06 100644
--- a/qa/qa/resource/api_fabricator.rb
+++ b/qa/qa/resource/api_fabricator.rb
@@ -8,9 +8,6 @@ module Resource
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 ffad3974b02e5..af0537dc17c37 100644
--- a/qa/qa/runtime/address.rb
+++ b/qa/qa/runtime/address.rb
@@ -15,6 +15,13 @@ def host
@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 0000000000000..1b4ae7adbbe8c
--- /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 dd12ea6d49275..038418be023b3 100644
--- a/qa/qa/scenario/bootable.rb
+++ b/qa/qa/scenario/bootable.rb
@@ -23,7 +23,7 @@ def launch!(argv)
arguments.parse!(argv)
- self.perform(Runtime::Scenario.attributes, *arguments.default_argv)
+ self.perform(Runtime::Scenario.attributes, *argv)
end
private
@@ -33,7 +33,13 @@ def attribute(name, arg, desc = '')
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 0000000000000..40d5c6b1ff161
--- /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 cb1a1de6b9a49..b8ea26e805e50 100644
--- a/qa/qa/scenario/template.rb
+++ b/qa/qa/scenario/template.rb
@@ -18,19 +18,44 @@ def focus
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 a07c26431bd78..168ac4c09a18f 100644
--- a/qa/qa/scenario/test/instance/all.rb
+++ b/qa/qa/scenario/test/instance/all.rb
@@ -8,6 +8,7 @@ module Test
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 a7d2cb27f276a..43f0623483ee7 100644
--- a/qa/qa/scenario/test/instance/smoke.rb
+++ b/qa/qa/scenario/test/instance/smoke.rb
@@ -8,6 +8,7 @@ module Instance
#
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 ece6fba75c97b..f5072ee227c2c 100644
--- a/qa/qa/scenario/test/integration/mattermost.rb
+++ b/qa/qa/scenario/test/integration/mattermost.rb
@@ -9,10 +9,13 @@ module Integration
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 229bfb44fa5a3..31cff5a241ce1 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 0000000000000..192299b785741
--- /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 7009192bcc0d6..70fc71ffc023b 100644
--- a/qa/spec/runtime/scenario_spec.rb
+++ b/qa/spec/runtime/scenario_spec.rb
@@ -13,6 +13,14 @@
.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 273aac7677e09..bd89b21f7fb22 100644
--- a/qa/spec/scenario/bootable_spec.rb
+++ b/qa/spec/scenario/bootable_spec.rb
@@ -4,14 +4,21 @@
.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 0000000000000..f97fc22daf94b
--- /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 c2aeb1ded1d97..6112ba7c69434 100644
--- a/qa/spec/scenario/test/integration/github_spec.rb
+++ b/qa/spec/scenario/test/integration/github_spec.rb
@@ -12,7 +12,7 @@
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 59caf2ba2cdb4..4e75e72f4d20a 100644
--- a/qa/spec/scenario/test/integration/mattermost_spec.rb
+++ b/qa/spec/scenario/test/integration/mattermost_spec.rb
@@ -4,14 +4,21 @@
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 5fd55d7d96b87..697e6cb39c828 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 @@
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
--
GitLab