Skip to content

Upgrade bootstrap-vue

Enrique Alcántara requested to merge upgrade-bootstrap-vue-testt into master

What does this MR do?

This MR ensures that upgrading GitLab UI will go smoothly after the BootstrapVue upgrade to v2.1.0: gitlab-ui!855 (merged)

Currently, GitLab UI depends on a release candidate of BootstrapVue 2 and it appears that the stable version comes with a number of changes that might break components in GitLab.

Failures summary

This MR caused a number of failures in our test suites. The affected tests are based on Karma, Jest and RSpec.

1. Karma

We have decided to migrate the following specs from Karma to Jest so that we can focus on a single approach for making our test compatible with the new tooltip component in BootstrapVue:

Issue boards !19265 (merged)

  • spec/javascripts/boards/components/issue_time_estimate_spec.js
  • spec/javascripts/boards/issue_card_spec.js

Monitoring !19266 (merged)

  • spec/javascripts/monitoring/charts/time_series_spec.js
  • spec/javascripts/monitoring/panel_type_spec.js

Notes !19267 (merged)

  • spec/javascripts/notes/components/comment_form_spec.js

Pipelines !19270 (merged)

  • spec/javascripts/pipelines/graph/action_component_spec.js
  • spec/javascripts/pipelines/pipeline_triggerer_spec.js
  • spec/javascripts/pipelines/pipelines_table_row_spec.js

Vue shared components !19269 (merged)

  • spec/javascripts/vue_shared/components/commit_spec.js
  • spec/javascripts/vue_shared/components/user_avatar/user_avatar_image_spec.js
  • spec/javascripts/vue_shared/components/user_popover/user_popover_spec.js

Approvals !19263 (merged)

  • ee/spec/javascripts/approvals/components/approval_check_popover_spec.js

Related items tree !19268 (merged)

  • ee/spec/javascripts/related_items_tree/components/related_items_tree_header_spec.js
  • ee/spec/javascripts/related_items_tree/components/tree_item_body_spec.js
  • ee/spec/javascripts/related_items_tree/components/tree_item_spec.js

Feature flags !19880 (merged)

  • ee/spec/javascripts/feature_flags/components/feature_flags_table_spec.js

2. Jest

NOTE: The migration path described below is outdated. We have decided to implement a Jest mock to fix tooltips-related failures, which removes the need to enable attachToDocument in Jest specs.

Now that we have migrated failing Karma specs to Jest, we can get an overview of what needs to be fixed on the Jest-side.

As of writing this, we have 674 Jest failures across 74 files that we can split into 31 individual categories.

Opening a MR to fix a spec

Use the following template to describe a merge request that fixes one or more specs. The template explains what changes the spec contains and why we have to apply those changes.

## Prepare [spec name] specs for BootstrapVue upgrade 

This MR lays the groundwork for the BootstrapVue upgrade that will be done in this MR: https://gitlab.com/gitlab-org/gitlab/merge_requests/18913. Due to some changes in the `BTooltip` and `BPopover` components, some related tests started failing after the upgrade. Those tests need to be updated accordingly:

- Migrate specs to use `vue-test-utils`
- `sync` needs to be set to `false`
- `attachToDocument` must be `true`

This MR focuses on upgrading the tests located in `[spec file relative paths]`:

### Why are these changes necessary?

- The reason to migrate the tests to `vue-test-utils` is that it allows us to set the `sync` and `attachToDocument` properties.
- The reason to set `attachToDocument` to `true` is that the BTooltip and BPopover directives expect to be attached to a document object. 
- The reason to set `sync` to `false` is that otherwise, specs for components that use `BTooltip` or `BPopover` will fail with the following error:

`TypeError: Cannot read property 'sync' of null`

Notes

  • You can install this version of gitlab-ui to test your changes with the latest version of bootstrap-vue: https://gitlab.com/gitlab-org/gitlab-ui/-/jobs/358950978/artifacts/raw/gitlab-ui.upgrade-bootstrap-vue.tgz

  • If you have to migrate a spec to vue-test-utils, we recommend opening a MR for that single spec file. If the spec only requires setting the sync and attachToDocument properties, you can group two or more specs in a single MR.

  • While updating the specs, you might notice that some assertions are trying to get the contents of the data-original-title attribute which has been removed in the latest version of bootstrap-vue. You should not change these assertions, otherwise the spec will fail when you rebase your branch on master. These assertions will be updated in this MR.

