From e8d24350247aae6cf4eca12c53b24caf09d7b298 Mon Sep 17 00:00:00 2001 From: Miguel Rincon <mrincon@gitlab.com> Date: Wed, 26 Apr 2023 09:29:20 +0000 Subject: [PATCH] Refactor runner name with id + sha to method --- .rubocop_todo/layout/space_inside_parens.yml | 2 - .../rspec/factory_bot/avoid_create.yml | 1 - .../rspec/missing_feature_category.yml | 2 - .../style/percent_literal_delimiters.yml | 1 - app/helpers/ci/runners_helper.rb | 4 ++ app/views/admin/runners/edit.html.haml | 2 +- app/views/admin/runners/register.html.haml | 4 +- app/views/admin/runners/show.html.haml | 6 +-- app/views/groups/runners/edit.html.haml | 2 +- app/views/groups/runners/register.html.haml | 2 +- app/views/groups/runners/show.html.haml | 6 +-- app/views/projects/runners/edit.html.haml | 5 ++- app/views/projects/runners/register.html.haml | 3 +- app/views/projects/runners/show.html.haml | 3 ++ .../shared/runners/_runner_details.html.haml | 3 -- spec/helpers/ci/runners_helper_spec.rb | 12 +++++- spec/support/rspec_order_todo.yml | 2 - .../runners/_runner_details.html.haml_spec.rb | 38 +++++++------------ 18 files changed, 46 insertions(+), 52 deletions(-) diff --git a/.rubocop_todo/layout/space_inside_parens.yml b/.rubocop_todo/layout/space_inside_parens.yml index 53e00f78f29db..5234f4152c214 100644 --- a/.rubocop_todo/layout/space_inside_parens.yml +++ b/.rubocop_todo/layout/space_inside_parens.yml @@ -97,7 +97,6 @@ Layout/SpaceInsideParens: - 'spec/helpers/application_helper_spec.rb' - 'spec/helpers/badges_helper_spec.rb' - 'spec/helpers/ci/builds_helper_spec.rb' - - 'spec/helpers/ci/runners_helper_spec.rb' - 'spec/helpers/dev_ops_report_helper_spec.rb' - 'spec/helpers/git_helper_spec.rb' - 'spec/helpers/gitlab_routing_helper_spec.rb' @@ -223,7 +222,6 @@ Layout/SpaceInsideParens: - 'spec/views/dashboard/projects/_blank_state_welcome.html.haml_spec.rb' - 'spec/views/profiles/keys/_form.html.haml_spec.rb' - 'spec/views/search/_results.html.haml_spec.rb' - - 'spec/views/shared/runners/_runner_details.html.haml_spec.rb' - 'spec/workers/concerns/gitlab/github_import/object_importer_spec.rb' - 'spec/workers/gitlab/jira_import/stage/import_labels_worker_spec.rb' - 'spec/workers/pipeline_schedule_worker_spec.rb' diff --git a/.rubocop_todo/rspec/factory_bot/avoid_create.yml b/.rubocop_todo/rspec/factory_bot/avoid_create.yml index 70046f783da79..3a990dcd12de0 100644 --- a/.rubocop_todo/rspec/factory_bot/avoid_create.yml +++ b/.rubocop_todo/rspec/factory_bot/avoid_create.yml @@ -619,7 +619,6 @@ RSpec/FactoryBot/AvoidCreate: - 'spec/views/shared/projects/_inactive_project_deletion_alert.html.haml_spec.rb' - 'spec/views/shared/projects/_list.html.haml_spec.rb' - 'spec/views/shared/projects/_project.html.haml_spec.rb' - - 'spec/views/shared/runners/_runner_details.html.haml_spec.rb' - 'spec/views/shared/snippets/_snippet.html.haml_spec.rb' - 'spec/views/shared/web_hooks/_web_hook_disabled_alert.html.haml_spec.rb' - 'spec/views/shared/wikis/_sidebar.html.haml_spec.rb' diff --git a/.rubocop_todo/rspec/missing_feature_category.yml b/.rubocop_todo/rspec/missing_feature_category.yml index 2941f2999d314..643d8af7ff1c0 100644 --- a/.rubocop_todo/rspec/missing_feature_category.yml +++ b/.rubocop_todo/rspec/missing_feature_category.yml @@ -2639,7 +2639,6 @@ RSpec/MissingFeatureCategory: - 'spec/helpers/ci/jobs_helper_spec.rb' - 'spec/helpers/ci/pipeline_editor_helper_spec.rb' - 'spec/helpers/ci/pipelines_helper_spec.rb' - - 'spec/helpers/ci/runners_helper_spec.rb' - 'spec/helpers/ci/secure_files_helper_spec.rb' - 'spec/helpers/ci/status_helper_spec.rb' - 'spec/helpers/ci/triggers_helper_spec.rb' @@ -6065,7 +6064,6 @@ RSpec/MissingFeatureCategory: - 'spec/views/shared/projects/_inactive_project_deletion_alert.html.haml_spec.rb' - 'spec/views/shared/projects/_list.html.haml_spec.rb' - 'spec/views/shared/projects/_project.html.haml_spec.rb' - - 'spec/views/shared/runners/_runner_details.html.haml_spec.rb' - 'spec/views/shared/snippets/_snippet.html.haml_spec.rb' - 'spec/views/shared/ssh_keys/_key_delete.html.haml_spec.rb' - 'spec/views/shared/web_hooks/_web_hook_disabled_alert.html.haml_spec.rb' diff --git a/.rubocop_todo/style/percent_literal_delimiters.yml b/.rubocop_todo/style/percent_literal_delimiters.yml index 7cb9379cec64b..0edb631461b3f 100644 --- a/.rubocop_todo/style/percent_literal_delimiters.yml +++ b/.rubocop_todo/style/percent_literal_delimiters.yml @@ -1109,7 +1109,6 @@ Style/PercentLiteralDelimiters: - 'spec/views/layouts/_head.html.haml_spec.rb' - 'spec/views/layouts/nav/sidebar/_project.html.haml_spec.rb' - 'spec/views/projects/commit/branches.html.haml_spec.rb' - - 'spec/views/shared/runners/_runner_details.html.haml_spec.rb' - 'spec/workers/concerns/worker_context_spec.rb' - 'spec/workers/container_registry/migration/enqueuer_worker_spec.rb' - 'spec/workers/gitlab/github_import/advance_stage_worker_spec.rb' diff --git a/app/helpers/ci/runners_helper.rb b/app/helpers/ci/runners_helper.rb index 41ef0bd20a83b..5012ac29816c0 100644 --- a/app/helpers/ci/runners_helper.rb +++ b/app/helpers/ci/runners_helper.rb @@ -35,6 +35,10 @@ def runner_status_icon(runner, size: 16, icon_class: '') end end + def runner_short_name(runner) + "##{runner.id} (#{runner.short_sha})" + end + def runner_link(runner) display_name = truncate(runner.display_name, length: 15) id = "\##{runner.id}" diff --git a/app/views/admin/runners/edit.html.haml b/app/views/admin/runners/edit.html.haml index e586a7a965e0b..05f0c95710050 100644 --- a/app/views/admin/runners/edit.html.haml +++ b/app/views/admin/runners/edit.html.haml @@ -1,4 +1,4 @@ -- runner_name = "##{@runner.id} (#{@runner.short_sha})" +- runner_name = runner_short_name(@runner) - breadcrumb_title _('Edit') - page_title _('Edit'), runner_name - add_to_breadcrumbs _('Runners'), admin_runners_path diff --git a/app/views/admin/runners/register.html.haml b/app/views/admin/runners/register.html.haml index 662bb9ea00e15..7aa58f9078e58 100644 --- a/app/views/admin/runners/register.html.haml +++ b/app/views/admin/runners/register.html.haml @@ -1,6 +1,6 @@ -- runner_name = "##{@runner.id} (#{@runner.short_sha})" +- runner_name = runner_short_name(@runner) - breadcrumb_title s_('Runners|Register') -- page_title s_('Runners|Register'), "##{@runner.id} (#{@runner.short_sha})" +- page_title s_('Runners|Register'), runner_name - add_to_breadcrumbs _('Runners'), admin_runners_path - add_to_breadcrumbs runner_name, register_admin_runner_path(@runner) diff --git a/app/views/admin/runners/show.html.haml b/app/views/admin/runners/show.html.haml index a9dbcf4a6a541..3942c427cc401 100644 --- a/app/views/admin/runners/show.html.haml +++ b/app/views/admin/runners/show.html.haml @@ -1,9 +1,9 @@ - add_page_specific_style 'page_bundles/ci_status' - add_page_specific_style 'page_bundles/runner_details' -- title = "##{@runner.id} (#{@runner.short_sha})" -- breadcrumb_title title -- page_title title +- runner_name = runner_short_name(@runner) +- breadcrumb_title runner_name +- page_title runner_name - add_to_breadcrumbs _('Runners'), admin_runners_path #js-admin-runner-show{ data: {runner_id: @runner.id, runners_path: admin_runners_path} } diff --git a/app/views/groups/runners/edit.html.haml b/app/views/groups/runners/edit.html.haml index 4bd550eaa473c..93f2f395df61c 100644 --- a/app/views/groups/runners/edit.html.haml +++ b/app/views/groups/runners/edit.html.haml @@ -1,4 +1,4 @@ -- runner_name = "##{@runner.id} (#{@runner.short_sha})" +- runner_name = runner_short_name(@runner) - breadcrumb_title _('Edit') - page_title _('Edit'), runner_name - add_to_breadcrumbs _('Runners'), group_runners_path(@group) diff --git a/app/views/groups/runners/register.html.haml b/app/views/groups/runners/register.html.haml index 7e12aa3ab3fbc..a5296c38618c2 100644 --- a/app/views/groups/runners/register.html.haml +++ b/app/views/groups/runners/register.html.haml @@ -1,4 +1,4 @@ -- runner_name = "##{@runner.id} (#{@runner.short_sha})" +- runner_name = runner_short_name(@runner) - breadcrumb_title s_('Runners|Register') - page_title s_('Runners|Register'), runner_name - add_to_breadcrumbs _('Runners'), group_runners_path(@group) diff --git a/app/views/groups/runners/show.html.haml b/app/views/groups/runners/show.html.haml index 43673d5447829..53ca704ecb764 100644 --- a/app/views/groups/runners/show.html.haml +++ b/app/views/groups/runners/show.html.haml @@ -1,9 +1,9 @@ - add_page_specific_style 'page_bundles/ci_status' - add_page_specific_style 'page_bundles/runner_details' -- title = "##{@runner.id} (#{@runner.short_sha})" -- breadcrumb_title title -- page_title title +- runner_name = runner_short_name(@runner) +- breadcrumb_title runner_name +- page_title runner_name - add_to_breadcrumbs _('Runners'), group_runners_path(@group) #js-group-runner-show{ data: {runner_id: @runner.id, runners_path: group_runners_path(@group), edit_group_runner_path: edit_group_runner_path(@group, @runner)} } diff --git a/app/views/projects/runners/edit.html.haml b/app/views/projects/runners/edit.html.haml index 50c46402b53f3..58955663bfa4a 100644 --- a/app/views/projects/runners/edit.html.haml +++ b/app/views/projects/runners/edit.html.haml @@ -1,7 +1,8 @@ +- runner_name = runner_short_name(@runner) - breadcrumb_title _('Edit') -- page_title _('Edit'), "##{@runner.id} (#{@runner.short_sha})" +- page_title _('Edit'), runner_name - add_to_breadcrumbs _('CI/CD Settings'), project_settings_ci_cd_path(@project) -- add_to_breadcrumbs "#{@runner.short_sha}", project_runner_path(@project, @runner) +- add_to_breadcrumbs runner_name, project_runner_path(@project, @runner) %h1.page-title.gl-font-size-h-display = s_('Runners|Runner #%{runner_id}') % { runner_id: @runner.id } diff --git a/app/views/projects/runners/register.html.haml b/app/views/projects/runners/register.html.haml index 561a919323da8..a8ce08f051f84 100644 --- a/app/views/projects/runners/register.html.haml +++ b/app/views/projects/runners/register.html.haml @@ -1,5 +1,4 @@ -- runner_name = "##{@runner.id} (#{@runner.short_sha})" - +- runner_name = runner_short_name(@runner) - add_to_breadcrumbs _('CI/CD Settings'), project_settings_ci_cd_path(@project) - breadcrumb_title s_('Runners|Register runner') - page_title s_('Runners|Register'), runner_name diff --git a/app/views/projects/runners/show.html.haml b/app/views/projects/runners/show.html.haml index cb7984729c897..60f403b101579 100644 --- a/app/views/projects/runners/show.html.haml +++ b/app/views/projects/runners/show.html.haml @@ -1,3 +1,6 @@ +- runner_name = runner_short_name(@runner) +- breadcrumb_title runner_name +- page_title runner_name - add_to_breadcrumbs _('CI/CD Settings'), project_settings_ci_cd_path(@project) = render 'shared/runners/runner_details', runner: @runner diff --git a/app/views/shared/runners/_runner_details.html.haml b/app/views/shared/runners/_runner_details.html.haml index 64ee4b38a73b4..686cd1a081b55 100644 --- a/app/views/shared/runners/_runner_details.html.haml +++ b/app/views/shared/runners/_runner_details.html.haml @@ -1,6 +1,3 @@ -- breadcrumb_title runner.short_sha -- page_title "##{runner.id} (#{runner.short_sha})" - %h1.page-title.gl-font-size-h-display = s_('Runners|Runner #%{runner_id}') % { runner_id: runner.id } = render 'shared/runners/runner_type_badge', runner: runner diff --git a/spec/helpers/ci/runners_helper_spec.rb b/spec/helpers/ci/runners_helper_spec.rb index 6d14abd6574be..a01002a2a1ed5 100644 --- a/spec/helpers/ci/runners_helper_spec.rb +++ b/spec/helpers/ci/runners_helper_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::RunnersHelper do +RSpec.describe Ci::RunnersHelper, feature_category: :runner_fleet do let_it_be(:user) { create(:user) } before do @@ -38,6 +38,14 @@ end end + describe '#runner_short_name' do + it 'shows runner short name' do + runner = build_stubbed(:ci_runner, id: non_existing_record_id) + + expect(helper.runner_short_name(runner)).to eq("##{runner.id} (#{runner.short_sha})") + end + end + describe '#runner_contacted_at' do let(:contacted_at_stored) { 1.hour.ago.change(usec: 0) } let(:contacted_at_cached) { 1.second.ago.change(usec: 0) } @@ -77,7 +85,7 @@ describe '#admin_runners_data_attributes' do let_it_be(:admin) { create(:user, :admin) } let_it_be(:instance_runner) { create(:ci_runner, :instance) } - let_it_be(:project_runner) { create(:ci_runner, :project ) } + let_it_be(:project_runner) { create(:ci_runner, :project) } before do allow(helper).to receive(:current_user).and_return(admin) diff --git a/spec/support/rspec_order_todo.yml b/spec/support/rspec_order_todo.yml index c81c5c4184408..3d8207fced118 100644 --- a/spec/support/rspec_order_todo.yml +++ b/spec/support/rspec_order_todo.yml @@ -5030,7 +5030,6 @@ - './spec/helpers/ci/jobs_helper_spec.rb' - './spec/helpers/ci/pipeline_editor_helper_spec.rb' - './spec/helpers/ci/pipelines_helper_spec.rb' -- './spec/helpers/ci/runners_helper_spec.rb' - './spec/helpers/ci/secure_files_helper_spec.rb' - './spec/helpers/ci/status_helper_spec.rb' - './spec/helpers/ci/triggers_helper_spec.rb' @@ -9982,7 +9981,6 @@ - './spec/views/shared/projects/_inactive_project_deletion_alert.html.haml_spec.rb' - './spec/views/shared/projects/_list.html.haml_spec.rb' - './spec/views/shared/projects/_project.html.haml_spec.rb' -- './spec/views/shared/runners/_runner_details.html.haml_spec.rb' - './spec/views/shared/snippets/_snippet.html.haml_spec.rb' - './spec/views/shared/wikis/_sidebar.html.haml_spec.rb' - './spec/workers/admin_email_worker_spec.rb' diff --git a/spec/views/shared/runners/_runner_details.html.haml_spec.rb b/spec/views/shared/runners/_runner_details.html.haml_spec.rb index 9776d29de442c..a597c719d8776 100644 --- a/spec/views/shared/runners/_runner_details.html.haml_spec.rb +++ b/spec/views/shared/runners/_runner_details.html.haml_spec.rb @@ -2,11 +2,11 @@ require 'spec_helper' -RSpec.describe 'shared/runners/_runner_details.html.haml' do +RSpec.describe 'shared/runners/_runner_details.html.haml', feature_category: :runner_fleet do include PageLayoutHelper - let(:runner) do - create( + let_it_be(:runner) do + build_stubbed( :ci_runner, name: 'test runner', version: '11.4.0', @@ -25,29 +25,19 @@ rendered end - describe 'Page title' do - before do - expect(view).to receive(:page_title).with("##{runner.id} (#{runner.short_sha})") - end - - it 'sets proper page title' do - render - end - end - describe 'Runner id and type' do context 'when runner is of type instance' do it { is_expected.to have_content("Runner ##{runner.id} shared") } end context 'when runner is of type group' do - let(:runner) { create(:ci_runner, :group) } + let(:runner) { build_stubbed(:ci_runner, :group) } it { is_expected.to have_content("Runner ##{runner.id} group") } end context 'when runner is of type project' do - let(:runner) { create(:ci_runner, :project) } + let(:runner) { build_stubbed(:ci_runner, :project) } it { is_expected.to have_content("Runner ##{runner.id} project") } end @@ -59,7 +49,7 @@ end context 'when runner is inactive' do - let(:runner) { create(:ci_runner, :inactive) } + let(:runner) { build_stubbed(:ci_runner, :inactive) } it { is_expected.to have_content('Active No') } end @@ -71,7 +61,7 @@ end context 'when runner is protected' do - let(:runner) { create(:ci_runner, :ref_protected) } + let(:runner) { build_stubbed(:ci_runner, :ref_protected) } it { is_expected.to have_content('Protected Yes') } end @@ -83,7 +73,7 @@ end context 'when runner run untagged job is unset' do - let(:runner) { create(:ci_runner, :tagged_only) } + let(:runner) { build_stubbed(:ci_runner, :tagged_only) } it { is_expected.to have_content('Can run untagged jobs No') } end @@ -94,19 +84,19 @@ it { is_expected.to have_content('Locked to this project No') } context 'when runner is of type group' do - let(:runner) { create(:ci_runner, :group) } + let(:runner) { build_stubbed(:ci_runner, :group) } it { is_expected.not_to have_content('Locked to this project') } end end context 'when runner locked is set' do - let(:runner) { create(:ci_runner, :locked) } + let(:runner) { build_stubbed(:ci_runner, :locked) } it { is_expected.to have_content('Locked to this project Yes') } context 'when runner is of type group' do - let(:runner) { create(:ci_runner, :group, :locked) } + let(:runner) { build_stubbed(:ci_runner, :group, :locked) } it { is_expected.not_to have_content('Locked to this project') } end @@ -120,7 +110,7 @@ end context 'when runner have tags' do - let(:runner) { create(:ci_runner, tag_list: %w(tag2 tag3 tag1)) } + let(:runner) { build_stubbed(:ci_runner, tag_list: %w[tag2 tag3 tag1]) } it { is_expected.to have_content('Tags tag1 tag2 tag3') } it { is_expected.to have_selector('span.gl-badge.badge.badge-info') } @@ -138,7 +128,7 @@ end describe 'Maximum job timeout value' do - let(:runner) { create(:ci_runner, maximum_timeout: 5400) } + let(:runner) { build_stubbed(:ci_runner, maximum_timeout: 5400) } it { is_expected.to have_content('Maximum job timeout 1h 30m') } end @@ -149,7 +139,7 @@ end context 'when runner have already contacted' do - let(:runner) { create(:ci_runner, contacted_at: DateTime.now - 6.days) } + let(:runner) { build_stubbed(:ci_runner, contacted_at: DateTime.now - 6.days) } let(:expected_contacted_at) { I18n.l(runner.contacted_at, format: "%b %d, %Y") } it { is_expected.to have_content("Last contact #{expected_contacted_at}") } -- GitLab