Refactor the Development documentation, and divide the Testing documentation into multiple pages

This commit is contained in:
Rémy Coutable 2017-10-11 05:59:34 +00:00 committed by Achilleas Pipinellis
parent 150f58decb
commit 676840ff9c
14 changed files with 1027 additions and 876 deletions

View file

@ -296,9 +296,9 @@ might be edited to make them small and simple.
Please submit Feature Proposals using the ['Feature Proposal' issue template](.gitlab/issue_templates/Feature Proposal.md) provided on the issue tracker.
For changes in the interface, it is helpful to include a mockup. Issues that add to, or change, the interface should
be given the ~"UX" label. This will allow the UX team to provide input and guidance. You may
need to ask one of the [core team] members to add the label, if you do not have permissions to do it by yourself.
For changes in the interface, it is helpful to include a mockup. Issues that add to, or change, the interface should
be given the ~"UX" label. This will allow the UX team to provide input and guidance. You may
need to ask one of the [core team] members to add the label, if you do not have permissions to do it by yourself.
If you want to create something yourself, consider opening an issue first to
discuss whether it is interesting to include this in GitLab.
@ -658,7 +658,7 @@ available at [http://contributor-covenant.org/version/1/1/0/](http://contributor
[license-finder-doc]: doc/development/licensing.md
[GitLab Inc engineering workflow]: https://about.gitlab.com/handbook/engineering/workflow/#labelling-issues
[polling-etag]: https://docs.gitlab.com/ce/development/polling.html
[testing]: doc/development/testing.md
[testing]: doc/development/testing_guide/index.md
[^1]: Please note that specs other than JavaScript specs are considered backend
code.

View file

@ -1,4 +1,4 @@
## GitLab Core Team & GitLab Inc. Contribution Process
## GitLab core team & GitLab Inc. contribution process
---

View file

@ -1,75 +1,89 @@
# Development
# GitLab development guides
## Outside of docs
## Get started!
- [CONTRIBUTING.md](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md) main contributing guide
- [PROCESS.md](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/PROCESS.md) contributing process
- [GitLab Development Kit (GDK)](https://gitlab.com/gitlab-org/gitlab-development-kit/blob/master/doc/howto/README.md) to install a development version
- Setup GitLab's development environment with [GitLab Development Kit (GDK)](https://gitlab.com/gitlab-org/gitlab-development-kit/blob/master/doc/howto/README.md)
- [GitLab contributing guide](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md)
- [Architecture](architecture.md) of GitLab
- [Rake tasks](rake_tasks.md) for development
## Styleguides
## Processes
- [GitLab core team & GitLab Inc. contribution process](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/PROCESS.md)
- [Generate a changelog entry with `bin/changelog`](changelog.md)
- [Code review guidelines](code_review.md) for reviewing code and having code reviewed.
- [Limit conflicts with EE when developing on CE](limit_ee_conflicts.md)
## UX and frontend guides
- [API styleguide](api_styleguide.md) Use this styleguide if you are
contributing to the API.
- [Documentation styleguide](doc_styleguide.md) Use this styleguide if you are
contributing to documentation.
- [Writing documentation](writing_documentation.md)
- [Distinction between general documentation and technical articles](writing_documentation.md#distinction-between-general-documentation-and-technical-articles)
- [SQL Migration Style Guide](migration_style_guide.md) for creating safe SQL migrations
- [Testing standards and style guidelines](testing.md)
- [UX guide](ux_guide/index.md) for building GitLab with existing CSS styles and elements
- [Frontend guidelines](fe_guide/index.md)
- [SQL guidelines](sql.md) for working with SQL queries
## Backend guides
- [Testing standards and style guidelines](testing_guide/index.md)
- [API styleguide](api_styleguide.md) Use this styleguide if you are
contributing to the API.
- [Sidekiq guidelines](sidekiq_style_guide.md) for working with Sidekiq workers
- [Working with Gitaly](gitaly.md)
- [Manage feature flags](feature_flags.md)
- [View sent emails or preview mailers](emails.md)
- [Shell commands](shell_commands.md) in the GitLab codebase
- [`Gemfile` guidelines](gemfile.md)
- [Sidekiq debugging](sidekiq_debugging.md)
- [Gotchas](gotchas.md) to avoid
- [Issue and merge requests state models](object_state_models.md)
- [How to dump production data to staging](db_dump.md)
## Process
## Performance guides
- [Generate a changelog entry with `bin/changelog`](changelog.md)
- [Limit conflicts with EE when developing on CE](limit_ee_conflicts.md)
- [Code review guidelines](code_review.md) for reviewing code and having code reviewed.
- [Instrumentation](instrumentation.md)
- [Performance guidelines](performance.md)
- [Merge request performance guidelines](merge_request_performance_guidelines.md)
for ensuring merge requests do not negatively impact GitLab performance
## Backend howtos
## Databases guides
- [Architecture](architecture.md) of GitLab
- [Gotchas](gotchas.md) to avoid
- [How to dump production data to staging](db_dump.md)
- [Instrumentation](instrumentation.md)
- [Performance guidelines](performance.md)
- [Rake tasks](rake_tasks.md) for development
- [Shell commands](shell_commands.md) in the GitLab codebase
- [Sidekiq debugging](sidekiq_debugging.md)
- [Object state models](object_state_models.md)
- [Building a package for testing purposes](build_test_package.md)
- [Manage feature flags](feature_flags.md)
- [View sent emails or preview mailers](emails.md)
- [Working with Gitaly](gitaly.md)
### Migrations
## Databases
- [Merge Request Checklist](database_merge_request_checklist.md)
- [What requires downtime?](what_requires_downtime.md)
- [Adding database indexes](adding_database_indexes.md)
- [Post Deployment Migrations](post_deployment_migrations.md)
- [Foreign Keys & Associations](foreign_keys.md)
- [Serializing Data](serializing_data.md)
- [Polymorphic Associations](polymorphic_associations.md)
- [Single Table Inheritance](single_table_inheritance.md)
- [Background Migrations](background_migrations.md)
- [Storing SHA1 Hashes As Binary](sha1_as_binary.md)
- [Iterating Tables In Batches](iterating_tables_in_batches.md)
- [Ordering Table Columns](ordering_table_columns.md)
- [Verifying Database Capabilities](verifying_database_capabilities.md)
- [Hash Indexes](hash_indexes.md)
- [Swapping Tables](swapping_tables.md)
- [SQL guidelines](sql.md) for working with SQL queries
- [Migrations style guide](migration_style_guide.md) for creating safe SQL migrations
- [Post deployment migrations](post_deployment_migrations.md)
- [Background migrations](background_migrations.md)
- [Swapping tables](swapping_tables.md)
## Internationalization (i18n)
### Best practices
- [Merge Request checklist](database_merge_request_checklist.md)
- [Adding database indexes](adding_database_indexes.md)
- [Foreign keys & associations](foreign_keys.md)
- [Single table inheritance](single_table_inheritance.md)
- [Polymorphic associations](polymorphic_associations.md)
- [Serializing data](serializing_data.md)
- [Hash indexes](hash_indexes.md)
- [Storing SHA1 hashes as binary](sha1_as_binary.md)
- [Iterating tables in batches](iterating_tables_in_batches.md)
- [Ordering table columns](ordering_table_columns.md)
- [Verifying database capabilities](verifying_database_capabilities.md)
## Documentation guides
- [Documentation styleguide](doc_styleguide.md): Use this styleguide if you are
contributing to the documentation.
- [Writing documentation](writing_documentation.md)
- [Distinction between general documentation and technical articles](writing_documentation.md#distinction-between-general-documentation-and-technical-articles)
## Internationalization (i18n) guides
- [Introduction](i18n/index.md)
- [Externalization](i18n/externalization.md)
- [Translation](i18n/translation.md)
## Build guides
- [Building a package for testing purposes](build_test_package.md)
## Compliance
- [Licensing](licensing.md) for ensuring license compliance

View file

@ -54,7 +54,8 @@ or make changes to our frontend development guidelines.
---
## [Testing](testing.md)
## [Testing](../testing_guide/frontend_testing.md)
How we write frontend tests, run the GitLab test suite, and debug test related
issues.

View file

@ -1,254 +1 @@
# Frontend Testing
There are two types of test suites you'll encounter while developing frontend code
at GitLab. We use Karma and Jasmine for JavaScript unit and integration testing, and RSpec
feature tests with Capybara for e2e (end-to-end) integration testing.
Unit and feature tests need to be written for all new features.
Most of the time, you should use rspec for your feature tests.
There are cases where the behaviour you are testing is not worth the time spent running the full application,
for example, if you are testing styling, animation, edge cases or small actions that don't involve the backend,
you should write an integration test using Jasmine.
![Testing priority triangle](img/testing_triangle.png)
_This diagram demonstrates the relative priority of each test type we use_
Regression tests should be written for bug fixes to prevent them from recurring in the future.
See [the Testing Standards and Style Guidelines](../testing.md)
for more information on general testing practices at GitLab.
## Karma test suite
GitLab uses the [Karma][karma] test runner with [Jasmine][jasmine] as its test
framework for our JavaScript unit and integration tests. For integration tests,
we generate HTML files using RSpec (see `spec/javascripts/fixtures/*.rb` for examples).
Some fixtures are still HAML templates that are translated to HTML files using the same mechanism (see `static_fixtures.rb`).
Adding these static fixtures should be avoided as they are harder to keep up to date with real views.
The existing static fixtures will be migrated over time.
Please see [gitlab-org/gitlab-ce#24753](https://gitlab.com/gitlab-org/gitlab-ce/issues/24753) to track our progress.
Fixtures are served during testing by the [jasmine-jquery][jasmine-jquery] plugin.
JavaScript tests live in `spec/javascripts/`, matching the folder structure
of `app/assets/javascripts/`: `app/assets/javascripts/behaviors/autosize.js`
has a corresponding `spec/javascripts/behaviors/autosize_spec.js` file.
Keep in mind that in a CI environment, these tests are run in a headless
browser and you will not have access to certain APIs, such as
[`Notification`](https://developer.mozilla.org/en-US/docs/Web/API/notification),
which will have to be stubbed.
### Best practice
#### Naming unit tests
When writing describe test blocks to test specific functions/methods,
please use the method name as the describe block name.
```javascript
// Good
describe('methodName', () => {
it('passes', () => {
expect(true).toEqual(true);
});
});
// Bad
describe('#methodName', () => {
it('passes', () => {
expect(true).toEqual(true);
});
});
// Bad
describe('.methodName', () => {
it('passes', () => {
expect(true).toEqual(true);
});
});
```
#### Testing Promises
When testing Promises you should always make sure that the test is asynchronous and rejections are handled.
Your Promise chain should therefore end with a call of the `done` callback and `done.fail` in case an error occurred.
```javascript
// Good
it('tests a promise', (done) => {
promise
.then((data) => {
expect(data).toBe(asExpected);
})
.then(done)
.catch(done.fail);
});
// Good
it('tests a promise rejection', (done) => {
promise
.then(done.fail)
.catch((error) => {
expect(error).toBe(expectedError);
})
.then(done)
.catch(done.fail);
});
// Bad (missing done callback)
it('tests a promise', () => {
promise
.then((data) => {
expect(data).toBe(asExpected);
})
});
// Bad (missing catch)
it('tests a promise', (done) => {
promise
.then((data) => {
expect(data).toBe(asExpected);
})
.then(done)
});
// Bad (use done.fail in asynchronous tests)
it('tests a promise', (done) => {
promise
.then((data) => {
expect(data).toBe(asExpected);
})
.then(done)
.catch(fail)
});
// Bad (missing catch)
it('tests a promise rejection', (done) => {
promise
.catch((error) => {
expect(error).toBe(expectedError);
})
.then(done)
});
```
#### Stubbing
For unit tests, you should stub methods that are unrelated to the current unit you are testing.
If you need to use a prototype method, instantiate an instance of the class and call it there instead of mocking the instance completely.
For integration tests, you should stub methods that will effect the stability of the test if they
execute their original behaviour. i.e. Network requests.
### Vue.js unit tests
See this [section][vue-test].
### Running frontend tests
`rake karma` runs the frontend-only (JavaScript) tests.
It consists of two subtasks:
- `rake karma:fixtures` (re-)generates fixtures
- `rake karma:tests` actually executes the tests
As long as the fixtures don't change, `rake karma:tests` (or `yarn karma`)
is sufficient (and saves you some time).
### Live testing and focused testing
While developing locally, it may be helpful to keep karma running so that you
can get instant feedback on as you write tests and modify code. To do this
you can start karma with `npm run karma-start`. It will compile the javascript
assets and run a server at `http://localhost:9876/` where it will automatically
run the tests on any browser which connects to it. You can enter that url on
multiple browsers at once to have it run the tests on each in parallel.
While karma is running, any changes you make will instantly trigger a recompile
and retest of the entire test suite, so you can see instantly if you've broken
a test with your changes. You can use [jasmine focused][jasmine-focus] or
excluded tests (with `fdescribe` or `xdescribe`) to get karma to run only the
tests you want while you're working on a specific feature, but make sure to
remove these directives when you commit your code.
## RSpec Feature Integration Tests
Information on setting up and running RSpec integration tests with
[Capybara][capybara] can be found in the
[general testing guide](../testing.md).
## Gotchas
### Errors due to use of unsupported JavaScript features
Similar errors will be thrown if you're using JavaScript features not yet
supported by the PhantomJS test runner which is used for both Karma and RSpec
tests. We polyfill some JavaScript objects for older browsers, but some
features are still unavailable:
- Array.from
- Array.first
- Async functions
- Generators
- Array destructuring
- For..Of
- Symbol/Symbol.iterator
- Spread
Until these are polyfilled appropriately, they should not be used. Please
update this list with additional unsupported features.
### RSpec errors due to JavaScript
By default RSpec unit tests will not run JavaScript in the headless browser
and will simply rely on inspecting the HTML generated by rails.
If an integration test depends on JavaScript to run correctly, you need to make
sure the spec is configured to enable JavaScript when the tests are run. If you
don't do this you'll see vague error messages from the spec runner.
To enable a JavaScript driver in an `rspec` test, add `:js` to the
individual spec or the context block containing multiple specs that need
JavaScript enabled:
```ruby
# For one spec
it 'presents information about abuse report', :js do
# assertions...
end
describe "Admin::AbuseReports", :js do
it 'presents information about abuse report' do
# assertions...
end
it 'shows buttons for adding to abuse report' do
# assertions...
end
end
```
### Spinach errors due to missing JavaScript
> **Note:** Since we are discouraging the use of Spinach when writing new
> feature tests, you shouldn't ever need to use this. This information is kept
> available for legacy purposes only.
In Spinach, the JavaScript driver is enabled differently. In the `*.feature`
file for the failing spec, add the `@javascript` flag above the Scenario:
```
@javascript
Scenario: Developer can approve merge request
Given I am a "Shop" developer
And I visit project "Shop" merge requests page
And merge request 'Bug NS-04' must be approved
And I click link "Bug NS-04"
When I click link "Approve"
Then I should see approved merge request "Bug NS-04"
```
[capybara]: http://teamcapybara.github.io/capybara/
[jasmine]: https://jasmine.github.io/
[jasmine-focus]: https://jasmine.github.io/2.5/focused_specs.html
[jasmine-jquery]: https://github.com/velesin/jasmine-jquery
[karma]: http://karma-runner.github.io/
[vue-test]:https://docs.gitlab.com/ce/development/fe_guide/vue.html#testing-vue-components
This document was moved to [../testing_guide/frontend_testing.md](../testing_guide/frontend_testing.md).

View file

@ -1,566 +1 @@
# Testing Standards and Style Guidelines
This guide outlines standards and best practices for automated testing of GitLab
CE and EE.
It is meant to be an _extension_ of the [thoughtbot testing
styleguide](https://github.com/thoughtbot/guides/tree/master/style/testing). If
this guide defines a rule that contradicts the thoughtbot guide, this guide
takes precedence. Some guidelines may be repeated verbatim to stress their
importance.
## Definitions
### Unit tests
Formal definition: https://en.wikipedia.org/wiki/Unit_testing
These kind of tests ensure that a single unit of code (a method) works as
expected (given an input, it has a predictable output). These tests should be
isolated as much as possible. For example, model methods that don't do anything
with the database shouldn't need a DB record. Classes that don't need database
records should use stubs/doubles as much as possible.
| Code path | Tests path | Testing engine | Notes |
| --------- | ---------- | -------------- | ----- |
| `app/finders/` | `spec/finders/` | RSpec | |
| `app/helpers/` | `spec/helpers/` | RSpec | |
| `app/db/{post_,}migrate/` | `spec/migrations/` | RSpec | More details at [`spec/migrations/README.md`](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/spec/migrations/README.md). |
| `app/policies/` | `spec/policies/` | RSpec | |
| `app/presenters/` | `spec/presenters/` | RSpec | |
| `app/routing/` | `spec/routing/` | RSpec | |
| `app/serializers/` | `spec/serializers/` | RSpec | |
| `app/services/` | `spec/services/` | RSpec | |
| `app/tasks/` | `spec/tasks/` | RSpec | |
| `app/uploaders/` | `spec/uploaders/` | RSpec | |
| `app/views/` | `spec/views/` | RSpec | |
| `app/workers/` | `spec/workers/` | RSpec | |
| `app/assets/javascripts/` | `spec/javascripts/` | Karma | More details in the [JavaScript](#javascript) section. |
### Integration tests
Formal definition: https://en.wikipedia.org/wiki/Integration_testing
These kind of tests ensure that individual parts of the application work well together, without the overhead of the actual app environment (i.e. the browser). These tests should assert at the request/response level: status code, headers, body. They're useful to test permissions, redirections, what view is rendered etc.
| Code path | Tests path | Testing engine | Notes |
| --------- | ---------- | -------------- | ----- |
| `app/controllers/` | `spec/controllers/` | RSpec | |
| `app/mailers/` | `spec/mailers/` | RSpec | |
| `lib/api/` | `spec/requests/api/` | RSpec | |
| `lib/ci/api/` | `spec/requests/ci/api/` | RSpec | |
| `app/assets/javascripts/` | `spec/javascripts/` | Karma | More details in the [JavaScript](#javascript) section. |
#### About controller tests
In an ideal world, controllers should be thin. However, when this is not the
case, it's acceptable to write a system/feature test without JavaScript instead
of a controller test. The reason is that testing a fat controller usually
involves a lot of stubbing, things like:
```ruby
controller.instance_variable_set(:@user, user)
```
and use methods which are deprecated in Rails 5 ([#23768]).
[#23768]: https://gitlab.com/gitlab-org/gitlab-ce/issues/23768
#### About Karma
As you may have noticed, Karma is both in the Unit tests and the Integration
tests category. That's because Karma is a tool that provides an environment to
run JavaScript tests, so you can either run unit tests (e.g. test a single
JavaScript method), or integration tests (e.g. test a component that is composed
of multiple components).
### System tests or Feature tests
Formal definition: https://en.wikipedia.org/wiki/System_testing.
These kind of tests ensure the application works as expected from a user point
of view (aka black-box testing). These tests should test a happy path for a
given page or set of pages, and a test case should be added for any regression
that couldn't have been caught at lower levels with better tests (i.e. if a
regression is found, regression tests should be added at the lowest-level
possible).
| Tests path | Testing engine | Notes |
| ---------- | -------------- | ----- |
| `spec/features/` | [Capybara] + [RSpec] | If your spec has the `:js` metadata, the browser driver will be [Poltergeist], otherwise it's using [RackTest]. |
| `features/` | Spinach | Spinach tests are deprecated, [you shouldn't add new Spinach tests](#spinach-feature-tests). |
[Capybara]: https://github.com/teamcapybara/capybara
[RSpec]: https://github.com/rspec/rspec-rails#feature-specs
[Poltergeist]: https://github.com/teamcapybara/capybara#poltergeist
[RackTest]: https://github.com/teamcapybara/capybara#racktest
#### Best practices
- 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
If we're confident that the low-level components work well (and we should be if
we have enough Unit & Integration tests), we shouldn't need to duplicate their
thorough testing at the System test level.
It's very easy to add tests, but a lot harder to remove or improve tests, so one
should take care of not introducing too many (slow and duplicated) specs.
The reasons why we should follow these best practices are as follows:
- System tests are slow to run since they spin up the entire application stack
in a headless browser, and even slower when they integrate a JS driver
- When system tests run with a JavaScript driver, the tests are run in a
different thread than the application. This means it does not share a
database connection and your test will have to commit the transactions in
order for the running application to see the data (and vice-versa). In that
case we need to truncate the database after each spec instead of simply
rolling back a transaction (the faster strategy that's in use for other kind
of tests). This is slower than transactions, however, so we want to use
truncation only when necessary.
### Black-box tests or End-to-end tests
GitLab consists of [multiple pieces] such as [GitLab Shell], [GitLab Workhorse],
[Gitaly], [GitLab Pages], [GitLab Runner], and GitLab Rails. All theses pieces
are configured and packaged by [GitLab Omnibus].
[GitLab QA] is a tool that allows to test that all these pieces integrate well
together by building a Docker image for a given version of GitLab Rails and
running feature tests (i.e. using Capybara) against it.
The actual test scenarios and steps are [part of GitLab Rails] so that they're
always in-sync with the codebase.
[multiple pieces]: ./architecture.md#components
[GitLab Shell]: https://gitlab.com/gitlab-org/gitlab-shell
[GitLab Workhorse]: https://gitlab.com/gitlab-org/gitlab-workhorse
[Gitaly]: https://gitlab.com/gitlab-org/gitaly
[GitLab Pages]: https://gitlab.com/gitlab-org/gitlab-pages
[GitLab Runner]: https://gitlab.com/gitlab-org/gitlab-ci-multi-runner
[GitLab Omnibus]: https://gitlab.com/gitlab-org/omnibus-gitlab
[GitLab QA]: https://gitlab.com/gitlab-org/gitlab-qa
[part of GitLab Rails]: https://gitlab.com/gitlab-org/gitlab-ce/tree/master/qa
## Test for what should not be there
This is particularly important for permission calls and might be called a
negative assertion: make sure only the bare minimum is returned and nothing else.
See an issue about [leaking tokens] as an example of a vulnerability that is
captured by such a test.
[leaking tokens]: https://gitlab.com/gitlab-org/gitlab-ce/issues/37948
## How to test at the correct level?
As many things in life, deciding what to test at each level of testing is a
trade-off:
- Unit tests are usually cheap, and you should consider them like the basement
of your house: you need them to be confident that your code is behaving
correctly. However if you run only unit tests without integration / system
tests, you might [miss] the [big] [picture]!
- Integration tests are a bit more expensive, but don't abuse them. A system test
is often better than an integration test that is stubbing a lot of internals.
- System tests are expensive (compared to unit tests), even more if they require
a JavaScript driver. Make sure to follow the guidelines in the [Speed](#test-speed)
section.
Another way to see it is to think about the "cost of tests", this is well
explained [in this article][tests-cost] and the basic idea is that the cost of a
test includes:
- The time it takes to write the test
- The time it takes to run the test every time the suite runs
- The time it takes to understand the test
- The time it takes to fix the test if it breaks and the underlying code is OK
- Maybe, the time it takes to change the code to make the code testable.
[miss]: https://twitter.com/ThePracticalDev/status/850748070698651649
[big]: https://twitter.com/timbray/status/822470746773409794
[picture]: https://twitter.com/withzombies/status/829716565834752000
[tests-cost]: https://medium.com/table-xi/high-cost-tests-and-high-value-tests-a86e27a54df#.2ulyh3a4e
## Frontend testing
Please consult the [dedicated "Frontend testing" guide](./fe_guide/testing.md).
## RSpec
### 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.
- 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)).
- 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`
[four-phase-test]: https://robots.thoughtbot.com/four-phase-test
### Automatic retries and flaky tests detection
On our CI, we use [rspec-retry] to automatically retry a failing example a few
times (see [`spec/spec_helper.rb`] for the precise retries count).
We also use a home-made `RspecFlaky::Listener` listener which records flaky
examples in a JSON report file on `master` (`retrieve-tests-metadata` and `update-tests-metadata` jobs), and warns when a new flaky example
is detected in any other branch (`flaky-examples-check` job). In the future, the
`flaky-examples-check` job will not be allowed to fail.
[rspec-retry]: https://github.com/NoRedInk/rspec-retry
[`spec/spec_helper.rb`]: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/spec/spec_helper.rb
### `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
```
### System / Feature tests
- Feature specs should be named `ROLE_ACTION_spec.rb`, such as
`user_changes_password_spec.rb`.
- Use only one `feature` block per feature spec file.
- 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.
### 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
```
### 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 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 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 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_girl] 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.
- FactoryGirl methods are mixed in to all RSpec groups. This means you can (and
should) call `create(...)` instead of `FactoryGirl.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_girl]: https://github.com/thoughtbot/factory_girl
[traits]: http://www.rubydoc.info/gems/factory_girl/file/GETTING_STARTED.md#Traits
### Fixtures
All fixtures should be be placed under `spec/fixtures/`.
### 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/config/`.
Each file should be related to a specific domain, e.g.
`spec/support/config/capybara.rb`, `spec/support/config/carrierwave.rb`, etc.
Helpers can be included in the `spec/support/config/rspec.rb` file. 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
```
## Testing Rake Tasks
To make testing Rake tasks a little easier, there is a helper that can be included
in lieu of the standard Spec helper. Instead of `require 'spec_helper'`, use
`require 'rake_helper'`. The helper includes `spec_helper` for you, and configures
a few other things to make testing Rake tasks easier.
At a minimum, requiring the Rake helper will redirect `stdout`, include the
runtime task helpers, and include the `RakeHelpers` Spec support module.
The `RakeHelpers` module exposes a `run_rake_task(<task>)` method to make
executing tasks simple. See `spec/support/rake_helpers.rb` for all available
methods.
Example:
```ruby
require 'rake_helper'
describe 'gitlab:shell rake tasks' do
before do
Rake.application.rake_require 'tasks/gitlab/shell'
stub_warn_user_is_not_gitlab
end
describe 'install task' do
it 'invokes create_hooks task' do
expect(Rake::Task['gitlab:shell:create_hooks']).to receive(:invoke)
run_rake_task('gitlab:shell:install')
end
end
end
```
## 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 `FactoryGirl.build(...)`
- `FactoryGirl.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 `@javascript` in
Spinach or `:js` in RSpec) unless it's _actually_ required for the test
to be valid. Headless browser testing is slow!
[parallelization]: #test-suite-parallelization-on-the-ci
### Test suite parallelization on the CI
Our current CI parallelization setup is as follows:
1. The `retrieve-tests-metadata` job in the `prepare` stage ensures that we have
a `knapsack/${CI_PROJECT_NAME}/rspec_report-master.json` file:
- The `knapsack/${CI_PROJECT_NAME}/rspec_report-master.json` file is fetched
from S3, if it's not here we initialize the file with `{}`.
1. Each `rspec-pg x y`/`rspec-mysql x y` job is run with `knapsack rspec` and
should have an evenly distributed share of tests:
- It works because the jobs have access to the
`knapsack/${CI_PROJECT_NAME}/rspec_report-master.json` since the "artifacts
from all previous stages are passed by default". [^1]
- The jobs set their own report path to
`KNAPSACK_REPORT_PATH=knapsack/${CI_PROJECT_NAME}/${JOB_NAME[0]}_node_${CI_NODE_INDEX}_${CI_NODE_TOTAL}_report.json`.
- If knapsack is doing its job, test files that are run should be listed under
`Report specs`, not under `Leftover specs`.
1. The `update-tests-metadata` job takes all the
`knapsack/${CI_PROJECT_NAME}/${JOB_NAME[0]}_node_${CI_NODE_INDEX}_${CI_NODE_TOTAL}_report.json`
files from the `rspec-pg x y`/`rspec-mysql x y`jobs and merge them all together
into a single `knapsack/${CI_PROJECT_NAME}/rspec_report-master.json` file that
is then uploaded to S3.
After that, the next pipeline will use the up-to-date
`knapsack/${CI_PROJECT_NAME}/rspec_report-master.json` file. The same strategy
is used for Spinach tests as well.
### Monitoring
The GitLab test suite is [monitored] for the `master` branch, and any branch
that includes `rspec-profile` in their name.
A [public dashboard] is available for everyone to see. Feel free to look at the
slowest test files and try to improve them.
[monitored]: ./performance.md#rspec-profiling
[public dashboard]: https://redash.gitlab.com/public/dashboards/l1WhHXaxrCWM5Ai9D7YDqHKehq6OU3bx5gssaiWe?org_slug=default
## CI setup
- On CE and EE, the test suite runs both PostgreSQL and MySQL.
- Rails logging to `log/test.log` is disabled by default in CI [for
performance reasons][logging]. To override this setting, provide the
`RAILS_ENABLE_TEST_LOG` environment variable.
[logging]: https://jtway.co/speed-up-your-rails-test-suite-by-6-in-1-line-13fedb869ec4
## Spinach (feature) tests
GitLab [moved from Cucumber to Spinach](https://github.com/gitlabhq/gitlabhq/pull/1426)
for its feature/integration tests in September 2012.
As of March 2016, we are [trying to avoid adding new Spinach
tests](https://gitlab.com/gitlab-org/gitlab-ce/issues/14121) going forward,
opting for [RSpec feature](#features-integration) specs.
Adding new Spinach scenarios is acceptable _only if_ the new scenario requires
no more than one new `step` definition. If more than that is required, the
test should be re-implemented using RSpec instead.
---
[Return to Development documentation](README.md)
[^1]: /ci/yaml/README.html#dependencies
This document was moved to [testing_guide/index.md](testing_guide/index.md).

View file

@ -0,0 +1,272 @@
# 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 `FactoryGirl.build(...)`
- `FactoryGirl.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 `@javascript` in
Spinach or `: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
### 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.
- 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)).
- 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`
[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
### `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
```
### 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
```
### 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 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 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 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_girl] 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.
- FactoryGirl methods are mixed in to all RSpec groups. This means you can (and
should) call `create(...)` instead of `FactoryGirl.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_girl]: https://github.com/thoughtbot/factory_girl
[traits]: http://www.rubydoc.info/gems/factory_girl/file/GETTING_STARTED.md#Traits
### Fixtures
All fixtures should be be placed under `spec/fixtures/`.
### 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/config/`.
Each file should be related to a specific domain, e.g.
`spec/support/config/capybara.rb`, `spec/support/config/carrierwave.rb`, etc.
Helpers can be included in the `spec/support/config/rspec.rb` file. 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
```
---
[Return to Testing documentation](index.md)

View file

@ -0,0 +1,52 @@
# GitLab tests in the Continuous Integration (CI) context
### Test suite parallelization on the CI
Our current CI parallelization setup is as follows:
1. The `knapsack` job in the prepare stage that is supposed to ensure we have a
`knapsack/${CI_PROJECT_NAME}/rspec_report-master.json` file:
- The `knapsack/${CI_PROJECT_NAME}/rspec_report-master.json` file is fetched
from S3, if it's not here we initialize the file with `{}`.
1. Each `rspec x y` job are run with `knapsack rspec` and should have an evenly
distributed share of tests:
- It works because the jobs have access to the
`knapsack/${CI_PROJECT_NAME}/rspec_report-master.json` since the "artifacts
from all previous stages are passed by default". [^1]
- the jobs set their own report path to
`KNAPSACK_REPORT_PATH=knapsack/${CI_PROJECT_NAME}/${JOB_NAME[0]}_node_${CI_NODE_INDEX}_${CI_NODE_TOTAL}_report.json`.
- if knapsack is doing its job, test files that are run should be listed under
`Report specs`, not under `Leftover specs`.
1. The `update-knapsack` job takes all the
`knapsack/${CI_PROJECT_NAME}/${JOB_NAME[0]}_node_${CI_NODE_INDEX}_${CI_NODE_TOTAL}_report.json`
files from the `rspec x y` jobs and merge them all together into a single
`knapsack/${CI_PROJECT_NAME}/rspec_report-master.json` file that is then
uploaded to S3.
After that, the next pipeline will use the up-to-date
`knapsack/${CI_PROJECT_NAME}/rspec_report-master.json` file. The same strategy
is used for Spinach tests as well.
### Monitoring
The GitLab test suite is [monitored] for the `master` branch, and any branch
that includes `rspec-profile` in their name.
A [public dashboard] is available for everyone to see. Feel free to look at the
slowest test files and try to improve them.
[monitored]: ../performance.md#rspec-profiling
[public dashboard]: https://redash.gitlab.com/public/dashboards/l1WhHXaxrCWM5Ai9D7YDqHKehq6OU3bx5gssaiWe?org_slug=default
## CI setup
- On CE and EE, the test suite runs both PostgreSQL and MySQL.
- Rails logging to `log/test.log` is disabled by default in CI [for
performance reasons][logging]. To override this setting, provide the
`RAILS_ENABLE_TEST_LOG` environment variable.
[logging]: https://jtway.co/speed-up-your-rails-test-suite-by-6-in-1-line-13fedb869ec4
---
[Return to Testing documentation](index.md)

View file

@ -0,0 +1,74 @@
# Flaky tests
## What's a flaky test?
It's a test that sometimes fails, but if you retry it enough times, it passes,
eventually.
## Automatic retries and flaky tests detection
On our CI, we use [rspec-retry] to automatically retry a failing example a few
times (see [`spec/spec_helper.rb`] for the precise retries count).
We also use a home-made `RspecFlaky::Listener` listener which records flaky
examples in a JSON report file on `master` (`retrieve-tests-metadata` and `update-tests-metadata` jobs), and warns when a new flaky example
is detected in any other branch (`flaky-examples-check` job). In the future, the
`flaky-examples-check` job will not be allowed to fail.
This was originally implemented in: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/13021.
[rspec-retry]: https://github.com/NoRedInk/rspec-retry
[`spec/spec_helper.rb`]: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/spec/spec_helper.rb
## Problems we had in the past at GitLab
- [`rspec-retry` is bitting us when some API specs fail](https://gitlab.com/gitlab-org/gitlab-ce/issues/29242): https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9825
- [Sporadic RSpec failures due to `PG::UniqueViolation`](https://gitlab.com/gitlab-org/gitlab-ce/issues/28307#note_24958837): https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9846
- Follow-up: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10688
- [Capybara.reset_session! should be called before requests are blocked](https://gitlab.com/gitlab-org/gitlab-ce/issues/33779): https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/12224
- FFaker generates funky data that tests are not ready to handle (and tests should be predictable so that's bad!):
- [Make `spec/mailers/notify_spec.rb` more robust](https://gitlab.com/gitlab-org/gitlab-ce/issues/20121): https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10015
- [Transient failure in spec/requests/api/commits_spec.rb](https://gitlab.com/gitlab-org/gitlab-ce/issues/27988#note_25342521): https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9944
- [Replace FFaker factory data with sequences](https://gitlab.com/gitlab-org/gitlab-ce/issues/29643): https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10184
- [Transient failure in spec/finders/issues_finder_spec.rb](https://gitlab.com/gitlab-org/gitlab-ce/issues/30211#note_26707685): https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10404
### Time-sensitive flaky tests
- https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10046
- https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10306
### Array order expectation
- https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10148
### Feature tests
- [Be sure to create all the data the test need before starting exercize](https://gitlab.com/gitlab-org/gitlab-ce/issues/32622#note_31128195): https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/12059
- [Bis](https://gitlab.com/gitlab-org/gitlab-ce/issues/34609#note_34048715): https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/12604
- [Bis](https://gitlab.com/gitlab-org/gitlab-ce/issues/34698#note_34276286): https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/12664
- [Assert against the underlying database state instead of against a page's content](https://gitlab.com/gitlab-org/gitlab-ce/issues/31437): https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10934
#### Capybara viewport size related issues
- [Transient failure of spec/features/issues/filtered_search/filter_issues_spec.rb](https://gitlab.com/gitlab-org/gitlab-ce/issues/29241#note_26743936): https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10411
#### Capybara JS driver related issues
- [Don't wait for AJAX when no AJAX request is fired](https://gitlab.com/gitlab-org/gitlab-ce/issues/30461): https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10454
- [Bis](https://gitlab.com/gitlab-org/gitlab-ce/issues/34647): https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/12626
#### PhantomJS / WebKit related issues
- Memory is through the roof! (TL;DR: Load images but block images requests!): https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/12003
## Resources
- [Flaky Tests: Are You Sure You Want to Rerun Them?](http://semaphoreci.com/blog/2017/04/20/flaky-tests.html)
- [How to Deal With and Eliminate Flaky Tests](https://semaphoreci.com/community/tutorials/how-to-deal-with-and-eliminate-flaky-tests)
- [Tips on Treating Flakiness in your Rails Test Suite](http://semaphoreci.com/blog/2017/08/03/tips-on-treating-flakiness-in-your-test-suite.html)
- ['Flaky' tests: a short story](https://www.ombulabs.com/blog/rspec/continuous-integration/how-to-track-down-a-flaky-test.html)
- [Using Insights to Discover Flaky, Slow, and Failed Tests](https://circleci.com/blog/using-insights-to-discover-flaky-slow-and-failed-tests/)
---
[Return to Testing documentation](index.md)

View file

@ -0,0 +1,254 @@
# Frontend testing standards and style guidelines
There are two types of test suites you'll encounter while developing frontend code
at GitLab. We use Karma and Jasmine for JavaScript unit and integration testing,
and RSpec feature tests with Capybara for e2e (end-to-end) integration testing.
Unit and feature tests need to be written for all new features.
Most of the time, you should use [RSpec] for your feature tests.
Regression tests should be written for bug fixes to prevent them from recurring
in the future.
See the [Testing Standards and Style Guidelines](index.md) page for more
information on general testing practices at GitLab.
## Karma test suite
GitLab uses the [Karma][karma] test runner with [Jasmine] as its test
framework for our JavaScript unit and integration tests. For integration tests,
we generate HTML files using RSpec (see `spec/javascripts/fixtures/*.rb` for examples).
Some fixtures are still HAML templates that are translated to HTML files using the same mechanism (see `static_fixtures.rb`).
Adding these static fixtures should be avoided as they are harder to keep up to date with real views.
The existing static fixtures will be migrated over time.
Please see [gitlab-org/gitlab-ce#24753](https://gitlab.com/gitlab-org/gitlab-ce/issues/24753) to track our progress.
Fixtures are served during testing by the [jasmine-jquery][jasmine-jquery] plugin.
JavaScript tests live in `spec/javascripts/`, matching the folder structure
of `app/assets/javascripts/`: `app/assets/javascripts/behaviors/autosize.js`
has a corresponding `spec/javascripts/behaviors/autosize_spec.js` file.
Keep in mind that in a CI environment, these tests are run in a headless
browser and you will not have access to certain APIs, such as
[`Notification`](https://developer.mozilla.org/en-US/docs/Web/API/notification),
which will have to be stubbed.
### Best practices
#### Naming unit tests
When writing describe test blocks to test specific functions/methods,
please use the method name as the describe block name.
```javascript
// Good
describe('methodName', () => {
it('passes', () => {
expect(true).toEqual(true);
});
});
// Bad
describe('#methodName', () => {
it('passes', () => {
expect(true).toEqual(true);
});
});
// Bad
describe('.methodName', () => {
it('passes', () => {
expect(true).toEqual(true);
});
});
```
#### Testing promises
When testing Promises you should always make sure that the test is asynchronous and rejections are handled.
Your Promise chain should therefore end with a call of the `done` callback and `done.fail` in case an error occurred.
```javascript
// Good
it('tests a promise', (done) => {
promise
.then((data) => {
expect(data).toBe(asExpected);
})
.then(done)
.catch(done.fail);
});
// Good
it('tests a promise rejection', (done) => {
promise
.then(done.fail)
.catch((error) => {
expect(error).toBe(expectedError);
})
.then(done)
.catch(done.fail);
});
// Bad (missing done callback)
it('tests a promise', () => {
promise
.then((data) => {
expect(data).toBe(asExpected);
})
});
// Bad (missing catch)
it('tests a promise', (done) => {
promise
.then((data) => {
expect(data).toBe(asExpected);
})
.then(done)
});
// Bad (use done.fail in asynchronous tests)
it('tests a promise', (done) => {
promise
.then((data) => {
expect(data).toBe(asExpected);
})
.then(done)
.catch(fail)
});
// Bad (missing catch)
it('tests a promise rejection', (done) => {
promise
.catch((error) => {
expect(error).toBe(expectedError);
})
.then(done)
});
```
#### Stubbing
For unit tests, you should stub methods that are unrelated to the current unit you are testing.
If you need to use a prototype method, instantiate an instance of the class and call it there instead of mocking the instance completely.
For integration tests, you should stub methods that will effect the stability of the test if they
execute their original behaviour. i.e. Network requests.
### Vue.js unit tests
See this [section][vue-test].
### Running frontend tests
`rake karma` runs the frontend-only (JavaScript) tests.
It consists of two subtasks:
- `rake karma:fixtures` (re-)generates fixtures
- `rake karma:tests` actually executes the tests
As long as the fixtures don't change, `rake karma:tests` (or `yarn karma`)
is sufficient (and saves you some time).
### Live testing and focused testing
While developing locally, it may be helpful to keep karma running so that you
can get instant feedback on as you write tests and modify code. To do this
you can start karma with `npm run karma-start`. It will compile the javascript
assets and run a server at `http://localhost:9876/` where it will automatically
run the tests on any browser which connects to it. You can enter that url on
multiple browsers at once to have it run the tests on each in parallel.
While karma is running, any changes you make will instantly trigger a recompile
and retest of the entire test suite, so you can see instantly if you've broken
a test with your changes. You can use [jasmine focused][jasmine-focus] or
excluded tests (with `fdescribe` or `xdescribe`) to get karma to run only the
tests you want while you're working on a specific feature, but make sure to
remove these directives when you commit your code.
## RSpec feature integration tests
Information on setting up and running RSpec integration tests with
[Capybara] can be found in the [Testing Best Practices](best_practices.md).
## Gotchas
### Errors due to use of unsupported JavaScript features
Similar errors will be thrown if you're using JavaScript features not yet
supported by the PhantomJS test runner which is used for both Karma and RSpec
tests. We polyfill some JavaScript objects for older browsers, but some
features are still unavailable:
- Array.from
- Array.first
- Async functions
- Generators
- Array destructuring
- For..Of
- Symbol/Symbol.iterator
- Spread
Until these are polyfilled appropriately, they should not be used. Please
update this list with additional unsupported features.
### RSpec errors due to JavaScript
By default RSpec unit tests will not run JavaScript in the headless browser
and will simply rely on inspecting the HTML generated by rails.
If an integration test depends on JavaScript to run correctly, you need to make
sure the spec is configured to enable JavaScript when the tests are run. If you
don't do this you'll see vague error messages from the spec runner.
To enable a JavaScript driver in an `rspec` test, add `:js` to the
individual spec or the context block containing multiple specs that need
JavaScript enabled:
```ruby
# For one spec
it 'presents information about abuse report', :js do
# assertions...
end
describe "Admin::AbuseReports", :js do
it 'presents information about abuse report' do
# assertions...
end
it 'shows buttons for adding to abuse report' do
# assertions...
end
end
```
### Spinach errors due to missing JavaScript
NOTE: **Note:** Since we are discouraging the use of Spinach when writing new
feature tests, you shouldn't ever need to use this. This information is kept
available for legacy purposes only.
In Spinach, the JavaScript driver is enabled differently. In the `*.feature`
file for the failing spec, add the `@javascript` flag above the Scenario:
```
@javascript
Scenario: Developer can approve merge request
Given I am a "Shop" developer
And I visit project "Shop" merge requests page
And merge request 'Bug NS-04' must be approved
And I click link "Bug NS-04"
When I click link "Approve"
Then I should see approved merge request "Bug NS-04"
```
[jasmine-focus]: https://jasmine.github.io/2.5/focused_specs.html
[jasmine-jquery]: https://github.com/velesin/jasmine-jquery
[karma]: http://karma-runner.github.io/
[vue-test]:https://docs.gitlab.com/ce/development/fe_guide/vue.html#testing-vue-components
[RSpec]: https://github.com/rspec/rspec-rails#feature-specs
[Capybara]: https://github.com/teamcapybara/capybara
[Karma]: http://karma-runner.github.io/
[Jasmine]: https://jasmine.github.io/
---
[Return to Testing documentation](index.md)

View file

Before

Width:  |  Height:  |  Size: 12 KiB

After

Width:  |  Height:  |  Size: 12 KiB

View file

@ -0,0 +1,90 @@
# Testing standards and style guidelines
This document describes various guidelines and best practices for automated
testing of the GitLab project.
It is meant to be an _extension_ of the [thoughtbot testing
styleguide](https://github.com/thoughtbot/guides/tree/master/style/testing). If
this guide defines a rule that contradicts the thoughtbot guide, this guide
takes precedence. Some guidelines may be repeated verbatim to stress their
importance.
## Overview
GitLab is built on top of [Ruby on Rails][rails], and we're using [RSpec] for all
the backend tests, with [Capybara] for end-to-end integration testing.
On the frontend side, we're using [Karma] and [Jasmine] for JavaScript unit and
integration testing.
Following are two great articles that everyone should read to understand what
automated testing means, and what are its principles:
- [Five Factor Testing](https://www.devmynd.com/blog/five-factor-testing): Why do we need tests?
- [Principles of Automated Testing](http://www.lihaoyi.com/post/PrinciplesofAutomatedTesting.html): Levels of testing. Prioritize tests. Cost of tests.
---
## [Testing levels](testing_levels.md)
Learn about the different testing levels, and how to decide at what level your
changes should be tested.
---
## [Testing best practices](best_practices.md)
Everything you should know about how to write good tests: RSpec, FactoryGirl,
system tests, parameterized tests etc.
---
## [Frontend testing standards and style guidelines](frontend_testing.md)
Everything you should know about how to write good Frontend tests: Karma,
testing promises, stubbing etc.
---
## [Flaky tests](flaky_tests.md)
What are flaky tests, the different kind of flaky tests we encountered, and what
we do about them.
---
## [GitLab tests in the Continuous Integration (CI) context](ci.md)
How GitLab test suite is run in the CI context: setup, caches, artifacts,
parallelization, monitoring.
---
## [Testing Rake tasks](testing_rake_tasks.md)
Everything you should know about how to test Rake tasks.
---
## Spinach (feature) tests
GitLab [moved from Cucumber to Spinach](https://github.com/gitlabhq/gitlabhq/pull/1426)
for its feature/integration tests in September 2012.
As of March 2016, we are [trying to avoid adding new Spinach
tests](https://gitlab.com/gitlab-org/gitlab-ce/issues/14121) going forward,
opting for [RSpec feature](#features-integration) specs.
Adding new Spinach scenarios is acceptable _only if_ the new scenario requires
no more than one new `step` definition. If more than that is required, the
test should be re-implemented using RSpec instead.
---
[Return to Development documentation](../README.md)
[^1]: /ci/yaml/README.html#dependencies
[RSpec]: https://github.com/rspec/rspec-rails#feature-specs
[Capybara]: https://github.com/teamcapybara/capybara
[Karma]: http://karma-runner.github.io/
[Jasmine]: https://jasmine.github.io/

View file

@ -0,0 +1,173 @@
# Testing levels
![Testing priority triangle](img/testing_triangle.png)
_This diagram demonstrates the relative priority of each test type we use. `e2e` stands for end-to-end._
## Unit tests
Formal definition: https://en.wikipedia.org/wiki/Unit_testing
These kind of tests ensure that a single unit of code (a method) works as
expected (given an input, it has a predictable output). These tests should be
isolated as much as possible. For example, model methods that don't do anything
with the database shouldn't need a DB record. Classes that don't need database
records should use stubs/doubles as much as possible.
| Code path | Tests path | Testing engine | Notes |
| --------- | ---------- | -------------- | ----- |
| `app/finders/` | `spec/finders/` | RSpec | |
| `app/helpers/` | `spec/helpers/` | RSpec | |
| `app/db/{post_,}migrate/` | `spec/migrations/` | RSpec | More details at [`spec/migrations/README.md`](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/spec/migrations/README.md). |
| `app/policies/` | `spec/policies/` | RSpec | |
| `app/presenters/` | `spec/presenters/` | RSpec | |
| `app/routing/` | `spec/routing/` | RSpec | |
| `app/serializers/` | `spec/serializers/` | RSpec | |
| `app/services/` | `spec/services/` | RSpec | |
| `app/tasks/` | `spec/tasks/` | RSpec | |
| `app/uploaders/` | `spec/uploaders/` | RSpec | |
| `app/views/` | `spec/views/` | RSpec | |
| `app/workers/` | `spec/workers/` | RSpec | |
| `app/assets/javascripts/` | `spec/javascripts/` | Karma | More details in the [Frontent Testing guide](frontend_testing.md) section. |
## Integration tests
Formal definition: https://en.wikipedia.org/wiki/Integration_testing
These kind of tests ensure that individual parts of the application work well together, without the overhead of the actual app environment (i.e. the browser). These tests should assert at the request/response level: status code, headers, body. They're useful to test permissions, redirections, what view is rendered etc.
| Code path | Tests path | Testing engine | Notes |
| --------- | ---------- | -------------- | ----- |
| `app/controllers/` | `spec/controllers/` | RSpec | |
| `app/mailers/` | `spec/mailers/` | RSpec | |
| `lib/api/` | `spec/requests/api/` | RSpec | |
| `lib/ci/api/` | `spec/requests/ci/api/` | RSpec | |
| `app/assets/javascripts/` | `spec/javascripts/` | Karma | More details in the [JavaScript](#javascript) section. |
### About controller tests
In an ideal world, controllers should be thin. However, when this is not the
case, it's acceptable to write a system/feature test without JavaScript instead
of a controller test. The reason is that testing a fat controller usually
involves a lot of stubbing, things like:
```ruby
controller.instance_variable_set(:@user, user)
```
and use methods which are deprecated in Rails 5 ([#23768]).
[#23768]: https://gitlab.com/gitlab-org/gitlab-ce/issues/23768
### About Karma
As you may have noticed, Karma is both in the Unit tests and the Integration
tests category. That's because Karma is a tool that provides an environment to
run JavaScript tests, so you can either run unit tests (e.g. test a single
JavaScript method), or integration tests (e.g. test a component that is composed
of multiple components).
## System tests or feature tests
Formal definition: https://en.wikipedia.org/wiki/System_testing.
These kind of tests ensure the application works as expected from a user point
of view (aka black-box testing). These tests should test a happy path for a
given page or set of pages, and a test case should be added for any regression
that couldn't have been caught at lower levels with better tests (i.e. if a
regression is found, regression tests should be added at the lowest-level
possible).
| Tests path | Testing engine | Notes |
| ---------- | -------------- | ----- |
| `spec/features/` | [Capybara] + [RSpec] | If your spec has the `:js` metadata, the browser driver will be [Poltergeist], otherwise it's using [RackTest]. |
| `features/` | Spinach | Spinach tests are deprecated, [you shouldn't add new Spinach tests](#spinach-feature-tests). |
### Consider **not** writing a system test!
If we're confident that the low-level components work well (and we should be if
we have enough Unit & Integration tests), we shouldn't need to duplicate their
thorough testing at the System test level.
It's very easy to add tests, but a lot harder to remove or improve tests, so one
should take care of not introducing too many (slow and duplicated) specs.
The reasons why we should follow these best practices are as follows:
- System tests are slow to run since they spin up the entire application stack
in a headless browser, and even slower when they integrate a JS driver
- When system tests run with a JavaScript driver, the tests are run in a
different thread than the application. This means it does not share a
database connection and your test will have to commit the transactions in
order for the running application to see the data (and vice-versa). In that
case we need to truncate the database after each spec instead of simply
rolling back a transaction (the faster strategy that's in use for other kind
of tests). This is slower than transactions, however, so we want to use
truncation only when necessary.
[Poltergeist]: https://github.com/teamcapybara/capybara#poltergeist
[RackTest]: https://github.com/teamcapybara/capybara#racktest
## Black-box tests or end-to-end tests
GitLab consists of [multiple pieces] such as [GitLab Shell], [GitLab Workhorse],
[Gitaly], [GitLab Pages], [GitLab Runner], and GitLab Rails. All theses pieces
are configured and packaged by [GitLab Omnibus].
[GitLab QA] is a tool that allows to test that all these pieces integrate well
together by building a Docker image for a given version of GitLab Rails and
running feature tests (i.e. using Capybara) against it.
The actual test scenarios and steps are [part of GitLab Rails] so that they're
always in-sync with the codebase.
[multiple pieces]: ../architecture.md#components
[GitLab Shell]: https://gitlab.com/gitlab-org/gitlab-shell
[GitLab Workhorse]: https://gitlab.com/gitlab-org/gitlab-workhorse
[Gitaly]: https://gitlab.com/gitlab-org/gitaly
[GitLab Pages]: https://gitlab.com/gitlab-org/gitlab-pages
[GitLab Runner]: https://gitlab.com/gitlab-org/gitlab-ci-multi-runner
[GitLab Omnibus]: https://gitlab.com/gitlab-org/omnibus-gitlab
[GitLab QA]: https://gitlab.com/gitlab-org/gitlab-qa
[part of GitLab Rails]: https://gitlab.com/gitlab-org/gitlab-ce/tree/master/qa
## How to test at the correct level?
As many things in life, deciding what to test at each level of testing is a
trade-off:
- Unit tests are usually cheap, and you should consider them like the basement
of your house: you need them to be confident that your code is behaving
correctly. However if you run only unit tests without integration / system
tests, you might [miss] the [big] [picture]!
- Integration tests are a bit more expensive, but don't abuse them. A system test
is often better than an integration test that is stubbing a lot of internals.
- System tests are expensive (compared to unit tests), even more if they require
a JavaScript driver. Make sure to follow the guidelines in the [Speed](#test-speed)
section.
Another way to see it is to think about the "cost of tests", this is well
explained [in this article][tests-cost] and the basic idea is that the cost of a
test includes:
- The time it takes to write the test
- The time it takes to run the test every time the suite runs
- The time it takes to understand the test
- The time it takes to fix the test if it breaks and the underlying code is OK
- Maybe, the time it takes to change the code to make the code testable.
### Frontend-related tests
There are cases where the behaviour you are testing is not worth the time spent
running the full application, for example, if you are testing styling, animation,
edge cases or small actions that don't involve the backend,
you should write an integration test using Jasmine.
[miss]: https://twitter.com/ThePracticalDev/status/850748070698651649
[big]: https://twitter.com/timbray/status/822470746773409794
[picture]: https://twitter.com/withzombies/status/829716565834752000
[tests-cost]: https://medium.com/table-xi/high-cost-tests-and-high-value-tests-a86e27a54df#.2ulyh3a4e
---
[Return to Testing documentation](index.md)

View file

@ -0,0 +1,39 @@
## Testing Rake tasks
To make testing Rake tasks a little easier, there is a helper that can be included
in lieu of the standard Spec helper. Instead of `require 'spec_helper'`, use
`require 'rake_helper'`. The helper includes `spec_helper` for you, and configures
a few other things to make testing Rake tasks easier.
At a minimum, requiring the Rake helper will redirect `stdout`, include the
runtime task helpers, and include the `RakeHelpers` Spec support module.
The `RakeHelpers` module exposes a `run_rake_task(<task>)` method to make
executing tasks simple. See `spec/support/rake_helpers.rb` for all available
methods.
Example:
```ruby
require 'rake_helper'
describe 'gitlab:shell rake tasks' do
before do
Rake.application.rake_require 'tasks/gitlab/shell'
stub_warn_user_is_not_gitlab
end
describe 'install task' do
it 'invokes create_hooks task' do
expect(Rake::Task['gitlab:shell:create_hooks']).to receive(:invoke)
run_rake_task('gitlab:shell:install')
end
end
end
```
---
[Return to Testing documentation](index.md)