Failing specs

If you want to work on some of these specs, put your name next to the specs group or spec file you want to work on. There are examples on the list.

  • Analytics - @pgascouvaillancourt - !19903 (merged)

    • ee/spec/frontend/analytics/cycle_analytics/components/stage_event_list_spec.js
    • ee/spec/frontend/analytics/cycle_analytics/components/stage_table_spec.js

  • Clusters (EE)

    • ee/spec/frontend/clusters/components/environments_spec.js
      • Fix tables slots in this MR

  • Dependencies - @pgascouvaillancourt - !19906 (merged)

    • ee/spec/frontend/dependencies/components/app_spec.js
    • ee/spec/frontend/dependencies/components/dependencies_actions_spec.js
      • Fix tabs slots in this MR
    • ee/spec/frontend/dependencies/components/dependency_license_links_spec.js

  • Environments dashboard - Paul - !19908 (merged)

    • ee/spec/frontend/environments_dashboard/components/environment_header_spec.js
    • ee/spec/frontend/environments_dashboard/components/environment_spec.js
    • ee/spec/frontend/environments_dashboard/components/project_header_spec.js

  • Related items tree - @pgascouvaillancourt - !19916 (merged)

    • ee/spec/frontend/related_items_tree/components/related_items_tree_header_spec.js
    • ee/spec/frontend/related_items_tree/components/tree_item_body_spec.js
    • ee/spec/frontend/related_items_tree/components/tree_item_spec.js

  • Security dashboard - @ekigbo

    • ee/spec/frontend/security_dashboard/components/filter_spec.js
    • ee/spec/frontend/security_dashboard/components/project_list_spec.js

  • MR widget - @vitallium - !20574 (merged)

    • ee/spec/frontend/vue_mr_widget/components/approvals/approvals_summary_optional_spec.js

  • Vue shared (EE) - @nmezzopera - !20712 (merged)

    • ee/spec/frontend/vue_shared/security_reports/card_security_reports_app_spec.js
    • ee/spec/frontend/vue_shared/security_reports/components/dismiss_button_spec.js
    • ee/spec/frontend/vue_shared/security_reports/components/dismissal_note_spec.js
    • ee/spec/frontend/vue_shared/security_reports/components/event_item_spec.js
    • ee/spec/frontend/vue_shared/security_reports/components/modal_footer_spec.js
    • ee/spec/frontend/vue_shared/security_reports/components/modal_spec.js

  • Clusters

    • spec/frontend/clusters/components/applications_spec.js

  • Diffs - @vitallium - !20989 (merged)

    • spec/frontend/diffs/components/diff_file_header_spec.js
    • spec/frontend/diffs/components/diff_gutter_avatars_spec.js
    • spec/frontend/diffs/components/edit_button_spec.js

  • Issuable suggestions - @vitallium - !20990 (merged)

    • spec/frontend/issuable_suggestions/components/app_spec.js
    • spec/frontend/issuable_suggestions/components/item_spec.js

  • Issuables list - @vitallium - !20991 (merged)

    • spec/frontend/issuables_list/components/issuable_spec.js
    • spec/frontend/issuables_list/components/issuables_list_app_spec.js

  • Monitoring - @dbodicherla

    • spec/frontend/monitoring/charts/time_series_spec.js

  • Pipelines - @vitallium

    • spec/frontend/pipelines/graph/action_component_spec.js !20987 (merged)
    • spec/frontend/pipelines/pipeline_triggerer_spec.js !20987 (merged)
    • spec/frontend/pipelines/pipelines_table_row_spec.js

  • Registry - @nkipling - !20504 (merged)

    • spec/frontend/registry/components/app_spec.js
    • spec/frontend/registry/components/collapsible_container_spec.js
    • spec/frontend/registry/components/project_empty_state_spec.js
    • spec/frontend/registry/components/table_registry_spec.js

  • Releases - @ekigbo

    • spec/frontend/releases/detail/components/app_spec.js !20541 (merged)
    • spec/frontend/releases/list/components/release_block_footer_spec.js
    • spec/frontend/releases/list/components/release_block_spec.js

  • Repository

    • spec/frontend/repository/components/last_commit_spec.js

  • Sidebar - @pgascouvaillancourt - !19901 (merged)

    • spec/frontend/sidebar/components/assignees/assignee_avatar_link_spec.js
    • spec/frontend/sidebar/components/assignees/collapsed_assignee_list_spec.js
    • spec/frontend/sidebar/components/assignees/uncollapsed_assignee_list_spec.js

  • Vue shared - @ealcantara

    • spec/frontend/vue_shared/components/changed_file_icon_spec.js !20240 (merged)
    • spec/frontend/vue_shared/components/commit_spec.js !20240 (merged)
    • spec/frontend/vue_shared/components/issue/issue_assignees_spec.js !20240 (merged)
    • spec/frontend/vue_shared/components/issue/issue_milestone_spec.js !20240 (merged)
    • spec/frontend/vue_shared/components/issue/related_issuable_item_spec.js !20240 (merged)
    • spec/frontend/vue_shared/components/markdown/header_spec.js !20376 (merged)
    • spec/frontend/vue_shared/components/markdown/suggestion_diff_header_spec.js !20240 (merged)
    • spec/frontend/vue_shared/components/modal_copy_button_spec.js !20240 (merged)
    • spec/frontend/vue_shared/components/notes/system_note_spec.js !20379 (merged)
    • spec/frontend/vue_shared/components/paginated_list_spec.js !20240 (merged)
    • spec/frontend/vue_shared/components/sidebar/labels_select/base_spec.js !20240 (merged)
    • spec/frontend/vue_shared/components/sidebar/labels_select/dropdown_value_spec.js !20552 (merged)
    • spec/frontend/vue_shared/components/time_ago_tooltip_spec.js !20238 (merged)
    • spec/frontend/vue_shared/components/user_popover/user_popover_spec.js !20553 (merged)

