diff options
32 files changed, 502 insertions, 220 deletions
diff --git a/app/assets/javascripts/pipelines/components/graph/job_group_dropdown.vue b/app/assets/javascripts/pipelines/components/graph/job_group_dropdown.vue index 482898b80c4..ebd7a17040a 100644 --- a/app/assets/javascripts/pipelines/components/graph/job_group_dropdown.vue +++ b/app/assets/javascripts/pipelines/components/graph/job_group_dropdown.vue @@ -69,7 +69,9 @@ export default { > <ci-icon :status="group.status" /> - <span class="ci-status-text"> {{ group.name }} </span> + <span class="ci-status-text text-truncate mw-70p gl-pl-1 d-inline-block align-bottom"> + {{ group.name }} + </span> <span class="dropdown-counter-badge"> {{ group.size }} </span> </button> diff --git a/app/assets/stylesheets/components/avatar.scss b/app/assets/stylesheets/components/avatar.scss index 4ab197b935b..25ee3ca944d 100644 --- a/app/assets/stylesheets/components/avatar.scss +++ b/app/assets/stylesheets/components/avatar.scss @@ -1,28 +1,111 @@ +$avatar-sizes: ( + 16: ( + font-size: 10px, + line-height: 16px, + border-radius: $border-radius-small + ), + 18: ( + border-radius: $border-radius-small + ), + 19: ( + border-radius: $border-radius-small + ), + 20: ( + border-radius: $border-radius-small + ), + 24: ( + font-size: 12px, + line-height: 24px, + border-radius: $border-radius-default + ), + 26: ( + font-size: 20px, + line-height: 1.33, + border-radius: $border-radius-default + ), + 32: ( + font-size: 14px, + line-height: 32px, + border-radius: $border-radius-default + ), + 36: ( + border-radius: $border-radius-default + ), + 40: ( + font-size: 16px, + line-height: 38px, + border-radius: $border-radius-default + ), + 46: ( + border-radius: $border-radius-default + ), + 48: ( + font-size: 20px, + line-height: 48px, + border-radius: $border-radius-large + ), + 60: ( + font-size: 32px, + line-height: 58px, + border-radius: $border-radius-large + ), + 64: ( + font-size: 28px, + line-height: 64px, + border-radius: $border-radius-large + ), + 70: ( + font-size: 34px, + line-height: 70px, + border-radius: $border-radius-large + ), + 90: ( + font-size: 36px, + line-height: 88px, + border-radius: $border-radius-large + ), + 96: ( + font-size: 48px, + line-height: 96px, + border-radius: $border-radius-large + ), + 100: ( + font-size: 36px, + line-height: 98px, + border-radius: $border-radius-large + ), + 110: ( + font-size: 40px, + line-height: 108px, + font-weight: $gl-font-weight-normal, + border-radius: $border-radius-large + ), + 140: ( + font-size: 72px, + line-height: 138px, + border-radius: $border-radius-large + ), + 160: ( + font-size: 96px, + line-height: 158px, + border-radius: $border-radius-large + ) +); + +$identicon-backgrounds: $identicon-red, $identicon-purple, $identicon-indigo, $identicon-blue, $identicon-teal, + $identicon-orange, $gray-darker; + .avatar-circle { float: left; margin-right: 15px; border-radius: $avatar-radius; border: 1px solid $gray-normal; - &.s16 { @include avatar-size(16px, 8px); } - &.s18 { @include avatar-size(18px, 8px); } - &.s19 { @include avatar-size(19px, 8px); } - &.s20 { @include avatar-size(20px, 8px); } - &.s24 { @include avatar-size(24px, 8px); } - &.s26 { @include avatar-size(26px, 8px); } - &.s32 { @include avatar-size(32px, 8px); } - &.s36 { @include avatar-size(36px, 16px); } - &.s40 { @include avatar-size(40px, 16px); } - &.s46 { @include avatar-size(46px, 16px); } - &.s48 { @include avatar-size(48px, 16px); } - &.s60 { @include avatar-size(60px, 16px); } - &.s64 { @include avatar-size(64px, 16px); } - &.s70 { @include avatar-size(70px, 16px); } - &.s90 { @include avatar-size(90px, 16px); } - &.s96 { @include avatar-size(96px, 16px); } - &.s100 { @include avatar-size(100px, 16px); } - &.s110 { @include avatar-size(110px, 16px); } - &.s140 { @include avatar-size(140px, 16px); } - &.s160 { @include avatar-size(160px, 16px); } + + @each $size, $size-config in $avatar-sizes { + &.s#{$size} { + @include avatar-size(#{$size}px, if($size < 36, 8px, 16px)); + } + } } .avatar { @@ -42,8 +125,13 @@ margin-left: 2px; flex-shrink: 0; - &.s16 { margin-right: 4px; } - &.s24 { margin-right: 4px; } + &.s16 { + margin-right: 4px; + } + + &.s24 { + margin-right: 4px; + } } &.center { @@ -69,60 +157,25 @@ background-color: $gray-darker; // Sizes - &.s16 { font-size: 10px; - line-height: 16px; } - - &.s24 { font-size: 12px; - line-height: 24px; } - - &.s26 { font-size: 20px; - line-height: 1.33; } - - &.s32 { font-size: 14px; - line-height: 32px; } - - &.s40 { font-size: 16px; - line-height: 38px; } - - &.s48 { font-size: 20px; - line-height: 48px; } - - &.s60 { font-size: 32px; - line-height: 58px; } - - &.s64 { font-size: 28px; - line-height: 64px; } - - &.s70 { font-size: 34px; - line-height: 70px; } - - &.s90 { font-size: 36px; - line-height: 88px; } - - &.s96 { font-size: 48px; - line-height: 96px; } - - &.s100 { font-size: 36px; - line-height: 98px; } - - &.s110 { font-size: 40px; - line-height: 108px; - font-weight: $gl-font-weight-normal; } - - &.s140 { font-size: 72px; - line-height: 138px; } - - &.s160 { font-size: 96px; - line-height: 158px; } + @each $size, $size-config in $avatar-sizes { + $keys: map-keys($size-config); + + &.s#{$size} { + @each $key in $keys { + // We don't want `border-radius` to be included here. + @if ($key != 'border-radius') { + #{$key}: map-get($size-config, #{$key}); + } + } + } + } // Background colors - &.bg1 { background-color: $identicon-red; } - &.bg2 { background-color: $identicon-purple; } - &.bg3 { background-color: $identicon-indigo; } - &.bg4 { background-color: $identicon-blue; } - &.bg5 { background-color: $identicon-teal; } - &.bg6 { background-color: $identicon-orange; } - &.bg7 { background-color: $gray-darker; } + @for $i from 1 through length($identicon-backgrounds) { + &.bg#{$i} { + background-color: nth($identicon-backgrounds, $i); + } + } } .avatar-container { @@ -139,41 +192,32 @@ .avatar { border-radius: 0; + border: 0; height: auto; width: 100%; margin: 0; align-self: center; } - &.s40 { min-width: 40px; - min-height: 40px; } + &.s40 { + min-width: 40px; + min-height: 40px; + } - &.s64 { min-width: 64px; - min-height: 64px; } + &.s64 { + min-width: 64px; + min-height: 64px; + } } .rect-avatar { border-radius: $border-radius-small; - &.s16 { border-radius: $border-radius-small; } - &.s18 { border-radius: $border-radius-small; } - &.s19 { border-radius: $border-radius-small; } - &.s20 { border-radius: $border-radius-small; } - &.s24 { border-radius: $border-radius-default; } - &.s26 { border-radius: $border-radius-default; } - &.s32 { border-radius: $border-radius-default; } - &.s36 { border-radius: $border-radius-default; } - &.s40 { border-radius: $border-radius-default; } - &.s46 { border-radius: $border-radius-default; } - &.s48 { border-radius: $border-radius-large; } - &.s60 { border-radius: $border-radius-large; } - &.s64 { border-radius: $border-radius-large; } - &.s70 { border-radius: $border-radius-large; } - &.s90 { border-radius: $border-radius-large; } - &.s96 { border-radius: $border-radius-large; } - &.s100 { border-radius: $border-radius-large; } - &.s110 { border-radius: $border-radius-large; } - &.s140 { border-radius: $border-radius-large; } - &.s160 { border-radius: $border-radius-large; } + + @each $size, $size-config in $avatar-sizes { + &.s#{$size} { + border-radius: map-get($size-config, 'border-radius'); + } + } } .avatar-counter { diff --git a/app/controllers/profiles/notifications_controller.rb b/app/controllers/profiles/notifications_controller.rb index b719b70c56e..617e5bb7cb3 100644 --- a/app/controllers/profiles/notifications_controller.rb +++ b/app/controllers/profiles/notifications_controller.rb @@ -14,9 +14,9 @@ class Profiles::NotificationsController < Profiles::ApplicationController result = Users::UpdateService.new(current_user, user_params.merge(user: current_user)).execute if result[:status] == :success - flash[:notice] = "Notification settings saved" + flash[:notice] = _("Notification settings saved") else - flash[:alert] = "Failed to save new settings" + flash[:alert] = _("Failed to save new settings") end redirect_back_or_default(default: profile_notifications_path) diff --git a/app/views/profiles/chat_names/_chat_name.html.haml b/app/views/profiles/chat_names/_chat_name.html.haml index 9e82e47c1e1..ff67f92ad07 100644 --- a/app/views/profiles/chat_names/_chat_name.html.haml +++ b/app/views/profiles/chat_names/_chat_name.html.haml @@ -21,7 +21,7 @@ - if chat_name.last_used_at = time_ago_with_tooltip(chat_name.last_used_at) - else - Never + = _('Never') %td - = link_to 'Remove', profile_chat_name_path(chat_name), method: :delete, class: 'btn btn-danger float-right', data: { confirm: 'Are you sure you want to revoke this nickname?' } + = link_to _('Remove'), profile_chat_name_path(chat_name), method: :delete, class: 'btn btn-danger float-right', data: { confirm: _('Are you sure you want to revoke this nickname?') } diff --git a/app/views/profiles/chat_names/index.html.haml b/app/views/profiles/chat_names/index.html.haml index 4b6e419af50..0c8098a97d5 100644 --- a/app/views/profiles/chat_names/index.html.haml +++ b/app/views/profiles/chat_names/index.html.haml @@ -1,4 +1,4 @@ -- page_title 'Chat' +- page_title _('Chat') - @content_class = "limit-container-width" unless fluid_layout .row.prepend-top-default @@ -6,7 +6,7 @@ %h4.prepend-top-0 = page_title %p - You can see your Chat accounts. + = _('You can see your chat accounts.') .col-lg-8 %h5 Active chat names (#{@chat_names.size}) @@ -16,15 +16,15 @@ %table.table.chat-names %thead %tr - %th Project - %th Service - %th Team domain - %th Nickname - %th Last used + %th= _('Project') + %th= _('Service') + %th= _('Team domain') + %th= _('Nickname') + %th= _('Last used') %th %tbody = render @chat_names - else .settings-message.text-center - You don't have any active chat names. + = _("You don't have any active chat names.") diff --git a/app/views/profiles/notifications/show.html.haml b/app/views/profiles/notifications/show.html.haml index e616e5546b3..fa35fbd3961 100644 --- a/app/views/profiles/notifications/show.html.haml +++ b/app/views/profiles/notifications/show.html.haml @@ -1,4 +1,4 @@ -- page_title "Notifications" +- page_title _('Notifications') - @content_class = "limit-container-width" unless fluid_layout %div @@ -14,12 +14,12 @@ %h4.prepend-top-0 = page_title %p - You can specify notification level per group or per project. + = _('You can specify notification level per group or per project.') %p - By default, all projects and groups will use the global notifications setting. + = _('By default, all projects and groups will use the global notifications setting.') .col-lg-8 %h5.prepend-top-0 - Global notification settings + = _('Global notification settings') = form_for @user, url: profile_notifications_path, method: :put, html: { class: 'update-notifications prepend-top-default' } do |f| = render_if_exists 'profiles/notifications/email_settings', form: f @@ -35,19 +35,19 @@ = form_for @user, url: profile_notifications_path, method: :put do |f| %label{ for: 'user_notified_of_own_activity' } = f.check_box :notified_of_own_activity - %span Receive notifications about your own activity + %span= _('Receive notifications about your own activity') %hr %h5 - Groups (#{@group_notifications.count}) + = _('Groups (%{count})') % { count: @group_notifications.count } %div %ul.bordered-list - @group_notifications.each do |setting| = render 'group_settings', setting: setting, group: setting.source %h5 - Projects (#{@project_notifications.count}) + = _('Projects (%{count})') % { count: @project_notifications.count } %p.account-well - To specify the notification level per project of a group you belong to, you need to visit project page and change notification level there. + = _('To specify the notification level per project of a group you belong to, you need to visit project page and change notification level there.') .append-bottom-default %ul.bordered-list - @project_notifications.each do |setting| diff --git a/app/views/shared/notifications/_button.html.haml b/app/views/shared/notifications/_button.html.haml index 2ece7b7f701..31cc0c091dd 100644 --- a/app/views/shared/notifications/_button.html.haml +++ b/app/views/shared/notifications/_button.html.haml @@ -15,7 +15,7 @@ = icon('caret-down') .sr-only Toggle dropdown - else - %button.dropdown-new.btn.btn-default.has-tooltip.notifications-btn#notifications-button{ type: "button", title: "Notification setting", class: "#{btn_class}", "aria-label" => "Notification setting: #{notification_title(notification_setting.level)}", data: { container: "body", toggle: "dropdown", target: notifications_menu_identifier("dropdown", notification_setting), flip: "false" } } + %button.dropdown-new.btn.btn-default.has-tooltip.notifications-btn#notifications-button{ type: "button", title: _("Notification setting"), class: "#{btn_class}", "aria-label" => _("Notification setting - %{notification_title}") % { notification_title: notification_title(notification_setting.level) }, data: { container: "body", toggle: "dropdown", target: notifications_menu_identifier("dropdown", notification_setting), flip: "false" } } = icon("bell", class: "js-notification-loading") = notification_title(notification_setting.level) = icon("caret-down") diff --git a/app/views/shared/notifications/_new_button.html.haml b/app/views/shared/notifications/_new_button.html.haml index af8ab992f0e..052e6da5bae 100644 --- a/app/views/shared/notifications/_new_button.html.haml +++ b/app/views/shared/notifications/_new_button.html.haml @@ -16,7 +16,7 @@ = sprite_icon("arrow-down", css_class: "icon mr-0") .sr-only Toggle dropdown - else - %button.dropdown-new.btn.btn-default.has-tooltip.notifications-btn#notifications-button{ type: "button", title: "Notification setting - #{notification_title(notification_setting.level)}", class: "#{btn_class}", "aria-label" => "Notification setting: #{notification_title(notification_setting.level)}", data: { container: "body", placement: 'top', toggle: "dropdown", target: notifications_menu_identifier("dropdown", notification_setting), flip: "false" } } + %button.dropdown-new.btn.btn-default.has-tooltip.notifications-btn#notifications-button{ type: "button", title: _("Notification setting - %{notification_title}") % { notification_title: notification_title(notification_setting.level) }, class: "#{btn_class}", "aria-label" => _("Notification setting - %{notification_title}") % { notification_title: notification_title(notification_setting.level) }, data: { container: "body", placement: 'top', toggle: "dropdown", target: notifications_menu_identifier("dropdown", notification_setting), flip: "false" } } = notification_setting_icon(notification_setting) %span.js-notification-loading.fa.hidden = sprite_icon("arrow-down", css_class: "icon") diff --git a/app/views/shared/notifications/_notification_dropdown.html.haml b/app/views/shared/notifications/_notification_dropdown.html.haml index 85ad74f9a39..a6ef2d51171 100644 --- a/app/views/shared/notifications/_notification_dropdown.html.haml +++ b/app/views/shared/notifications/_notification_dropdown.html.haml @@ -8,5 +8,5 @@ %li.divider %li %a.update-notification{ href: "#", role: "button", class: ("is-active" if notification_setting.custom?), data: { toggle: "modal", target: "#" + notifications_menu_identifier("modal", notification_setting), notification_level: "custom", notification_title: "Custom" } } - %strong.dropdown-menu-inner-title Custom + %strong.dropdown-menu-inner-title= s_('NotificationSetting|Custom') %span.dropdown-menu-inner-content= notification_description("custom") diff --git a/changelogs/unreleased/62408-dropdown-truncate.yml b/changelogs/unreleased/62408-dropdown-truncate.yml new file mode 100644 index 00000000000..7204016efdf --- /dev/null +++ b/changelogs/unreleased/62408-dropdown-truncate.yml @@ -0,0 +1,5 @@ +--- +title: Fix job name in graph dropdown overflowing +merge_request: 28824 +author: +type: fixed diff --git a/changelogs/unreleased/i18n-chat-of-user-profile.yml b/changelogs/unreleased/i18n-chat-of-user-profile.yml new file mode 100644 index 00000000000..663b4ffc1a1 --- /dev/null +++ b/changelogs/unreleased/i18n-chat-of-user-profile.yml @@ -0,0 +1,5 @@ +--- +title: Externalize strings of chat page in user profile +merge_request: 28632 +author: +type: other diff --git a/doc/ci/multi_project_pipelines.md b/doc/ci/multi_project_pipelines.md index 556059c01b6..e9deabf27f8 100644 --- a/doc/ci/multi_project_pipelines.md +++ b/doc/ci/multi_project_pipelines.md @@ -134,6 +134,34 @@ staging: The `ENVIRONMENT` variable will be passed to every job defined in a downstream pipeline. It will be available as an environment variable when GitLab Runner picks a job. +In the following configuration, the `MY_VARIABLE` variable will be passed +downstream, because jobs inherit variables declared in top-level `variables`: + +```yaml +variables: + MY_VARIABLE: my-value + +my-pipeline: + variables: + ENVIRONMENT: something + trigger: my/project +``` + +You might want to pass some information about the upstream pipeline using, for +example, predefined variables. In order to do that, you can use interpolation +to pass any variable. For example: + +```yaml +my-pipeline: + variables: + UPSTREAM_BRANCH: $CI_COMMIT_REF_NAME + trigger: my/project +``` + +In this scenario, the `UPSTREAM_BRANCH` variable with a value related to the +upstream pipeline will be passed to a `downstream` job, and will be available +within the context of all downstream builds. + ### Limitations Because bridge jobs are a little different to regular jobs, it is not diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 72bff621059..9e7ff8d1847 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -1126,6 +1126,9 @@ msgstr "" msgid "Are you sure you want to reset the health check token?" msgstr "" +msgid "Are you sure you want to revoke this nickname?" +msgstr "" + msgid "Are you sure you want to stop this environment?" msgstr "" @@ -1629,6 +1632,9 @@ msgstr "" msgid "By default GitLab sends emails in HTML and plain text formats so mail clients can choose what format to use. Disable this option if you only want to send emails in plain text format." msgstr "" +msgid "By default, all projects and groups will use the global notifications setting." +msgstr "" + msgid "ByAuthor|by" msgstr "" @@ -4653,6 +4659,9 @@ msgstr "" msgid "Given access %{time_ago}" msgstr "" +msgid "Global notification settings" +msgstr "" + msgid "Go Back" msgstr "" @@ -4809,6 +4818,9 @@ msgstr "" msgid "Groups" msgstr "" +msgid "Groups (%{count})" +msgstr "" + msgid "Groups can also be nested by creating %{subgroup_docs_link_start}subgroups%{subgroup_docs_link_end}." msgstr "" @@ -5641,6 +5653,9 @@ msgstr "" msgid "Last updated" msgstr "" +msgid "Last used" +msgstr "" + msgid "LastPushEvent|You pushed to" msgstr "" @@ -6377,6 +6392,9 @@ msgstr "" msgid "Next" msgstr "" +msgid "Nickname" +msgstr "" + msgid "No" msgstr "" @@ -6557,6 +6575,9 @@ msgstr "" msgid "Notification setting - %{notification_title}" msgstr "" +msgid "Notification settings saved" +msgstr "" + msgid "NotificationEvent|Close issue" msgstr "" @@ -6608,6 +6629,9 @@ msgstr "" msgid "NotificationLevel|Watch" msgstr "" +msgid "NotificationSetting|Custom" +msgstr "" + msgid "Notifications" msgstr "" @@ -7771,6 +7795,9 @@ msgstr "" msgid "Projects" msgstr "" +msgid "Projects (%{count})" +msgstr "" + msgid "Projects Successfully Retrieved" msgstr "" @@ -7999,6 +8026,9 @@ msgstr "" msgid "Real-time features" msgstr "" +msgid "Receive notifications about your own activity" +msgstr "" + msgid "Recent Project Activity" msgstr "" @@ -8714,6 +8744,9 @@ msgstr "" msgid "Serverless|There is currently no function data available from Knative. This could be for a variety of reasons including:" msgstr "" +msgid "Service" +msgstr "" + msgid "Service Templates" msgstr "" @@ -9499,6 +9532,9 @@ msgstr "" msgid "Team" msgstr "" +msgid "Team domain" +msgstr "" + msgid "Template" msgstr "" @@ -10324,6 +10360,9 @@ msgstr "" msgid "To preserve performance only <strong>%{display_size} of %{real_size}</strong> files are displayed." msgstr "" +msgid "To specify the notification level per project of a group you belong to, you need to visit project page and change notification level there." +msgstr "" + msgid "To start serving your jobs you can add Runners to your group" msgstr "" @@ -11310,9 +11349,15 @@ msgstr "" msgid "You can resolve the merge conflict using either the Interactive mode, by choosing %{use_ours} or %{use_theirs} buttons, or by editing the files directly. Commit these changes into %{branch_name}" msgstr "" +msgid "You can see your chat accounts." +msgstr "" + msgid "You can set up jobs to only use Runners with specific tags. Separate tags with commas." msgstr "" +msgid "You can specify notification level per group or per project." +msgstr "" + msgid "You can test your .gitlab-ci.yml in %{linkStart}CI Lint%{linkEnd}." msgstr "" @@ -11343,6 +11388,9 @@ msgstr "" msgid "You do not have permission to leave this %{namespaceType}." msgstr "" +msgid "You don't have any active chat names." +msgstr "" + msgid "You don't have any applications" msgstr "" diff --git a/qa/README.md b/qa/README.md index 002ad4c65f5..f75205133e6 100644 --- a/qa/README.md +++ b/qa/README.md @@ -49,10 +49,10 @@ will need to [modify your GDK setup](https://gitlab.com/gitlab-org/gitlab-qa/blo ### Writing tests -- [Writing tests from scratch tutorial](docs/WRITING_TESTS_FROM_SCRATCH.md) - - [Best practices](docs/BEST_PRACTICES.md) +- [Writing tests from scratch tutorial](docs/writing_tests_from_scratch.md) + - [Best practices](docs/best_practices.md) - [Using page objects](qa/page/README.md) - - [Guidelines](docs/GUIDELINES.md) + - [Guidelines](docs/guidelines.md) ### Running specific tests diff --git a/qa/docs/GUIDELINES.md b/qa/docs/GUIDELINES.md deleted file mode 100644 index 9db52cd07e6..00000000000 --- a/qa/docs/GUIDELINES.md +++ /dev/null @@ -1,46 +0,0 @@ -# Style guide for writing GUI tests - -This document describes the conventions used at GitLab for writing GUI tests using the GitLab QA project. - -## `click_` versus `go_to_` - -### When to use `click_`? - -When clicking in a single link to navigate, use `click_`. - -E.g.: - -```ruby -def click_ci_cd_pipelines - within_sidebar do - click_element :link_pipelines - end -end -``` - -From a testing perspective, if we want to check that clicking a link, or a button (a single interaction) is working as intended, we would want the test to read as: - -- Click a certain element -- Verify the action took place - -### When to use `go_to_`? - -When interacting with multiple elements to go to a page, use `go_to_`. - -E.g.: - -```ruby -def go_to_operations_environments - hover_operations do - within_submenu do - click_element(:operations_environments_link) - end - end -end -``` - -`go_to_` fits the definition of interacting with multiple elements very well given it's more of a meta-navigation action that includes multiple interactions. - -Notice that in the above example, before clicking the `:operations_environments_link`, another element is hovered over. - -> We can create these methods as helpers to abstract multi-step navigation. diff --git a/qa/docs/BEST_PRACTICES.md b/qa/docs/best_practices.md index 3a2640607e4..d6e5350b0c8 100644 --- a/qa/docs/BEST_PRACTICES.md +++ b/qa/docs/best_practices.md @@ -35,4 +35,4 @@ Finally, interacting with the application only by its GUI generates a higher rat - Building state through the GUI is time consuming and it's not sustainable as the test suite grows. - When depending only on the GUI to create the application's state and tests fail due to front-end issues, we can't rely on the test failures rate, and we generates a higher rate of test flakiness. -Now that we are aware of all of it, [let's go create some tests](./WRITING_TESTS_FROM_SCRATCH.md). +Now that we are aware of all of it, [let's go create some tests](writing_tests_from_scratch.md). diff --git a/qa/docs/guidelines.md b/qa/docs/guidelines.md new file mode 100644 index 00000000000..cd4b939fd71 --- /dev/null +++ b/qa/docs/guidelines.md @@ -0,0 +1,97 @@ +# Style guide for writing E2E tests + +This document describes the conventions used at GitLab for writing E2E tests using the GitLab QA project. + +## `click_` versus `go_to_` + +### When to use `click_`? + +When clicking in a single link to navigate, use `click_`. + +E.g.: + +```ruby +def click_ci_cd_pipelines + within_sidebar do + click_element :link_pipelines + end +end +``` + +From a testing perspective, if we want to check that clicking a link, or a button (a single interaction) is working as intended, we would want the test to read as: + +- Click a certain element +- Verify the action took place + +### When to use `go_to_`? + +When interacting with multiple elements to go to a page, use `go_to_`. + +E.g.: + +```ruby +def go_to_operations_environments + hover_operations do + within_submenu do + click_element(:operations_environments_link) + end + end +end +``` + +`go_to_` fits the definition of interacting with multiple elements very well given it's more of a meta-navigation action that includes multiple interactions. + +Notice that in the above example, before clicking the `:operations_environments_link`, another element is hovered over. + +> We can create these methods as helpers to abstract multi-step navigation. + +### Element Naming Convention + +When adding new elements to a page, it's important that we have a uniform element naming convention. + +We follow a simple formula roughly based on hungarian notation. + +*Formula*: `element :<descriptor>_<type>` + +- `descriptor`: The natural-language description of what the element is. On the login page, this could be `username`, or `password`. +- `type`: A physical control on the page that can be seen by a user. + - `_button` + - `_link` + - `_tab` + - `_dropdown` + - `_field` + - `_checkbox` + - `_radio` + - `_content` + +*Note: This list is a work in progress. This list will eventually be the end-all enumeration of all available types. + I.e., any element that does not end with something in this list is bad form.* + +#### Examples + +**Good** +```ruby +view '...' do + element :edit_button + element :notes_tab + element :squash_checkbox + element :username_field + element :issue_title_content +end +``` + +**Bad** +```ruby +view '...' do + # `_confirmation` should be `_field`. what sort of confirmation? a checkbox confirmation? no real way to disambiguate. + # an appropriate replacement would be `element :password_confirmation_field` + element :password_confirmation + + # `clone_options` is too vague. If it's a dropdown menu, it should be `clone_dropdown`. + # If it's a checkbox, it should be `clone_checkbox` + element :clone_options + + # how is this url being displayed? is it a textbox? a simple span? + element :ssh_clone_url +end +``` diff --git a/qa/docs/WRITING_TESTS_FROM_SCRATCH.md b/qa/docs/writing_tests_from_scratch.md index 309fcc4064c..65e7a78a8b5 100644 --- a/qa/docs/WRITING_TESTS_FROM_SCRATCH.md +++ b/qa/docs/writing_tests_from_scratch.md @@ -8,7 +8,7 @@ In this tutorial, you will find different examples, and the steps involved, in t It's important to understand that end-to-end tests of isolated features, such as the ones described in the above note, doesn't mean that everything needs to happen through the GUI. -If you don't exactly understand what we mean by **not everything needs to happen through the GUI,** please make sure you've read the [best practices](./BEST_PRACTICES.md) before moving on. +If you don't exactly understand what we mean by **not everything needs to happen through the GUI,** please make sure you've read the [best practices](best_practices.md) before moving on. ## This document covers the following items: @@ -367,7 +367,7 @@ With that in mind, resources can be a project, an epic, an issue, a label, a com As you saw in the tests' pre-conditions and the optimization sections, we're already creating some of these resources, and we are doing that by calling the `fabricate_via_api!` method. -> We could be using the `fabricate!` method instead, which would use the `fabricate_via_api!` method if it exists, and fallback to GUI fabrication otherwise, but we recommend being explicit to make it clear what the test does. Also, we recommend fabricating resources via API since this makes tests faster and more reliable, unless the test is focusing on the GUI itself, or there's no GUI coverage for that specific part in any other test. +> We could be using the `fabricate!` method instead, which would use the `fabricate_via_api!` method if it exists, and fallback to GUI fabrication otherwise, but we recommend being explicit to make it clear what the test does. Also, we always recommend fabricating resources via API since this makes tests faster and more reliable. For our test suite example, the [project resource](https://gitlab.com/gitlab-org/gitlab-ee/blob/d3584e80b4236acdf393d815d604801573af72cc/qa/qa/resource/project.rb#L55) already had a `fabricate_via_api!` method available, while other resources don't have it, so we will have to create them, like for the issue and label resources. Also, we will have to make a small change in the project resource to expose its `id` attribute so that we can refer to it when fabricating the issue. @@ -130,6 +130,7 @@ module QA autoload :View, 'qa/page/view' autoload :Element, 'qa/page/element' autoload :Validator, 'qa/page/validator' + autoload :Validatable, 'qa/page/validatable' module Main autoload :Login, 'qa/page/main/login' diff --git a/qa/qa/ce/strategy.rb b/qa/qa/ce/strategy.rb index d7748a976f0..7e2d02424fe 100644 --- a/qa/qa/ce/strategy.rb +++ b/qa/qa/ce/strategy.rb @@ -13,7 +13,6 @@ module QA # The login page could take some time to load the first time it is visited. # We visit the login page and wait for it to properly load only once before the tests. QA::Runtime::Browser.visit(:gitlab, QA::Page::Main::Login) - QA::Page::Main::Login.perform(&:assert_page_loaded) end end end diff --git a/qa/qa/page/base.rb b/qa/qa/page/base.rb index c395e5f6011..389f4e0032e 100644 --- a/qa/qa/page/base.rb +++ b/qa/qa/page/base.rb @@ -8,6 +8,7 @@ module QA prepend Support::Page::Logging if Runtime::Env.debug? include Capybara::DSL include Scenario::Actable + extend Validatable extend SingleForwardable ElementNotFound = Class.new(RuntimeError) @@ -93,8 +94,10 @@ module QA find_element(name).set(false) end - def click_element(name) + # replace with (..., page = self.class) + def click_element(name, page = nil) find_element(name).click + page.validate_elements_present! if page end def fill_element(name, content) diff --git a/qa/qa/page/element.rb b/qa/qa/page/element.rb index d92e71467fe..7a01320901d 100644 --- a/qa/qa/page/element.rb +++ b/qa/qa/page/element.rb @@ -1,28 +1,41 @@ # frozen_string_literal: true +require 'active_support/core_ext/array/extract_options' + module QA module Page class Element - attr_reader :name + attr_reader :name, :attributes - def initialize(name, pattern = nil) + def initialize(name, *options) @name = name - @pattern = pattern || selector + @attributes = options.extract_options! + @attributes[:pattern] ||= selector + + options.each do |option| + if option.is_a?(String) || option.is_a?(Regexp) + @attributes[:pattern] = option + end + end end def selector "qa-#{@name.to_s.tr('_', '-')}" end + def required? + !!@attributes[:required] + end + def selector_css ".#{selector}" end def expression - if @pattern.is_a?(String) - @_regexp ||= Regexp.new(Regexp.escape(@pattern)) + if @attributes[:pattern].is_a?(String) + @_regexp ||= Regexp.new(Regexp.escape(@attributes[:pattern])) else - @pattern + @attributes[:pattern] end end diff --git a/qa/qa/page/main/login.rb b/qa/qa/page/main/login.rb index 99b3d1b83d3..8970eeb6678 100644 --- a/qa/qa/page/main/login.rb +++ b/qa/qa/page/main/login.rb @@ -39,19 +39,7 @@ module QA end view 'app/views/layouts/devise.html.haml' do - element :login_page - end - - def assert_page_loaded - unless page_loaded? - raise QA::Runtime::Browser::NotRespondingError, "Login page did not load at #{QA::Page::Main::Login.perform(&:current_url)}" - end - end - - def page_loaded? - wait(max: 60) do - has_element?(:login_page) - end + element :login_page, required: true end def sign_in_using_credentials(user = nil) @@ -159,7 +147,7 @@ module QA fill_element :login_field, user.username fill_element :password_field, user.password - click_element :sign_in_button + click_element :sign_in_button, Page::Main::Menu end def set_initial_password_if_present diff --git a/qa/qa/page/main/menu.rb b/qa/qa/page/main/menu.rb index e98d531c86e..5eb24d2d2ba 100644 --- a/qa/qa/page/main/menu.rb +++ b/qa/qa/page/main/menu.rb @@ -10,15 +10,15 @@ module QA end view 'app/views/layouts/header/_default.html.haml' do - element :navbar - element :user_avatar + element :navbar, required: true + element :user_avatar, required: true element :user_menu, '.dropdown-menu' # rubocop:disable QA/ElementWithPattern end view 'app/views/layouts/nav/_dashboard.html.haml' do element :admin_area_link - element :projects_dropdown - element :groups_dropdown + element :projects_dropdown, required: true + element :groups_dropdown, required: true element :snippets_link end diff --git a/qa/qa/page/validatable.rb b/qa/qa/page/validatable.rb new file mode 100644 index 00000000000..8467d261285 --- /dev/null +++ b/qa/qa/page/validatable.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +module QA + module Page + module Validatable + PageValidationError = Class.new(StandardError) + + def validate_elements_present! + base_page = self.new + + elements.each do |element| + next unless element.required? + + # TODO: this wait needs to be replaced by the wait class + unless base_page.has_element?(element.name, wait: 60) + raise Validatable::PageValidationError, "#{element.name} did not appear on #{self.name} as expected" + end + end + end + end + end +end diff --git a/qa/qa/page/view.rb b/qa/qa/page/view.rb index 96f3917a8ab..613059b2d32 100644 --- a/qa/qa/page/view.rb +++ b/qa/qa/page/view.rb @@ -50,8 +50,8 @@ module QA @elements = [] end - def element(name, pattern = nil) - @elements.push(Page::Element.new(name, pattern)) + def element(name, *args) + @elements.push(Page::Element.new(name, *args)) end end end diff --git a/qa/qa/runtime/browser.rb b/qa/qa/runtime/browser.rb index 174a52bd376..3bf4b3bbbfb 100644 --- a/qa/qa/runtime/browser.rb +++ b/qa/qa/runtime/browser.rb @@ -33,6 +33,7 @@ module QA def self.visit(address, page = nil, &block) new.visit(address, page, &block) + page.validate_elements_present! end def self.configure! diff --git a/qa/qa/specs/features/browser_ui/1_manage/login/log_into_mattermost_via_gitlab_spec.rb b/qa/qa/specs/features/browser_ui/1_manage/login/log_into_mattermost_via_gitlab_spec.rb index b1d641b507f..67610b62ed7 100644 --- a/qa/qa/specs/features/browser_ui/1_manage/login/log_into_mattermost_via_gitlab_spec.rb +++ b/qa/qa/specs/features/browser_ui/1_manage/login/log_into_mattermost_via_gitlab_spec.rb @@ -4,16 +4,14 @@ module QA context 'Manage', :orchestrated, :mattermost do describe 'Mattermost login' do it 'user logs into Mattermost using GitLab OAuth' do - Runtime::Browser.visit(:gitlab, Page::Main::Login) do - Page::Main::Login.act { sign_in_using_credentials } + Runtime::Browser.visit(:gitlab, Page::Main::Login) + Page::Main::Login.perform(&:sign_in_using_credentials) - Runtime::Browser.visit(:mattermost, Page::Mattermost::Login) do - Page::Mattermost::Login.act { sign_in_using_oauth } + Runtime::Browser.visit(:mattermost, Page::Mattermost::Login) + Page::Mattermost::Login.perform(&:sign_in_using_oauth) - Page::Mattermost::Main.perform do |page| - expect(page).to have_content(/(Welcome to: Mattermost|Logout GitLab Mattermost)/) - end - end + Page::Mattermost::Main.perform do |page| + expect(page).to have_content(/(Welcome to: Mattermost|Logout GitLab Mattermost)/) end end end diff --git a/qa/qa/support/page/logging.rb b/qa/qa/support/page/logging.rb index ff505fdbddd..3fe567d7757 100644 --- a/qa/qa/support/page/logging.rb +++ b/qa/qa/support/page/logging.rb @@ -56,8 +56,11 @@ module QA elements end - def click_element(name) - log("clicking :#{name}") + def click_element(name, page = nil) + msg = ["clicking :#{name}"] + msg << ", expecting to be at #{page.class}" if page + + log(msg.compact.join(' ')) super end diff --git a/qa/spec/page/element_spec.rb b/qa/spec/page/element_spec.rb index d5d6dff69da..f746fe06e40 100644 --- a/qa/spec/page/element_spec.rb +++ b/qa/spec/page/element_spec.rb @@ -50,4 +50,60 @@ describe QA::Page::Element do expect(subject.matches?('some_name selector')).to be false end end + + describe 'attributes' do + context 'element with no args' do + subject { described_class.new(:something) } + + it 'defaults pattern to #selector' do + expect(subject.attributes[:pattern]).to eq 'qa-something' + expect(subject.attributes[:pattern]).to eq subject.selector + end + + it 'is not required by default' do + expect(subject.required?).to be false + end + end + + context 'element with a pattern' do + subject { described_class.new(:something, /link_to 'something'/) } + + it 'has an attribute[pattern] of the pattern' do + expect(subject.attributes[:pattern]).to eq /link_to 'something'/ + end + + it 'is not required by default' do + expect(subject.required?).to be false + end + end + + context 'element with requirement; no pattern' do + subject { described_class.new(:something, required: true) } + + it 'has an attribute[pattern] of the selector' do + expect(subject.attributes[:pattern]).to eq 'qa-something' + expect(subject.attributes[:pattern]).to eq subject.selector + end + + it 'is required' do + expect(subject.required?).to be true + end + end + + context 'element with requirement and pattern' do + subject { described_class.new(:something, /link_to 'something_else_entirely'/, required: true) } + + it 'has an attribute[pattern] of the passed pattern' do + expect(subject.attributes[:pattern]).to eq /link_to 'something_else_entirely'/ + end + + it 'is required' do + expect(subject.required?).to be true + end + + it 'has a selector of the name' do + expect(subject.selector).to eq 'qa-something' + end + end + end end diff --git a/rubocop/cop/qa/element_with_pattern.rb b/rubocop/cop/qa/element_with_pattern.rb index 9d80946f1ba..d14eeaaeaf3 100644 --- a/rubocop/cop/qa/element_with_pattern.rb +++ b/rubocop/cop/qa/element_with_pattern.rb @@ -1,18 +1,21 @@ +# frozen_string_literal: true + require_relative '../../qa_helpers' module RuboCop module Cop module QA - # This cop checks for the usage of factories in migration specs + # This cop checks for the usage of patterns in QA elements # # @example # # # bad - # let(:user) { create(:user) } + # element :some_element, "link_to 'something'" + # element :some_element, /link_to 'something'/ # # # good - # let(:users) { table(:users) } - # let(:user) { users.create!(name: 'User 1', username: 'user1') } + # element :some_element + # element :some_element, required: true class ElementWithPattern < RuboCop::Cop::Cop include QAHelpers @@ -22,10 +25,13 @@ module RuboCop return unless in_qa_file?(node) return unless method_name(node).to_s == 'element' - element_name, pattern = node.arguments - return unless pattern + element_name, *args = node.arguments + + return if args.first.nil? - add_offense(node, location: pattern.source_range, message: MESSAGE % "qa-#{element_name.value.to_s.tr('_', '-')}") + args.first.each_node(:str) do |arg| + add_offense(arg, message: MESSAGE % "qa-#{element_name.value.to_s.tr('_', '-')}") + end end private diff --git a/spec/rubocop/cop/qa/element_with_pattern_spec.rb b/spec/rubocop/cop/qa/element_with_pattern_spec.rb index c5beb40f9fd..ef20d9a1f26 100644 --- a/spec/rubocop/cop/qa/element_with_pattern_spec.rb +++ b/spec/rubocop/cop/qa/element_with_pattern_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' require 'rubocop' @@ -23,7 +25,7 @@ describe RuboCop::Cop::QA::ElementWithPattern do element :groups_filter, 'search_field_tag :filter' ^^^^^^^^^^^^^^^^^^^^^^^^^^ Don't use a pattern for element, create a corresponding `qa-groups-filter` instead. element :groups_filter_placeholder, /Search by name/ - ^^^^^^^^^^^^^^^^ Don't use a pattern for element, create a corresponding `qa-groups-filter-placeholder` instead. + ^^^^^^^^^^^^^^ Don't use a pattern for element, create a corresponding `qa-groups-filter-placeholder` instead. end RUBY end @@ -35,6 +37,13 @@ describe RuboCop::Cop::QA::ElementWithPattern do element :groups_filter_placeholder end RUBY + + expect_no_offenses(<<-RUBY) + view 'app/views/shared/groups/_search_form.html.haml' do + element :groups_filter, required: true + element :groups_filter_placeholder, required: false + end + RUBY end end |