Enable Deprecation Toolkit for qa tests
What does this MR do?
In order to prepare our code base to play nice with Ruby 3, we started to fail fast in CI whenever we use deprecated features such as hashes in place of keyword arguments. To detect these early, we use the deprecation_toolkit
gem for our main test suite, but it wasn't enabled for the test suite in qa/
.
This MR:
- enables the deprecation toolkit for
qa
specs, so that we catch them in CI - fixes existing kwargs violations in our QA code
- updates
rspec
to 3.10 to resolve library-internal Ruby 3 deprecations
Broken out of !50640 (closed)
Example
A failing spec would typically look like this:
git@6e1226f9d8b9:~/gitlab/qa$ WEBDRIVER_HEADLESS=1 bundle exec rspec spec/runtime/feature_spec.rb:93
warning: parser/current is loading parser/ruby27, which recognizes
warning: 2.7.1-compliant syntax, but you are running 2.7.2.
warning: please see https://github.com/whitequark/parser#compatibility-with-ruby-mri.
/data/cache/bundle-2.7.2/ruby/2.7.0/gems/capybara-3.35.3/lib/capybara/registration_container.rb:21: warning: DEPRECATED: Calling 'include?' on the drivers/servers container is deprecated without replacement
DEPRECATION WARNING: CHROME_HEADLESS is deprecated. Use WEBDRIVER_HEADLESS instead. (called from block in configure! at /home/git/gitlab/qa/qa/runtime/browser.rb:94)
2021-08-03 07:07:39 / CONF ::
==> Base URL:
==> Browser: #<Selenium::WebDriver::Chrome::Driver:0x00007f3e65816730>
==> Libraries: Chemlab::Vendor
/data/cache/bundle-2.7.2/ruby/2.7.0/gems/chemlab-0.7.2/lib/chemlab/configuration.rb:79: warning: already initialized constant Chemlab::Vendor
/data/cache/bundle-2.7.2/ruby/2.7.0/gems/chemlab-0.7.2/lib/chemlab/configuration.rb:79: warning: previous definition of Vendor was here
Run options: include {:locations=>{"./spec/runtime/feature_spec.rb"=>[93]}}
Randomized with seed 39580
QA::Runtime::Feature
feature_flag: "a_flag"
.enable
when a group scope is provided
behaves like enables a feature flag
DEPRECATION WARNING: /home/git/gitlab/qa/spec/runtime/feature_spec.rb:31: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call
/home/git/gitlab/qa/qa/runtime/feature.rb:25: warning: The called method `enable' is defined here
(called from block (4 levels) in <top (required)> at /home/git/gitlab/qa/spec/runtime/feature_spec.rb:31)
enables a feature flag for a scope (FAILED - 1)
feature_flag: :a_flag
.enable
when a group scope is provided
behaves like enables a feature flag
enables a feature flag for a scope
Failures:
1) QA::Runtime::Feature feature_flag: "a_flag" .enable when a group scope is provided behaves like enables a feature flag enables a feature flag for a scope
Failure/Error: raise error_class.new(current_deprecations, recorded_deprecations)
DeprecationToolkit::Behaviors::DeprecationIntroduced:
You have introduced new deprecations in the codebase. Fix or record them in order to discard this error.
You can record deprecations by adding the `DEPRECATION_BEHAVIOR='record'` ENV when running your specs.
DEPRECATION WARNING: /home/git/gitlab/qa/spec/runtime/feature_spec.rb:31: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call
/home/git/gitlab/qa/qa/runtime/feature.rb:25: warning: The called method `enable' is defined here
(called from block (4 levels) in <top (required)> at /home/git/gitlab/qa/spec/runtime/feature_spec.rb:31)
Shared Example Group: "enables a feature flag" called from ./spec/runtime/feature_spec.rb:93
# /data/cache/bundle-2.7.2/ruby/2.7.0/gems/deprecation_toolkit-1.5.1/lib/deprecation_toolkit/behaviors/raise.rb:15:in `trigger'
# /data/cache/bundle-2.7.2/ruby/2.7.0/gems/deprecation_toolkit-1.5.1/lib/deprecation_toolkit/test_triggerer.rb:9:in `trigger_deprecation_toolkit_behavior'
# /data/cache/bundle-2.7.2/ruby/2.7.0/gems/deprecation_toolkit-1.5.1/lib/deprecation_toolkit/rspec.rb:7:in `block (2 levels) in <top (required)>'
# /data/cache/bundle-2.7.2/ruby/2.7.0/gems/rspec-retry-0.6.1/lib/rspec/retry.rb:123:in `block in run'
# /data/cache/bundle-2.7.2/ruby/2.7.0/gems/rspec-retry-0.6.1/lib/rspec/retry.rb:110:in `loop'
# /data/cache/bundle-2.7.2/ruby/2.7.0/gems/rspec-retry-0.6.1/lib/rspec/retry.rb:110:in `run'
# /data/cache/bundle-2.7.2/ruby/2.7.0/gems/rspec-retry-0.6.1/lib/rspec_ext/rspec_ext.rb:12:in `run_with_retry'
# /data/cache/bundle-2.7.2/ruby/2.7.0/gems/rspec-retry-0.6.1/lib/rspec/retry.rb:37:in `block (2 levels) in setup'
Top 2 slowest examples (0.02269 seconds, 66.6% of total time):
QA::Runtime::Feature feature_flag: "a_flag" .enable when a group scope is provided behaves like enables a feature flag enables a feature flag for a scope
0.02082 seconds ./spec/runtime/feature_spec.rb:18
QA::Runtime::Feature feature_flag: :a_flag .enable when a group scope is provided behaves like enables a feature flag enables a feature flag for a scope
0.00187 seconds ./spec/runtime/feature_spec.rb:18
Finished in 0.03409 seconds (files took 1.88 seconds to load)
2 examples, 1 failure
However, since the problem may occur deep in the application stack, it might not surface the same way in an end-to-end test, since this exception will instead cause Selenium to exhaust retries. In this case, one must look in application logs for this error to diagnose the problem, both deprecation warnings logged and the stacktrace itself give away what the problem is.
Does this MR meet the acceptance criteria?
Conformity
-
I have included changelog trailers, or none are needed. (Does this MR need a changelog?) -
I have added/updated documentation, or it's not needed. (Is documentation required?) -
I have properly separated EE content from FOSS, or this MR is FOSS only. (Where should EE code go?) -
I have added information for database reviewers in the MR description, or it's not needed. (Does this MR have database related changes?) -
I have self-reviewed this MR per code review guidelines. -
This MR does not harm performance, or I have asked a reviewer to help assess the performance impact. (Merge request performance guidelines) -
I have followed the style guides. -
This change is backwards compatible across updates, or this does not apply.
Availability and Testing
-
I have added/updated tests following the Testing Guide, or it's not needed. (Consider all test levels. See the Test Planning Process.) -
I have tested this MR in all supported browsers, or it's not needed. -
I have informed the Infrastructure department of a default or new setting change per definition of done, or it's not needed.