3. Migrate to new slots syntax

BootstrapVue now has a new slots syntax for BTable and BTabs components, which are wrapped by GlTable and GlTabs respectively. We'll need to update every usage of these components to the new syntax. Here's an example for GlTable:

diff --git a/ee/app/assets/javascripts/packages/details/components/app.vue b/ee/app/assets/javascripts/packages/details/components/app.vue
index 8fad3f20d37..cb43387f235 100644
--- a/ee/app/assets/javascripts/packages/details/components/app.vue
+++ b/ee/app/assets/javascripts/packages/details/components/app.vue
@@ -225,7 +225,7 @@ export default {
       :items="filesTableRows"
       tbody-tr-class="js-file-row"
     >
-      <template #name="items">
+      <template #cell(name)="items">
         <icon name="doc-code" class="space-right" />
         <gl-link :href="items.item.downloadPath" class="js-file-download">{{
           items.item.name

What changes will we ship in this MR?

We're trying to do as many changes as possible in separate MRs to reduce the scope of this one. However, some changes can't be decoupled from the actual upgrade because making them beforehand would simply break master. Those changes will have to be done here. Here's what we're planning on shipping with this MR:

  • Update tests that depend on data-original-title: this attribute has been deprecated in favor of title
  • Update gl-table slots syntax: the slots syntax for the underlying b-table component has changed and will have to be updated. Here's an example:
diff --git a/ee/app/assets/javascripts/packages/details/components/app.vue b/ee/app/assets/javascripts/packages/details/components/app.vue
index 8fad3f20d37..cb43387f235 100644
--- a/ee/app/assets/javascripts/packages/details/components/app.vue
+++ b/ee/app/assets/javascripts/packages/details/components/app.vue
@@ -225,7 +225,7 @@ export default {
       :items="filesTableRows"
       tbody-tr-class="js-file-row"
     >
-      <template #name="items">
+      <template #cell(name)="items">
         <icon name="doc-code" class="space-right" />
         <gl-link :href="items.item.downloadPath" class="js-file-download">{{
           items.item.name
  • Update tabs to use the new v-slot:tabs-start and v-slot:tabs-end slots

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Edited by Paul Gascou-Vaillancourt

Merge request reports