Add latest changes from gitlab-org/gitlab@master

4 files changed, 211 insertions, 80 deletions

diff --git a/doc/ci/jobs/job_artifacts.md b/doc/ci/jobs/job_artifacts.md
index a6838f2e87a..0d8c9dcf715 100644
--- a/doc/ci/jobs/job_artifacts.md
+++ b/doc/ci/jobs/job_artifacts.md
@@ -188,7 +188,10 @@ job:
 
 ## View all job artifacts in a project
 
-> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/254938) in GitLab 15.11 [with a flag](../../administration/feature_flags.md) named `artifacts_management_page`. Disabled by default.
+> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/31271) in GitLab 12.4 [with a flag](../../administration/feature_flags.md) named `artifacts_management_page`. Disabled by default.
+> - [Improved look](https://gitlab.com/gitlab-org/gitlab/-/issues/33418) in GitLab 15.6.
+> - [Improved performance](https://gitlab.com/gitlab-org/gitlab/-/issues/387765) in GitLab 15.9.
+> - [Generally available](https://gitlab.com/gitlab-org/gitlab/-/issues/407475) in GitLab 16.0. Feature flag `artifacts_management_page` removed.
 
 You can view all artifacts stored in a project from the **CI/CD > Artifacts** page.
 This list displays all jobs and their associated artifacts. Expand an entry to access
diff --git a/doc/development/cicd/cicd_tables.md b/doc/development/cicd/cicd_tables.md
index c86540c10f0..b246328a817 100644
--- a/doc/development/cicd/cicd_tables.md
+++ b/doc/development/cicd/cicd_tables.md
@@ -15,25 +15,27 @@ should be partitioned from the start.
 
 ## Create a new routing table
 
-The database helpers for creating tables do not accept partitioning options,
-so the best solution is to create the tables using raw SQL:
+Here is an example on how to use database helpers to create a new table and foreign keys:
 
 ```ruby
-  enable_lock_retries!
+  include Gitlab::Database::PartitioningMigrationHelpers
+  disable_ddl_transaction!
 
   def up
-    execute(<<~SQL)
-      CREATE TABLE p_ci_examples (
-        id bigint NOT NULL,
-        partition_id bigint NOT NULL,
-        build_id bigint NOT NULL,
-        PRIMARY KEY (id, partition_id),
-        CONSTRAINT fk_bb490f12fe_p FOREIGN KEY (partition_id, build_id)
-           REFERENCES ci_builds(partition_id, id)
-           ON UPDATE CASCADE ON DELETE CASCADE
-      )
-      PARTITION BY LIST (partition_id);
-    SQL
+    create_table(:p_ci_examples, primary_key: [:id, :partition_id], options: 'PARTITION BY LIST (partition_id)', if_not_exists: true) do |t|
+      t.bigserial :id, null: false
+      t.bigint :partition_id, null: false
+      t.bigint :build_id, null: false
+    end
+
+    add_concurrent_partitioned_foreign_key(
+      :p_ci_examples, :ci_builds,
+      column: [:partition_id, :build_id],
+      target_column: [:partition_id, :id],
+      on_update: :cascade,
+      on_delete: :cascade,
+      reverse_lock_order: true
+    )
   end
 
   def down
diff --git a/doc/development/testing_guide/frontend_testing.md b/doc/development/testing_guide/frontend_testing.md
index ba965ab204a..c815851579a 100644
--- a/doc/development/testing_guide/frontend_testing.md
+++ b/doc/development/testing_guide/frontend_testing.md
@@ -1230,15 +1230,14 @@ You can download any older version of Firefox from the releases FTP server, <htt
 
 ## Snapshots
 
-By now you've probably heard of [Jest snapshot tests](https://jestjs.io/docs/snapshot-testing) and why they are useful for various reasons.
-To use them within GitLab, there are a few guidelines that should be highlighted:
+[Jest snapshot tests](https://jestjs.io/docs/snapshot-testing) are a useful way to prevent unexpected changes to the HTML output of a given component. They should **only** be used when other testing methods (such as asserting elements with `vue-tests-utils`) do not cover the required usecase. 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 black box
 - 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.
+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/snapshot-testing#best-practices) that we should keep in mind when creating snapshots.
 
@@ -1250,6 +1249,161 @@ Should the outcome of your spec be different from what is in the generated snaps
 
 Find all the details in Jests official documentation [https://jestjs.io/docs/snapshot-testing](https://jestjs.io/docs/snapshot-testing)
 
+### Pros and Cons
+
+**Pros**
+
+- Provides a good warning against accidental changes of important HTML structures
+- Ease of setup
+
+**Cons**
+
+- Lacks the clarity or guard rails that `vue-tests-utils` provides by finding elements and asserting their presence directly
+- Creates unnecessary noise when updating components purposefully
+- High risk of taking a snapshot of bugs, which then turns the tests against us since the test will now fail when fixing the issue
+- No meaningful assertions or expectations within snapshots makes them harder to reason about or replace
+- When used with dependencies like [GitLab UI](https://gitlab.com/gitlab-org/gitlab-ui), it creates fragility in tests when the underlying library changes the HTML of a component we are testing
+
+### When to use
+
+**Use snapshots when**
+
+- Protecting critical HTML structures so it doesn't change by accident
+- Asserting JS object or JSON outputs of complex utility functions
+
+### When not to use
+
+**Don't use snapshots when**
+
+- Tests could be written using `vue-tests-utils` instead
+- Asserting the logic of a component
+- Predicting data structure(s) outputs
+- There are UI elements outside of the repository (think of GitLab UI version updates)
+
+### Examples
+
+As you can see, the cons of snapshot tests far outweight the pros in general. To illustrate this better, this section will show a few examples of when you might be tempted to
+use snapshot testing and why they are not good patterns.
+
+#### Example #1 - Element visiblity
+
+When testing elements visibility, favour using `vue-tests-utils (VTU)` to find a given component and then a basic `.exists()` method call on the VTU wrapper. This provides better readability and more resilient testing. If you look at the examples below, notice how the assertions on the snapshots do not tell you what you are expecting to see. We are relying entirely on `it` description to give us context and on the assumption that the snapshot has captured the desired behavior.
+
+```vue
+<template>
+  <my-component v-if="isVisible" />
+</template>
+```
+
+Bad:
+
+```javascript
+it('hides the component', () => {
+  createComponent({ props: { isVisible: false }})
+
+  expect(wrapper.element).toMatchSnapshot()
+})
+
+it('shows the component', () => {
+  createComponent({ props: { isVisible: true }})
+
+  expect(wrapper.element).toMatchSnapshot()
+})
+```
+
+Good:
+
+```javascript
+it('hides the component', () => {
+  createComponent({ props: { isVisible: false }})
+
+  expect(findMyComponent().exists()).toBe(false)
+})
+
+it('shows the component', () => {
+  createComponent({ props: { isVisible: true }})
+
+  expect(findMyComponent().exists()).toBe(true)
+})
+```
+
+Not only that, but imagine having passed the wrong prop to your component and having the wrong visibility: the snapshot test would still pass because you would have captured the HTML **with the issue** and so unless you double-checked the output of the snapshot, you would never know that your test is broken.
+
+#### Example #2 - Presence of text
+
+Finding text within a component is very easy by using the `vue-test-utils` method `wrapper.text()`. However, there are some cases where it might be tempting to use snapshots when the value returned has a lot of inconsistent spacing due to formatting or HTML nesting.
+
+In these instances, it is better to assert each string individually and make multiple assertions than to use a snapshot to ignore the spaces. This is because any change to the DOM layout will fail the snapshot test even if the text is still perfectly formatted.
+
+```vue
+<template>
+  <gl-sprintf :message="my-message">
+    <template #code="{ content }">
+      <code>{{ content }}</code>
+    </template>
+  </gl-sprintf>
+  <p> My second message </p>
+</template>
+```
+
+Bad:
+
+```javascript
+it('renders the text as I expect', () => {
+  expect(wrapper.text()).toMatchSnapshot()
+})
+```
+
+Good:
+
+```javascript
+it('renders the code snippet', () => {
+  expect(findCodeTag().text()).toContain("myFunction()")
+})
+
+it('renders the paragraph text', () => {
+  expect(findOtherText().text()).toBe("My second message")
+})
+```
+
+#### Example #3 - Complex HTML
+
+When we have very complex HTML, we should focus on asserting specific sensitive and meaningful points rather than capturing it as a whole. The value in a snapshot test is to **warn developers** that they might have accidentally change an HTML structure that they did not intend to change. If the output of the change is hard to read, which is often the case with complex HTML output, then **is the signal itself that something changed** sufficient? And if it is, can it be accomplished without snapshots?
+
+A good example of a complex HTML output is `GlTable`. Snapshot testing might feel like a good option since you can capture rows and columns structure, but we should instead try to assert text we expect or count the number of rows and columns manually.
+
+```vue
+<template>
+  <gl-table ...all-them-props />
+</template>
+```
+
+Bad:
+
+```javascript
+it('renders GlTable as I expect', () => {
+  expect(findGlTable().element).toMatchSnapshot()
+})
+```
+
+Good:
+
+```javascript
+it('renders the right number of rows', () => {
+  expect(findGlTable().findAllRows()).toHaveLength(expectedLength)
+})
+
+it('renders the special icon that only appears on a full moon', () => {
+  expect(findGlTable().findMoonIcon().exists()).toBe(true)
+})
+
+it('renders the correct email format', () => {
+  expect(findGlTable().text()).toContain('my_strange_email@shaddyprovide.com')
+})
+```
+
+Although more verbose, this now means that our tests are not going to break if `GlTable` changes its internal implementation, we communicate to other developers (or ourselves in 6 months) what is important to preserve when refactoring or adding to our table.
+
 ### How to take a snapshot
 
 ```javascript
@@ -1280,43 +1434,6 @@ it('renders the component correctly', () => {
 
 The above test will create two snapshots. It's important to decide which of the snapshots provide more value for codebase safety. That is, if one of these snapshots changes, does that highlight a possible 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)
-
 ## Get started with feature tests
 
 ### What is a feature test
diff --git a/doc/update/deprecations.md b/doc/update/deprecations.md
index 7b5b48a1d74..09e0693509b 100644
--- a/doc/update/deprecations.md
+++ b/doc/update/deprecations.md
@@ -545,6 +545,39 @@ During the transition to the GitLab Observability UI, we will migrate the [GitLa
 </div>
 </div>
 
+<div class="milestone-wrapper" data-milestone="16.5">
+
+## GitLab 16.5
+
+<div class="deprecation breaking-change" data-milestone="16.5">
+
+### Old versions of JSON web tokens are deprecated
+
+<div class="deprecation-notes">
+- Announced in: GitLab <span class="milestone">15.9</span>
+- [Breaking change](https://docs.gitlab.com/ee/development/deprecation_guidelines/)
+</div>
+
+Now that we have released [ID tokens](https://docs.gitlab.com/ee/ci/secrets/id_token_authentication.html)
+with OIDC support, the old JSON web tokens are deprecated.
+Both the `CI_JOB_JWT` and `CI_JOB_JWT_V2` tokens, exposed to jobs as predefined variables, will:
+
+- Not be creatable in GitLab 16.0 and later.
+- Be removed in GitLab 16.5.
+
+To prepare for this change:
+
+- Before the release of GitLab 16.5, configure your pipelines to use the fully configurable and more secure
+  [`id_token`](https://docs.gitlab.com/ee/ci/yaml/index.html#id_tokens) keyword instead.
+- [Enable the **Limit JSON Web Token (JWT) access**](https://docs.gitlab.com/ee/ci/secrets/id_token_authentication.html#enable-automatic-id-token-authentication)
+  setting, which prevents the old tokens from being exposed to any jobs.
+
+  In GitLab 16.0 and later, the ability to set this option will be removed and all new projects will have the option
+  enabled.
+
+</div>
+</div>
+
 <div class="milestone-wrapper" data-milestone="16.1">
 
 ## GitLab 16.1
@@ -1394,30 +1427,6 @@ and `config/redis.shared_state.yml` files.
 
 <div class="deprecation breaking-change" data-milestone="16.0">
 
-### Old versions of JSON web tokens are deprecated
-
-<div class="deprecation-notes">
-- Announced in: GitLab <span class="milestone">15.9</span>
-- [Breaking change](https://docs.gitlab.com/ee/development/deprecation_guidelines/)
-</div>
-
-Now that we have released [ID tokens](https://docs.gitlab.com/ee/ci/secrets/id_token_authentication.html)
-with OIDC support, the old JSON web tokens are deprecated and will be removed.
-Both the `CI_JOB_JWT` and `CI_JOB_JWT_V2` tokens, exposed to jobs as predefined variables,
-will no longer be available in GitLab 16.0.
-
-To prepare for this change, you should:
-
-- Configure your pipelines to use the fully configurable and more secure
-  [`id_token`](https://docs.gitlab.com/ee/ci/yaml/index.html#id_tokens) keyword instead.
-- [Enable the **Limit JSON Web Token (JWT) access**](https://docs.gitlab.com/ee/ci/secrets/id_token_authentication.html#enable-automatic-id-token-authentication)
-  setting, which prevents the old tokens from being exposed to any jobs. This setting
-  will be permanently enabled for all projects in GitLab 16.0.
-
-</div>
-
-<div class="deprecation breaking-change" data-milestone="16.0">
-
 ### Option to delete projects immediately is deprecated from deletion protection settings
 
 <div class="deprecation-notes">