Add latest changes from gitlab-org/gitlab@13-10-stable-eev13.10.0-rc40

10 files changed, 152 insertions, 53 deletions

diff --git a/doc/development/testing_guide/best_practices.md b/doc/development/testing_guide/best_practices.md
index 79ff46ae352..ee8401e08d4 100644
--- a/doc/development/testing_guide/best_practices.md
+++ b/doc/development/testing_guide/best_practices.md
@@ -168,7 +168,7 @@ can be used:
 
 ```ruby
 RSpec.describe API::Search, factory_default: :keep do
-  let_it_be(:namespace) { create_default(:namespace) }
+  let_it_be(:namespace) { create_default(:namespace).freeze }
 ```
 
 Then every project we create uses this `namespace`, without us having to pass
@@ -176,13 +176,17 @@ it as `namespace: namespace`. In order to make it work along with `let_it_be`, `
 must be explicitly specified. That keeps the default factory for every example in a suite instead of
 recreating it for each example.
 
+Objects created inside a `factory_default: :keep`, and using
+`create_default` inside a `let_it_be` should be frozen to prevent accidental reliance
+between test examples.
+
 Maybe we don't need to create 208 different projects - we
 can create one and reuse it. In addition, we can see that only about 1/3 of the
 projects we create are ones we ask for (76/208). There is benefit in setting
 a default value for projects as well:
 
 ```ruby
-  let_it_be(:project) { create_default(:project) }
+  let_it_be(:project) { create_default(:project).freeze }
 ```
 
 In this case, the `total time` and `top-level time` numbers match more closely:
@@ -541,7 +545,7 @@ end
 
 ### Feature flags in tests
 
-This section was moved to [developing with feature flags](../feature_flags/development.md).
+This section was moved to [developing with feature flags](../feature_flags/index.md).
 
 ### Pristine test environments
 
@@ -796,10 +800,11 @@ end
 ```
 
 WARNING:
-Only use simple values as input in the `where` block. Using procs, stateful
+Only use simple values as input in the `where` block. Using
+<!-- vale gitlab.Spelling = NO --> procs, stateful
 objects, FactoryBot-created objects, and similar items can lead to
 [unexpected results](https://github.com/tomykaira/rspec-parameterized/issues/8).
-
+<!-- vale gitlab.Spelling = YES -->
 ### Prometheus tests
 
 Prometheus metrics may be preserved from one test run to another. To ensure that metrics are
@@ -981,6 +986,7 @@ GitLab uses [factory_bot](https://github.com/thoughtbot/factory_bot) as a test f
   See [issue #262624](https://gitlab.com/gitlab-org/gitlab/-/issues/262624) for further context.
 - Factories don't have to be limited to `ActiveRecord` objects.
   [See example](https://gitlab.com/gitlab-org/gitlab-foss/commit/0b8cefd3b2385a21cfed779bd659978c0402766d).
+- Factories and their traits should produce valid objects that are [verified by specs](https://gitlab.com/gitlab-org/gitlab/-/blob/master/spec/factories_spec.rb).
 
 ### Fixtures
 
diff --git a/doc/development/testing_guide/end_to_end/best_practices.md b/doc/development/testing_guide/end_to_end/best_practices.md
index a5a2d2a1113..2b4212a0172 100644
--- a/doc/development/testing_guide/end_to_end/best_practices.md
+++ b/doc/development/testing_guide/end_to_end/best_practices.md
@@ -309,10 +309,10 @@ This action can also unintentionally click other elements, altering the test sta
 # Clicking another element to blur an input
 def add_issue_to_epic(issue_url)
   find_element(:issue_actions_split_button).find('button', text: 'Add an issue').click
-  fill_element :add_issue_input, issue_url
+  fill_element(:add_issue_input, issue_url)
   # Clicking the title blurs the input
-  click_element :title
-  click_element :add_issue_button
+  click_element(:title)
+  click_element(:add_issue_button)
 end
 
 # Using native mouse click events in the case of a mask/overlay
diff --git a/doc/development/testing_guide/end_to_end/dynamic_element_validation.md b/doc/development/testing_guide/end_to_end/dynamic_element_validation.md
index 1e7f528f6ff..6c504e6fa28 100644
--- a/doc/development/testing_guide/end_to_end/dynamic_element_validation.md
+++ b/doc/development/testing_guide/end_to_end/dynamic_element_validation.md
@@ -39,7 +39,7 @@ appear on the webpage, or the test to navigate away from the page entirely.
 Dynamic element validation is instituted when using
 
 ```ruby
-click_element :my_element, Some::Page
+click_element(:my_element, Some::Page)
 ```
 
 ### Required Elements
@@ -79,7 +79,7 @@ class MyPage < Page::Base
   end
 
   def open_layer
-    click_element :my_element, Layer::MyLayer
+    click_element(:my_element, Layer::MyLayer)
   end
 end
 
@@ -109,7 +109,7 @@ Given the [source](#examples) ...
 
 ```ruby
 def open_layer
-  click_element :my_element, Layer::MyLayer
+  click_element(:my_element, Layer::MyLayer)
 end
 ```
 
diff --git a/doc/development/testing_guide/end_to_end/environment_selection.md b/doc/development/testing_guide/end_to_end/environment_selection.md
index 7e34b6c265d..bcdf0e104dd 100644
--- a/doc/development/testing_guide/end_to_end/environment_selection.md
+++ b/doc/development/testing_guide/end_to_end/environment_selection.md
@@ -58,7 +58,7 @@ end
 If the test has a `before` or `after`, you must add the `only` metadata
 to the outer `RSpec.describe`.
 
-If you want to run an `only: { :pipeline }` tagged test on your local GDK make sure either the `CI_PROJECT_NAME` environment variable is unset, or that the `CI_PROJECT_NAME` environment variable matches the specified pipeline in the `only: { :pipeline }` tag, or just delete the `only: { :pipeline }` tag.
+If you want to run an `only: { :pipeline }` tagged test on your local GDK make sure either the `CI_PROJECT_NAME` CI/CD variable is unset, or that the `CI_PROJECT_NAME` variable matches the specified pipeline in the `only: { :pipeline }` tag, or just delete the `only: { :pipeline }` tag.
 
 ## Quarantining a test for a specific environment
 
diff --git a/doc/development/testing_guide/end_to_end/feature_flags.md b/doc/development/testing_guide/end_to_end/feature_flags.md
index 1bc33b79c7c..e3719393d41 100644
--- a/doc/development/testing_guide/end_to_end/feature_flags.md
+++ b/doc/development/testing_guide/end_to_end/feature_flags.md
@@ -18,8 +18,8 @@ Please be sure to include the tag `:requires_admin` so that the test can be skip
 where admin access is not available.
 
 WARNING:
-You are strongly advised to [enable feature flags only for a group, project, user](../../feature_flags/development.md#feature-actors),
-or [feature group](../../feature_flags/development.md#feature-groups). This makes it possible to
+You are strongly advised to [enable feature flags only for a group, project, user](../../feature_flags/index.md#feature-actors),
+or [feature group](../../feature_flags/index.md#feature-groups). This makes it possible to
 test a feature in a shared environment without affecting other users.
 
 For example, the code below would enable a feature flag named `:feature_flag_name` for the project
diff --git a/doc/development/testing_guide/end_to_end/page_objects.md b/doc/development/testing_guide/end_to_end/page_objects.md
index d9309f74e0e..b124ac430f6 100644
--- a/doc/development/testing_guide/end_to_end/page_objects.md
+++ b/doc/development/testing_guide/end_to_end/page_objects.md
@@ -201,7 +201,7 @@ We can select on that specific issue by matching on the Rails model.
 ```ruby
 class Page::Project::Issues::Index < Page::Base
   def has_issue?(issue)
-    has_element? :issue, issue_title: issue
+    has_element?(:issue, issue_title: issue)
   end
 end
 ```
diff --git a/doc/development/testing_guide/end_to_end/running_tests_that_require_special_setup.md b/doc/development/testing_guide/end_to_end/running_tests_that_require_special_setup.md
index b6293ec41b8..ea48a3aa8b8 100644
--- a/doc/development/testing_guide/end_to_end/running_tests_that_require_special_setup.md
+++ b/doc/development/testing_guide/end_to_end/running_tests_that_require_special_setup.md
@@ -151,7 +151,7 @@ To run the Monitor tests locally, against the GDK, please follow the preparation
 1. The test setup deploys the app in a Kubernetes cluster, using the Auto DevOps deployment strategy.
 To enable Auto DevOps in GDK, follow the [associated setup](https://gitlab.com/gitlab-org/gitlab-development-kit/-/blob/master/doc/howto/auto_devops/index.md#setup) instructions. If you have problems, review the [troubleshooting guide](https://gitlab.com/gitlab-org/gitlab-development-kit/-/blob/master/doc/howto/auto_devops/tips_and_troubleshooting.md) or reach out to the `#gdk` channel in the internal GitLab Slack.
 1. Do [secure your GitLab instance](https://gitlab.com/gitlab-org/gitlab-development-kit/-/blob/master/doc/howto/auto_devops/index.md#secure-your-gitlab-instance) since it is now publicly accessible on `https://[YOUR-PORT].qa-tunnel.gitlab.info`.
-1. Install the Kubernetes command line tool known as `kubectl`. Use the [official installation instructions](https://kubernetes.io/docs/tasks/tools/install-kubectl/).
+1. Install the Kubernetes command line tool known as `kubectl`. Use the [official installation instructions](https://kubernetes.io/docs/tasks/tools/).
 
 You might see NGINX issues when you run `gdk start` or `gdk restart`. In that case, run `sft login` to revalidate your credentials and regain access the QA Tunnel.
 
diff --git a/doc/development/testing_guide/end_to_end/style_guide.md b/doc/development/testing_guide/end_to_end/style_guide.md
index ac4d26df794..f9c13d5dd67 100644
--- a/doc/development/testing_guide/end_to_end/style_guide.md
+++ b/doc/development/testing_guide/end_to_end/style_guide.md
@@ -19,7 +19,7 @@ E.g.:
 ```ruby
 def click_ci_cd_pipelines
   within_sidebar do
-    click_element :link_pipelines
+    click_element(:link_pipelines)
   end
 end
 ```
diff --git a/doc/development/testing_guide/frontend_testing.md b/doc/development/testing_guide/frontend_testing.md
index 73fce3a38d7..9facca10142 100644
--- a/doc/development/testing_guide/frontend_testing.md
+++ b/doc/development/testing_guide/frontend_testing.md
@@ -126,7 +126,7 @@ It does not make sense to test our `getFahrenheit` function because underneath i
 Let's take a short look into Vue land. Vue is a critical part of the GitLab JavaScript codebase. When writing specs for Vue components, a common gotcha is to actually end up testing Vue provided functionality, because it appears to be the easiest thing to test. Here's an example taken from our codebase.
 
 ```javascript
-// Component
+// Component script
 {
   computed: {
     hasMetricTypes() {
@@ -135,27 +135,46 @@ Let's take a short look into Vue land. Vue is a critical part of the GitLab Java
 }
 ```
 
-and here's the corresponding spec
+```html
+<!-- Component template -->
+<template>
+  <gl-dropdown v-if="hasMetricTypes">
+    <!-- Dropdown content -->
+  </gl-dropdown>
+</template>
+```
+
+Testing the `hasMetricTypes` computed prop would seem like a given here. But to test if the computed property is returning the length of `metricTypes`, is testing the Vue library itself. There is no value in this, besides it adding to the test suite. It's better to test a component in the way the user interacts with it: checking the rendered template.
 
 ```javascript
- describe('computed', () => {
-    describe('hasMetricTypes', () => {
-      it('returns true if metricTypes exist', () => {
-        factory({ metricTypes });
-        expect(wrapper.vm.hasMetricTypes).toBe(2);
-      });
-
-      it('returns true if no metricTypes exist', () => {
-        factory();
-        expect(wrapper.vm.hasMetricTypes).toBe(0);
-      });
+// Bad
+describe('computed', () => {
+  describe('hasMetricTypes', () => {
+    it('returns true if metricTypes exist', () => {
+      factory({ metricTypes });
+      expect(wrapper.vm.hasMetricTypes).toBe(2);
+    });
+
+    it('returns true if no metricTypes exist', () => {
+      factory();
+      expect(wrapper.vm.hasMetricTypes).toBe(0);
     });
+  });
 });
-```
 
-Testing the `hasMetricTypes` computed prop would seem like a given, but to test if the computed property is returning the length of `metricTypes`, is testing the Vue library itself. There is no value in this, besides it adding to the test suite. Better is to test it in the way the user interacts with it. Probably through the template.
+// Good
+it('displays a dropdown if metricTypes exist', () => {
+  factory({ metricTypes });
+  expect(wrapper.findComponent(GlDropdown).exists()).toBe(true);
+});
+
+it('does not display a dropdown if no metricTypes exist', () => {
+  factory();
+  expect(wrapper.findComponent(GlDropdown).exists()).toBe(false);
+});
+```
 
-Keep an eye out for these kinds of tests, as they just make updating logic more fragile and tedious than it needs to be. This is also true for other libraries.
+Keep an eye out for these kinds of tests, as they just make updating logic more fragile and tedious than it needs to be. This is also true for other libraries. A rule of thumb here is: if you are checking a `wrapper.vm` property, you should probably stop and rethink the test to check the rendered template instead.
 
 Some more examples can be found in the [Frontend unit tests section](testing_levels.md#frontend-unit-tests)
 
@@ -193,7 +212,7 @@ When it comes to querying DOM elements in your tests, it is best to uniquely and
 the element.
 
 Preferentially, this is done by targeting what the user actually sees using [DOM Testing Library](https://testing-library.com/docs/dom-testing-library/intro/).
-When selecting by text it is best to use [`getByRole` or `findByRole`](https://testing-library.com/docs/dom-testing-library/api-queries/#byrole)
+When selecting by text it is best to use [`getByRole` or `findByRole`](https://testing-library.com/docs/queries/byrole/)
 as these enforce accessibility best practices as well. The examples below demonstrate the order of preference.
 
 When writing Vue component unit tests, it can be wise to query children by component, so that the unit test can focus on comprehensive value coverage
@@ -539,21 +558,6 @@ When looking at this initially you'd suspect that the component is setup before
 
 This is however not entirely true as the `destroy` method does not remove everything which has been mutated on the `wrapper` object. For functional components, destroy only removes the rendered DOM elements from the document.
 
-In order to ensure that a clean wrapper object and DOM are being used in each test, the breakdown of the component should rather be performed as follows:
-
-```javascript
-  afterEach(() => {
-    wrapper.destroy();
-    wrapper = null;
-  });
-```
-
-<!-- vale gitlab.Spelling = NO -->
-
-See also the [Vue Test Utils documentation on `destroy`](https://vue-test-utils.vuejs.org/api/wrapper/#destroy).
-
-<!-- vale gitlab.Spelling = YES -->
-
 ### Jest best practices
 
 > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/34209) in GitLab 13.2.
@@ -687,7 +691,7 @@ Similarly, if you really need to use the real `Date` class, then you can import
 ```javascript
 import { useRealDate } from 'helpers/fake_date';
 
-// NOTE: `useRealDate` cannot be called during test execution (i.e. inside `it`, `beforeEach`, `beforeAll`, etc.). 
+// NOTE: `useRealDate` cannot be called during test execution (i.e. inside `it`, `beforeEach`, `beforeAll`, etc.).
 describe('with real date', () => {
   useRealDate();
 });
@@ -1034,7 +1038,7 @@ describe "Admin::AbuseReports", :js do
 end
 ```
 
-### Jest test timeout due to async imports
+### Jest test timeout due to asynchronous imports
 
 If a module asynchronously imports some other modules at runtime, these modules must be
 transpiled by the Jest loaders at runtime. It's possible that this can cause [Jest to timeout](https://gitlab.com/gitlab-org/gitlab/-/issues/280809).
@@ -1183,6 +1187,95 @@ You can download any older version of Firefox from the releases FTP server, <htt
 1. Open up a terminal and run `/Applications/Firefox_Old.app/Contents/MacOS/firefox-bin -profilemanager` to create a new profile specific to that Firefox version.
 1. Once the profile has been created, quit the app, and run it again like normal. You now have a working older Firefox version.
 
+## Snapshots
+
+By now you've probably heard of [Jest snapshot tests](https://jestjs.io/docs/en/snapshot-testing) and why they are useful for various reasons.
+To use them within GitLab, there are a few guidelines that should be highlighted:
+
+- Treat snapshots as code
+- Don't think of a snapshot file as a Blackbox
+- Care for the output of the snapshot, otherwise, it's not providing any real value. This will usually involve reading the generated snapshot file as you would read any other piece of code
+
+Think of a snapshot test as a simple way to store a raw `String` representation of what you've put into the item being tested. This can be used to evaluate changes in a component, a store, a complex piece of generated output, etc. You can see more in the list below for some recommended `Do's and Don'ts`.
+While snapshot tests can be a very powerful tool. They are meant to supplement, not to replace unit tests.
+
+Jest provides a great set of docs on [best practices](https://jestjs.io/docs/en/snapshot-testing#best-practices) that we should keep in mind when creating snapshots.
+
+### How does a snapshot work?
+
+A snapshot is purely a stringified version of what you ask to be tested on the lefthand side of the function call. This means any kind of changes you make to the formatting of the string has an impact on the outcome. This process is done by leveraging serializers for an automatic transform step. For Vue this is already taken care of by leveraging the `vue-jest` package, which offers the proper serializer.
+
+Should the outcome of your spec be different from what is in the generated snapshot file, you'll be notified about it by a failing test in your test suite.
+
+Find all the details in Jests official documentation [https://jestjs.io/docs/en/snapshot-testing](https://jestjs.io/docs/en/snapshot-testing)
+
+### How to take a snapshot
+
+```javascript
+it('makes the name look pretty', () => {
+  expect(prettifyName('Homer Simpson')).toMatchSnapshot()
+})
+```
+
+When this test runs the first time a fresh `.snap` file will be created. It will look something like this:
+
+```txt
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`makes the name look pretty`] = `
+Sir Homer Simpson the Third
+`
+```
+
+Now, every time you call this test, the new snapshot will be evaluated against the previously created version. This should highlight the fact that it's important to understand the content of your snapshot file and treat it with care. Snapshots will lose their value if the output of the snapshot is too big or complex to read, this means keeping snapshots isolated to human-readable items that can be either evaluated in a merge request review or are guaranteed to never change.
+The same can be done for `wrappers` or `elements`
+
+```javascript
+it('renders the component correctly', () => {
+  expect(wrapper).toMatchSnapshot()
+  expect(wrapper.element).toMatchSnapshot();
+})
+```
+
+The above test will create two snapshots, what's important is to decide which of the snapshots provide more value for the codebase safety i.e. if one of these snapshots changes, does that highlight a possible un-wanted break in the codebase? This can help catch unexpected changes if something in an underlying dependency changes without our knowledge.
+
+### Pros and Cons
+
+**Pros**
+
+- Speed up the creation of unit tests
+- Easy to maintain
+- Provides a good safety net to protect against accidental breakage of important HTML structures
+
+**Cons**
+
+- Is not a catch-all solution that replaces the work of integration or unit tests
+- No meaningful assertions or expectations within snapshots
+- When carelessly used with [GitLab UI](https://gitlab.com/gitlab-org/gitlab-ui) it can create fragility in tests when the underlying library changes the HTML of a component we are testing
+
+A good guideline to follow: the more complex the component you may want to steer away from just snapshot testing. But that's not to say you can't still snapshot test and test your component as normal.
+
+### When to use
+
+**Use snapshots when**
+
+- to capture a components rendered output
+- to fully or partially match templates
+- to match readable data structures
+- to verify correctly composed native HTML elements
+- as a safety net for critical structures so others don't break it by accident
+- Template heavy component
+- Not a lot of logic in the component
+- Composed of native HTML elements
+
+### When not to use
+
+**Don't use snapshots when**
+
+- To capture large data structures just to have something
+- To just have some kind of test written
+- To capture highly volatile ui elements without stubbing them (Think of GitLab UI version updates)
+
 ---
 
 [Return to Testing documentation](index.md)
diff --git a/doc/development/testing_guide/review_apps.md b/doc/development/testing_guide/review_apps.md
index f1c74f990cb..c4194be23a4 100644
--- a/doc/development/testing_guide/review_apps.md
+++ b/doc/development/testing_guide/review_apps.md
@@ -109,10 +109,10 @@ subgraph "CNG-mirror pipeline"
 ### Auto-stopping of Review Apps
 
 Review Apps are automatically stopped 2 days after the last deployment thanks to
-the [Environment auto-stop](../../ci/environments/index.md#environments-auto-stop) feature.
+the [Environment auto-stop](../../ci/environments/index.md#stop-an-environment-after-a-certain-time-period) feature.
 
 If you need your Review App to stay up for a longer time, you can
-[pin its environment](../../ci/environments/index.md#auto-stop-example) or retry the
+[pin its environment](../../ci/environments/index.md#override-a-deployments-scheduled-stop-time) or retry the
 `review-deploy` job to update the "latest deployed at" time.
 
 The `review-cleanup` job that automatically runs in scheduled