Skip to content
Snippets Groups Projects

Run all new backend specs in random order

Merged Peter Leitzen requested to merge pl-rspec-order into master
1 unresolved thread

What does this MR do and why?

With this MR all :new: specs are run in random order to surface flaky tests that are dependent on test order.

All (minus spec/rubocop - see !93143 (merged)) old specs are still run in defined order and excluded via spec/support/rspec_order_todo.yml (10930 files).

Note that the example group description ends with " (random)" if run in random order to help debugging spec failures. See example below.

This MR also enables all RuboCop specs to be run in random order. This has been checked via scripts/rspec_check_order_dependence spec/rubocop which checks passed spec files and deletes them from spec/support/rspec_order_todo.yml on success :tada:. See screenshot below.

Refs #337399 (closed).

Mixed mode

Note that running specs in random and defined order still works just fine because randomization happens on example group basis.

For example:

bin/rspec spec/lib/gitlab_spec.rb spec/rubocop/cop_todo_spec.rb
Gitlab
  delegates root to GitlabEdition
  delegates jh? to GitlabEdition
  delegates ee? to GitlabEdition
  delegates extensions to GitlabEdition
...

RuboCop::CopTodo (random)
  #to_yaml
    when previously disabled
...

Gitlab example group is run in defined order where as RuboCop::CopTodo (random) (note the (random) appendix is run in random order.

Reverse order?

When passing RSPEC_ORDER=reverse you can run specs in reverse order which is used by scripts/rspec_check_order_dependence.

Reproducibility

In order to reproduce spec failures due to order issues you can use the seed via --seed SEED. Example: Randomized with seed 27443.

qa/?

QA specs are already run in random order and are out of scope of this MR.

Screenshots or screen recordings

scripts/rspec_check_order_dependence spec/rubocop - see !93143 (merged)
Screenshot_from_2022-07-23_15-33-42

How to set up and validate locally

bin/rspec --format documentation spec/rubocop

Run options: include {:focus=>true}

All examples were filtered out; ignoring {:focus=>true}

Randomized with seed 22503

RuboCop::Cop::Migration::Datetime (random)
  when in migration
    registers an offense when the ":datetime" data type is used on add_column
    registers an offense when the ":timestamp" data type is used on create_table
    does not register an offense when the ":datetime_with_timezone" data type is used on create_table
    does not register an offense when the ":datetime" data type is not used on add_column
    does not register an offense when the ":datetime_with_timezone" data type is used on add_column
    registers an offense when the ":timestamp" data type is used on add_column
    does not register an offense when the ":datetime" data type is not used on create_table
    registers an offense when the ":datetime" data type is used on create_table
  when outside of migration
    registers no offense

RuboCop::Cop::AvoidReturnFromBlocks (random)
  doesn't create more than one offense for nested blocks
  doesn't flag violation for return used inside a method definition
  doesn't flag violation for next inside a block
  doesn't check when block is empty
  doesn't flag violation for return inside a lambda
  flags violation for return inside a block
  flags violation for return inside included > def > block
  doesn't flag violation for break inside a block
  behaves like examples with def methods
    doesn't flag violation for return inside define_method
  behaves like examples with def methods
    doesn't flag violation for return inside lambda
  behaves like examples with whitelisted method
    doesn't flag violation for return inside loop
  behaves like examples with whitelisted method
    doesn't flag violation for return inside times
  behaves like examples with whitelisted method
    doesn't flag violation for return inside each
  behaves like examples with whitelisted method
    doesn't flag violation for return inside each_filename
...

Note the (random) appendix to each example group to denote that it's run in random order.

TODO YAML

Generated via

find spec ee/spec -type f -name "*.rb" | grep -E "_spec\.rb$|frontend/fixtures" | sort | perl -lne "print qq{- './\$_'}" > spec/support/rspec_order_todo.yml

Current TODOs are:

  • All spec files ending with _spec.rb
  • All frontend fixtures (matching frontend/fixtures)

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Peter Leitzen

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Peter Leitzen
  • Peter Leitzen
  • Peter Leitzen changed the description

    changed the description

  • Peter Leitzen changed the description

    changed the description

  • Peter Leitzen added 3 commits

    added 3 commits

    • eb35ec85 - Run all new backend specs in random order
    • 851b7773 - Add script to check order dependency of RSpec examples
    • 60d9ca78 - Add a new section "Test order" to Testing Guide's Best Practices

    Compare with previous version

  • Peter Leitzen added 1 commit

    added 1 commit

    • f3076ee9 - Add a new section "Test order" to Testing Guide's Best Practices

    Compare with previous version

  • Peter Leitzen changed the description

    changed the description

  • Peter Leitzen added 1221 commits

    added 1221 commits

    • f3076ee9...d0459d2e - 1218 commits from branch master
    • d053e904 - Run all new backend specs in random order
    • d5e0baca - Add script to check order dependency of RSpec examples
    • ceda61f7 - Add a new section "Test order" to Testing Guide's Best Practices

    Compare with previous version

  • Author Maintainer

    :wave: @godfat-gitlab

    Since you were already involved in discussions, can you do the first review? :pray:

  • requested review from @godfat-gitlab

  • Peter Leitzen added 3 commits

    added 3 commits

    • 429ee59a - Run all new backend specs in random order
    • 5c69e4e8 - Add script to check order dependency of RSpec examples
    • ddbd204c - Add a new section "Test order" to Testing Guide's Best Practices

    Compare with previous version

  • Peter Leitzen changed the description

    changed the description

  • Lin Jen-Shin
  • Lin Jen-Shin
  • Lin Jen-Shin
  • Lin Jen-Shin
  • Lin Jen-Shin removed review request for @godfat-gitlab

    removed review request for @godfat-gitlab

  • Peter Leitzen added 135 commits

    added 135 commits

    • ddbd204c...5aba5edb - 132 commits from branch master
    • 4bd3983d - Run all new backend specs in random order
    • bda61c69 - Add script to check order dependency of RSpec examples
    • f974beb4 - Add a new section "Test order" to Testing Guide's Best Practices

    Compare with previous version

  • requested review from @godfat-gitlab

  • Peter Leitzen added 1 commit

    added 1 commit

    • b7c022a3 - Add a new section "Test order" to Testing Guide's Best Practices

    Compare with previous version

  • Lin Jen-Shin removed review request for @godfat-gitlab

    removed review request for @godfat-gitlab

  • Peter Leitzen added 1 commit

    added 1 commit

    • 7fdad77d - Apply 1 suggestion(s) to 1 file(s)

    Compare with previous version

  • requested review from @godfat-gitlab

  • Peter Leitzen added 995 commits

    added 995 commits

    • 7fdad77d...169e9b61 - 992 commits from branch master
    • 8ccf8639 - Run all new backend specs in random order
    • f0758e72 - Add script to check order dependency of RSpec examples
    • 217e56d5 - Add a new section "Test order" to Testing Guide's Best Practices

    Compare with previous version

  • Peter Leitzen
  • Peter Leitzen added 2 commits

    added 2 commits

    • e4531512 - Add script to check order dependency of RSpec examples
    • 54d07556 - Add a new section "Test order" to Testing Guide's Best Practices

    Compare with previous version

  • Lin Jen-Shin approved this merge request

    approved this merge request

  • :wave: @godfat-gitlab, thanks for approving this merge request.

    This is the first time the merge request is approved. To ensure full test coverage, a new pipeline has been started.

    For more info, please refer to the following links:

  • Lin Jen-Shin requested review from @rymai and removed review request for @godfat-gitlab

    requested review from @rymai and removed review request for @godfat-gitlab

  • Rémy Coutable
  • Rémy Coutable
  • Rémy Coutable removed review request for @rymai

    removed review request for @rymai

  • Peter Leitzen requested review from @rymai

    requested review from @rymai

  • Peter Leitzen added 583 commits

    added 583 commits

    • 54d07556...3d24bfa7 - 580 commits from branch master
    • b48cbf81 - Run all new backend specs in random order
    • 82bd8a97 - Add script to check order dependency of RSpec examples
    • 93cee6dd - Add a new section "Test order" to Testing Guide's Best Practices

    Compare with previous version

  • Author Maintainer

    Rebased against latest master regenerated the TODO list to stay as close as possible to current master.

  • Peter Leitzen changed the description

    changed the description

  • Peter Leitzen added 594 commits

    added 594 commits

    • 93cee6dd...d25f16b3 - 591 commits from branch master
    • 83885682 - Run all new backend specs in random order
    • 0d3008cb - Add script to check order dependency of RSpec examples
    • c29334ec - Add a new section "Test order" to Testing Guide's Best Practices

    Compare with previous version

  • Peter Leitzen
  • Peter Leitzen mentioned in issue #350700

    mentioned in issue #350700

  • 🤖 GitLab Bot 🤖 changed milestone to %15.4

    changed milestone to %15.4

  • Peter Leitzen added 744 commits

    added 744 commits

    • c29334ec...4fa1f8e2 - 741 commits from branch master
    • dfceae5d - Run all new backend specs in random order
    • f7b86604 - Add script to check order dependency of RSpec examples
    • c92c9ef6 - Add a new section "Test order" to Testing Guide's Best Practices

    Compare with previous version

  • Rémy Coutable approved this merge request

    approved this merge request

  • Rémy Coutable resolved all threads

    resolved all threads

  • 1 Warning
    :warning: This merge request is definitely too big (11475 lines changed), please split it into multiple merge requests.
    2 Messages
    :book: CHANGELOG missing:

    If you want to create a changelog entry for GitLab FOSS, add the Changelog trailer to the commit message you want to add to the changelog.

    If you want to create a changelog entry for GitLab EE, also add the EE: true trailer to your commit message.

    If this merge request doesn't need a CHANGELOG entry, feel free to ignore this message.

    :book: This merge request adds or changes documentation files. A review from the Technical Writing team before you merge is recommended. Reviews can happen after you merge.

    Documentation review

    The following files require a review from a technical writer:

    • doc/development/testing_guide/best_practices.md

    The review does not need to block merging this merge request. See the:

    Reviewer roulette

    Changes that require review have been detected!

    Please refer to the table below for assigning reviewers and maintainers suggested by Danger in the specified category:

    Category Reviewer Maintainer
    backend Felipe Artur (@felipe_artur) (UTC-3, 5 hours behind @splattael) Jan Provaznik (@jprovaznik) (UTC+2, same timezone as @splattael)
    maintenanceworkflow / maintenancepipelines for CI, Danger Aman Luthra (@aluthra2) (UTC+5.5, 3.5 hours ahead of @splattael) Jan Provaznik (@jprovaznik) (UTC+2, same timezone as @splattael)

    To spread load more evenly across eligible reviewers, Danger has picked a candidate for each review slot, based on their timezone. Feel free to override these selections if you think someone else would be better-suited or use the GitLab Review Workload Dashboard to find other available reviewers.

    To read more on how to use the reviewer roulette, please take a look at the Engineering workflow and code review guidelines. Please consider assigning a reviewer or maintainer who is a domain expert in the area of the merge request.

    Once you've decided who will review this merge request, assign them as a reviewer! Danger does not automatically notify them for you.

    If needed, you can retry the :repeat: danger-review job that generated this comment.

    Generated by :no_entry_sign: Danger

  • Rémy Coutable enabled an automatic merge when the pipeline for f261161b succeeds

    enabled an automatic merge when the pipeline for f261161b succeeds

  • Peter Leitzen resolved all threads

    resolved all threads

  • Rémy Coutable mentioned in commit bf8ef431

    mentioned in commit bf8ef431

  • Peter Leitzen mentioned in merge request !93143 (merged)

    mentioned in merge request !93143 (merged)

  • added workflowstaging label and removed workflowcanary label

  • Author Maintainer

    Announced in #backend, #backend_maintainers, #development and Engineering: Week in Review.

  • Peter Leitzen mentioned in merge request !95640 (merged)

    mentioned in merge request !95640 (merged)

  • Peter Leitzen mentioned in merge request !96137 (merged)

    mentioned in merge request !96137 (merged)

  • 33 end
    34 end
    35
    36 RSpec.configure do |config|
    37 # Useful to find order-dependent specs.
    38 config.register_ordering(:reverse, &:reverse)
    39
    40 # Randomization can be reproduced across test runs.
    41 Kernel.srand config.seed
    42
    43 config.on_example_group_definition do |example_group|
    44 order = Support::RspecOrder.order_for(example_group)
    45
    46 if order
    47 example_group.metadata[:order] = order.to_sym
    48 example_group.metadata[:description] += " (order #{order})"
  • Peter Leitzen mentioned in issue #407877

    mentioned in issue #407877

  • Author Maintainer

    #407877 which tracks that all backend specs should be run in random order.

  • Mohamed Hamda mentioned in merge request !124300 (merged)

    mentioned in merge request !124300 (merged)

  • Peter Leitzen mentioned in merge request !157448 (merged)

    mentioned in merge request !157448 (merged)

  • Please register or sign in to reply
    Loading