# Testing best practices ## Test speed GitLab has a massive test suite that, without [parallelization], can take hours to run. It's important that we make an effort to write tests that are accurate and effective _as well as_ fast. Here are some things to keep in mind regarding test performance: - `double` and `spy` are faster than `FactoryBot.build(...)` - `FactoryBot.build(...)` and `.build_stubbed` are faster than `.create`. - Don't `create` an object when `build`, `build_stubbed`, `attributes_for`, `spy`, or `double` will do. Database persistence is slow! - Don't mark a feature as requiring JavaScript (through `:js` in RSpec) unless it's _actually_ required for the test to be valid. Headless browser testing is slow! [parallelization]: ci.md#test-suite-parallelization-on-the-ci ## RSpec To run rspec tests: ```sh # run all tests bundle exec rspec # run test for path bundle exec rspec spec/[path]/[to]/[spec].rb ``` ### General guidelines - Use a single, top-level `describe ClassName` block. - Use `.method` to describe class methods and `#method` to describe instance methods. - Use `context` to test branching logic. - Try to match the ordering of tests to the ordering within the class. - Try to follow the [Four-Phase Test][four-phase-test] pattern, using newlines to separate phases. - Use `Gitlab.config.gitlab.host` rather than hard coding `'localhost'` - Don't assert against the absolute value of a sequence-generated attribute (see [Gotchas](../gotchas.md#dont-assert-against-the-absolute-value-of-a-sequence-generated-attribute)). - Don't supply the `:each` argument to hooks since it's the default. - On `before` and `after` hooks, prefer it scoped to `:context` over `:all` - When using `evaluate_script("$('.js-foo').testSomething()")` (or `execute_script`) which acts on a given element, use a Capyabara matcher beforehand (e.g. `find('.js-foo')`) to ensure the element actually exists. [four-phase-test]: https://robots.thoughtbot.com/four-phase-test ### System / Feature tests NOTE: **Note:** Before writing a new system test, [please consider **not** writing one](testing_levels.md#consider-not-writing-a-system-test)! - Feature specs should be named `ROLE_ACTION_spec.rb`, such as `user_changes_password_spec.rb`. - Use scenario titles that describe the success and failure paths. - Avoid scenario titles that add no information, such as "successfully". - Avoid scenario titles that repeat the feature title. - Create only the necessary records in the database - Test a happy path and a less happy path but that's it - Every other possible path should be tested with Unit or Integration tests - Test what's displayed on the page, not the internals of ActiveRecord models. For instance, if you want to verify that a record was created, add expectations that its attributes are displayed on the page, not that `Model.count` increased by one. - It's ok to look for DOM elements but don't abuse it since it makes the tests more brittle #### Debugging Capybara Sometimes you may need to debug Capybara tests by observing browser behavior. #### Live debug You can pause Capybara and view the website on the browser by using the `live_debug` method in your spec. The current page will be automatically opened in your default browser. You may need to sign in first (the current user's credentials are displayed in the terminal). To resume the test run, press any key. For example: ``` $ bin/rspec spec/features/auto_deploy_spec.rb:34 Running via Spring preloader in process 8999 Run options: include {:locations=>{"./spec/features/auto_deploy_spec.rb"=>[34]}} Current example is paused for live debugging The current user credentials are: user2 / 12345678 Press any key to resume the execution of the example! Back to the example! . Finished in 34.51 seconds (files took 0.76702 seconds to load) 1 example, 0 failures ``` Note: `live_debug` only works on javascript enabled specs. #### Run `:js` spec in a visible browser Run the spec with `CHROME_HEADLESS=0`, e.g.: ``` CHROME_HEADLESS=0 bundle exec rspec some_spec.rb ``` The test will go by quickly, but this will give you an idea of what's happening. You can also add `byebug` or `binding.pry` to pause execution and [step through](../pry_debugging.md#stepping) the test. #### Screenshots We use the `capybara-screenshot` gem to automatically take a screenshot on failure. In CI you can download these files as job artifacts. Also, you can manually take screenshots at any point in a test by adding the methods below. Be sure to remove them when they are no longer needed! See https://github.com/mattheworiordan/capybara-screenshot#manual-screenshots for more. Add `screenshot_and_save_page` in a `:js` spec to screenshot what Capybara "sees", and save the page source. Add `screenshot_and_open_image` in a `:js` spec to screenshot what Capybara "sees", and automatically open the image. The HTML dumps created by this are missing CSS. This results in them looking very different from the actual application. There is a [small hack](https://gitlab.com/gitlab-org/gitlab-ce/snippets/1718469) to add CSS which makes debugging easier. ### Fast unit tests Some classes are well-isolated from Rails and you should be able to test them without the overhead added by the Rails environment and Bundler's `:default` group's gem loading. In these cases, you can `require 'fast_spec_helper'` instead of `require 'spec_helper'` in your test file, and your test should run really fast since: - Gems loading is skipped - Rails app boot is skipped - gitlab-shell and Gitaly setup are skipped - Test repositories setup are skipped `fast_spec_helper` also support autoloading classes that are located inside the `lib/` directory. It means that as long as your class / module is using only code from the `lib/` directory you will not need to explicitly load any dependencies. `fast_spec_helper` also loads all ActiveSupport extensions, including core extensions that are commonly used in the Rails environment. Note that in some cases, you might still have to load some dependencies using `require_dependency` when a code is using gems or a dependency is not located in `lib/`. For example, if you want to test your code that is calling the `Gitlab::UntrustedRegexp` class, which under the hood uses `re2` library, you should either add `require_dependency 're2'` to files in your library that need `re2` gem, to make this requirement explicit, or you can add it to the spec itself, but the former is preferred. It takes around one second to load tests that are using `fast_spec_helper` instead of 30+ seconds in case of a regular `spec_helper`. ### `let` variables GitLab's RSpec suite has made extensive use of `let` variables to reduce duplication. However, this sometimes [comes at the cost of clarity][lets-not], so we need to set some guidelines for their use going forward: - `let` variables are preferable to instance variables. Local variables are preferable to `let` variables. - Use `let` to reduce duplication throughout an entire spec file. - Don't use `let` to define variables used by a single test; define them as local variables inside the test's `it` block. - Don't define a `let` variable inside the top-level `describe` block that's only used in a more deeply-nested `context` or `describe` block. Keep the definition as close as possible to where it's used. - Try to avoid overriding the definition of one `let` variable with another. - Don't define a `let` variable that's only used by the definition of another. Use a helper method instead. [lets-not]: https://robots.thoughtbot.com/lets-not ### `set` variables In some cases there is no need to recreate the same object for tests again for each example. For example, a project is needed to test issues on the same project, one project will do for the entire file. This can be achieved by using `set` in the same way you would use `let`. `rspec-set` only works on ActiveRecord objects, and before new examples it reloads or recreates the model, _only_ if needed. That is, when you changed properties or destroyed the object. There is one gotcha; you can't reference a model defined in a `let` block in a `set` block. ### Time-sensitive tests [Timecop](https://github.com/travisjeffery/timecop) is available in our Ruby-based tests for verifying things that are time-sensitive. Any test that exercises or verifies something time-sensitive should make use of Timecop to prevent transient test failures. Example: ```ruby it 'is overdue' do issue = build(:issue, due_date: Date.tomorrow) Timecop.freeze(3.days.from_now) do expect(issue).to be_overdue end end ``` ### Pristine test environments The code exercised by a single GitLab test may access and modify many items of data. Without careful preparation before a test runs, and cleanup afterward, data can be changed by a test in such a way that it affects the behaviour of following tests. This should be avoided at all costs! Fortunately, the existing test framework handles most cases already. When the test environment does get polluted, a common outcome is [flaky tests](flaky_tests.md). Pollution will often manifest as an order dependency: running spec A followed by spec B will reliably fail, but running spec B followed by spec A will reliably succeed. In these cases, you can use `rspec --bisect` (or a manual pairwise bisect of spec files) to determine which spec is at fault. Fixing the problem requires some understanding of how the test suite ensures the environment is pristine. Read on to discover more about each data store! #### SQL database This is managed for us by the `database_cleaner` gem. Each spec is surrounded in a transaction, which is rolled back once the test completes. Certain specs will instead issue `DELETE FROM` queries against every table after completion; this allows the created rows to be viewed from multiple database connections, which is important for specs that run in a browser, or migration specs, among others. One consequence of using these strategies, instead of the well-known `TRUNCATE TABLES` approach, is that primary keys and other sequences are **not** reset across specs. So if you create a project in spec A, then create a project in spec B, the first will have `id=1`, while the second will have `id=2`. This means that specs should **never** rely on the value of an ID, or any other sequence-generated column. To avoid accidental conflicts, specs should also avoid manually specifying any values in these kinds of columns. Instead, leave them unspecified, and look up the value after the row is created. #### Redis GitLab stores two main categories of data in Redis: cached items, and sidekiq jobs. In most specs, the Rails cache is actually an in-memory store. This is replaced between specs, so calls to `Rails.cache.read` and `Rails.cache.write` are safe. However, if a spec makes direct Redis calls, it should mark itself with the `:clean_gitlab_redis_cache`, `:clean_gitlab_redis_shared_state` or `:clean_gitlab_redis_queues` traits as appropriate. Sidekiq jobs are typically not run in specs, but this behaviour can be altered in each spec through the use of `Sidekiq::Testing.inline!` blocks. Any spec that causes Sidekiq jobs to be pushed to Redis should use the `:sidekiq` trait, to ensure that they are removed once the spec completes. #### Filesystem Filesystem data can be roughly split into "repositories", and "everything else". Repositories are stored in `tmp/tests/repositories`. This directory is emptied before a test run starts, and after the test run ends. It is not emptied between specs, so created repositories accumulate within this directory over the lifetime of the process. Deleting them is expensive, but this could lead to pollution unless carefully managed. To avoid this, [hashed storage](../../administration/repository_storage_types.md) is enabled in the test suite. This means that repositories are given a unique path that depends on their project's ID. Since the project IDs are not reset between specs, this guarantees that each spec gets its own repository on disk, and prevents changes from being visible between specs. If a spec manually specifies a project ID, or inspects the state of the `tmp/tests/repositories/` directory directly, then it should clean up the directory both before and after it runs. In general, these patterns should be completely avoided. Other classes of file linked to database objects, such as uploads, are generally managed in the same way. With hashed storage enabled in the specs, they are written to disk in locations determined by ID, so conflicts should not occur. Some specs disable hashed storage by passing the `:legacy_storage` trait to the `projects` factory. Specs that do this must **never** override the `path` of the project, or any of its groups. The default path includes the project ID, so will not conflict; but if two specs create a `:legacy_storage` project with the same path, they will use the same repository on disk and lead to test environment pollution. Other files must be managed manually by the spec. If you run code that creates a `tmp/test-file.csv` file, for instance, the spec must ensure that the file is removed as part of cleanup. #### Persistent in-memory application state All the specs in a given `rspec` run share the same Ruby process, which means they can affect each other by modifying Ruby objects that are accessible between specs. In practice, this means global variables, and constants (which includes Ruby classes, modules, etc). Global variables should generally not be modified. If absolutely necessary, a block like this can be used to ensure the change is rolled back afterwards: ```ruby around(:each) do |example| old_value = $0 begin $0 = "new-value" example.run ensure $0 = old_value end end ``` If a spec needs to modify a constant, it should use the `stub_const` helper to ensure the change is rolled back. If you need to modify the contents of the `ENV` constant, you can use the `stub_env` helper method instead. While most Ruby **instances** are not shared between specs, **classes** and **modules** generally are. Class and module instance variables, accessors, class variables, and other stateful idioms, should be treated in the same way as global variables - don't modify them unless you have to! In particular, prefer using expectations, or dependency injection along with stubs, to avoid the need for modifications. If you have no other choice, an `around` block similar to the example for global variables, above, can be used, but this should be avoided if at all possible. ### Table-based / Parameterized tests This style of testing is used to exercise one piece of code with a comprehensive range of inputs. By specifying the test case once, alongside a table of inputs and the expected output for each, your tests can be made easier to read and more compact. We use the [rspec-parameterized](https://github.com/tomykaira/rspec-parameterized) gem. A short example, using the table syntax and checking Ruby equality for a range of inputs, might look like this: ```ruby describe "#==" do using RSpec::Parameterized::TableSyntax let(:project1) { create(:project) } let(:project2) { create(:project) } where(:a, :b, :result) do 1 | 1 | true 1 | 2 | false true | true | true true | false | false project1 | project1 | true project2 | project2 | true project 1 | project2 | false end with_them do it { expect(a == b).to eq(result) } it 'is isomorphic' do expect(b == a).to eq(result) end end end ``` ### Prometheus tests Prometheus metrics may be preserved from one test run to another. To ensure that metrics are reset before each example, add the `:prometheus` tag to the Rspec test. ### Matchers Custom matchers should be created to clarify the intent and/or hide the complexity of RSpec expectations.They should be placed under `spec/support/matchers/`. Matchers can be placed in subfolder if they apply to a certain type of specs only (e.g. features, requests etc.) but shouldn't be if they apply to multiple type of specs. #### `have_gitlab_http_status` Prefer `have_gitlab_http_status` over `have_http_status` because the former could also show the response body whenever the status mismatched. This would be very useful whenever some tests start breaking and we would love to know why without editing the source and rerun the tests. This is especially useful whenever it's showing 500 internal server error. ### Shared contexts All shared contexts should be placed under `spec/support/shared_contexts/`. Shared contexts can be placed in subfolder if they apply to a certain type of specs only (e.g. features, requests etc.) but shouldn't be if they apply to multiple type of specs. Each file should include only one context and have a descriptive name, e.g. `spec/support/shared_contexts/controllers/githubish_import_controller_shared_context.rb`. ### Shared examples All shared examples should be placed under `spec/support/shared_examples/`. Shared examples can be placed in subfolder if they apply to a certain type of specs only (e.g. features, requests etc.) but shouldn't be if they apply to multiple type of specs. Each file should include only one context and have a descriptive name, e.g. `spec/support/shared_examples/controllers/githubish_import_controller_shared_example.rb`. ### Helpers Helpers are usually modules that provide some methods to hide the complexity of specific RSpec examples. You can define helpers in RSpec files if they're not intended to be shared with other specs. Otherwise, they should be placed under `spec/support/helpers/`. Helpers can be placed in subfolder if they apply to a certain type of specs only (e.g. features, requests etc.) but shouldn't be if they apply to multiple type of specs. Helpers should follow the Rails naming / namespacing convention. For instance `spec/support/helpers/cycle_analytics_helpers.rb` should define: ```ruby module Spec module Support module Helpers module CycleAnalyticsHelpers def create_commit_referencing_issue(issue, branch_name: random_git_name) project.repository.add_branch(user, branch_name, 'master') create_commit("Commit for ##{issue.iid}", issue.project, user, branch_name) end end end end end ``` Helpers should not change the RSpec config. For instance, the helpers module described above should not include: ```ruby RSpec.configure do |config| config.include Spec::Support::Helpers::CycleAnalyticsHelpers end ``` ### Factories GitLab uses [factory_bot] as a test fixture replacement. - Factory definitions live in `spec/factories/`, named using the pluralization of their corresponding model (`User` factories are defined in `users.rb`). - There should be only one top-level factory definition per file. - FactoryBot methods are mixed in to all RSpec groups. This means you can (and should) call `create(...)` instead of `FactoryBot.create(...)`. - Make use of [traits] to clean up definitions and usages. - When defining a factory, don't define attributes that are not required for the resulting record to pass validation. - When instantiating from a factory, don't supply attributes that aren't required by the test. - Factories don't have to be limited to `ActiveRecord` objects. [See example](https://gitlab.com/gitlab-org/gitlab-ce/commit/0b8cefd3b2385a21cfed779bd659978c0402766d). [factory_bot]: https://github.com/thoughtbot/factory_bot [traits]: http://www.rubydoc.info/gems/factory_bot/file/GETTING_STARTED.md#Traits ### Fixtures All fixtures should be placed under `spec/fixtures/`. ### Repositories Testing some functionality, e.g., merging a merge request, requires a git repository with a certain state to be present in the test environment. GitLab maintains the [gitlab-test](https://gitlab.com/gitlab-org/gitlab-test) repository for certain common cases - you can ensure a copy of the repository is used with the `:repository` trait for project factories: ```ruby let(:project) { create(:project, :repository) } ``` Where you can, consider using the `:custom_repo` trait instead of `:repository`. This allows you to specify exactly what files will appear in the `master` branch of the project's repository. For example: ```ruby let(:project) do create( :project, :custom_repo, files: { 'README.md' => 'Content here', 'foo/bar/baz.txt' => 'More content here' } ) end ``` This will create a repository containing two files, with default permissions and the specified content. ### Config RSpec config files are files that change the RSpec config (i.e. `RSpec.configure do |config|` blocks). They should be placed under `spec/support/`. Each file should be related to a specific domain, e.g. `spec/support/capybara.rb`, `spec/support/carrierwave.rb`, etc. If a helpers module applies only to a certain kind of specs, it should add modifiers to the `config.include` call. For instance if `spec/support/helpers/cycle_analytics_helpers.rb` applies to `:lib` and `type: :model` specs only, you would write the following: ```ruby RSpec.configure do |config| config.include Spec::Support::Helpers::CycleAnalyticsHelpers, :lib config.include Spec::Support::Helpers::CycleAnalyticsHelpers, type: :model end ``` If a config file only consists of `config.include`, you can add these `config.include` directly in `spec/spec_helper.rb`. For very generic helpers, consider including them in the `spec/support/rspec.rb` file which is used by the `spec/fast_spec_helper.rb` file. See [Fast unit tests](#fast-unit-tests) for more details about the `spec/fast_spec_helper.rb` file. --- [Return to Testing documentation](index.md)