Merge branch 'fix-pended-feature-specs-for-combined-menu-flag-2' into 'master'

Fix and unpend specs currently failing with :combined_menu flag on (take 2)

See merge request gitlab-org/gitlab!62715

app/assets/javascripts/nav/components/top_nav_app.vue

@@ -36,7 +36,7 @@ export default {
     <gl-nav-item-dropdown
       :text="navData.activeTitle"
       icon="dot-grid"
-      menu-class="gl-mt-3! gl-max-w-none! gl-max-h-none! gl-sm-w-auto!"
+      menu-class="gl-mt-3! gl-max-w-none! gl-max-h-none! gl-sm-w-auto! js-top-nav-dropdown-menu"
       toggle-class="top-nav-toggle js-top-nav-dropdown-toggle gl-px-3!"
       no-flip
     >

ee/spec/features/operations_nav_link_spec.rb

@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 RSpec.describe 'Operations dropdown navbar EE' do
+  include Spec::Support::Helpers::Features::TopNavSpecHelpers
+
   let(:user) { create(:user) }
   let(:project) { create(:project) }
 
@@ -17,19 +19,19 @@ RSpec.describe 'Operations dropdown navbar EE' do
     end
 
     it 'has an `Operations` link' do
-      pending_on_combined_menu_flag
+      open_top_nav
 
       expect(page).to have_link('Operations', href: operations_path)
     end
 
     it 'has an `Environments` link' do
-      pending_on_combined_menu_flag
+      open_top_nav
 
       expect(page).to have_link('Environments', href: operations_environments_path)
     end
   end
 
-  context 'with combined_menu: feature flag on' do
+  context 'with combined_menu feature flag on', :js do
     let(:needs_rewrite_for_combined_menu_flag_on) { true }
 
     before do
@@ -48,8 +50,4 @@ RSpec.describe 'Operations dropdown navbar EE' do
 
     it_behaves_like 'combined_menu: feature flag examples'
   end
-
-  def pending_on_combined_menu_flag
-    pending 'https://gitlab.com/gitlab-org/gitlab/-/merge_requests/56587' if needs_rewrite_for_combined_menu_flag_on
-  end
 end

spec/features/admin/admin_mode/logout_spec.rb

@@ -5,28 +5,32 @@ require 'spec_helper'
 RSpec.describe 'Admin Mode Logout', :js do
   include TermsHelper
   include UserLoginHelper
+  include Spec::Support::Helpers::Features::TopNavSpecHelpers
 
   let(:user) { create(:admin) }
 
   shared_examples 'combined_menu: feature flag examples' do
     before do
-      gitlab_sign_in(user)
+      # TODO: This used to use gitlab_sign_in, instead of sign_in, but that is buggy.  See
+      #   this issue to look into why: https://gitlab.com/gitlab-org/gitlab/-/issues/331851
+      sign_in(user)
       gitlab_enable_admin_mode_sign_in(user)
       visit admin_root_path
     end
 
     it 'disable removes admin mode and redirects to root page' do
-      pending_on_combined_menu_flag
-
       gitlab_disable_admin_mode
 
       expect(current_path).to eq root_path
-      expect(page).to have_link(href: new_admin_session_path)
+
+      open_top_nav
+
+      within_top_nav do
+        expect(page).to have_link(href: new_admin_session_path)
+      end
     end
 
     it 'disable shows flash notice' do
-      pending_on_combined_menu_flag
-
       gitlab_disable_admin_mode
 
       expect(page).to have_selector('.flash-notice')
@@ -38,17 +42,20 @@ RSpec.describe 'Admin Mode Logout', :js do
       end
 
       it 'disable removes admin mode and redirects to root page' do
-        pending_on_combined_menu_flag
-
         gitlab_disable_admin_mode
 
         expect(current_path).to eq root_path
-        expect(page).to have_link(href: new_admin_session_path)
+
+        open_top_nav
+
+        within_top_nav do
+          expect(page).to have_link(href: new_admin_session_path)
+        end
       end
     end
   end
 
-  context 'with combined_menu: feature flag on' do
+  context 'with combined_menu feature flag on' do
     let(:needs_rewrite_for_combined_menu_flag_on) { true }
 
     before do
@@ -67,8 +74,4 @@ RSpec.describe 'Admin Mode Logout', :js do
 
     it_behaves_like 'combined_menu: feature flag examples'
   end
-
-  def pending_on_combined_menu_flag
-    pending 'https://gitlab.com/gitlab-org/gitlab/-/merge_requests/56587' if needs_rewrite_for_combined_menu_flag_on
-  end
 end

spec/features/admin/admin_mode_spec.rb

@@ -4,6 +4,7 @@ require 'spec_helper'
 
 RSpec.describe 'Admin mode' do
   include MobileHelpers
+  include Spec::Support::Helpers::Features::TopNavSpecHelpers
   include StubENV
 
   let(:admin) { create(:admin) }
@@ -21,6 +22,7 @@ RSpec.describe 'Admin mode' do
       context 'when not in admin mode' do
         it 'has no leave admin mode button' do
           visit new_admin_session_path
+          open_top_nav
 
           page.within('.navbar-sub-nav') do
             expect(page).not_to have_link(href: destroy_admin_session_path)
@@ -28,12 +30,11 @@ RSpec.describe 'Admin mode' do
         end
 
         it 'can open pages not in admin scope' do
-          pending_on_combined_menu_flag
-
           visit new_admin_session_path
+          open_top_nav_projects
 
-          page.within('.navbar-sub-nav') do
-            find_all('a', text: 'Projects').first.click
+          within_top_nav do
+            click_link('Your projects')
           end
 
           expect(page).to have_current_path(dashboard_projects_path)
@@ -78,71 +79,64 @@ RSpec.describe 'Admin mode' do
         end
 
         it 'contains link to leave admin mode' do
-          pending_on_combined_menu_flag
+          open_top_nav
 
-          page.within('.navbar-sub-nav') do
+          within_top_nav do
             expect(page).to have_link(href: destroy_admin_session_path)
           end
         end
 
         it 'can leave admin mode using main dashboard link', :js do
-          pending_on_combined_menu_flag
+          gitlab_disable_admin_mode
 
-          page.within('.navbar-sub-nav') do
-            click_on 'Leave Admin Mode'
+          open_top_nav
 
+          within_top_nav do
             expect(page).to have_link(href: new_admin_session_path)
           end
         end
 
         it 'can leave admin mode using dropdown menu on smaller screens', :js do
-          pending_on_combined_menu_flag
-
           resize_screen_xs
           visit root_dashboard_path
 
-          find('.header-more').click
+          find('.header-more').click unless Feature.enabled?(:combined_menu)
 
-          page.within '.navbar-sub-nav' do
-            click_on 'Leave Admin Mode'
+          gitlab_disable_admin_mode
 
-            find('.header-more').click
+          open_top_nav
+          find('.header-more').click unless Feature.enabled?(:combined_menu)
 
-            expect(page).to have_link(href: new_admin_session_path)
-          end
+          expect(page).to have_link(href: new_admin_session_path)
         end
 
         it 'can open pages not in admin scope' do
-          pending_on_combined_menu_flag
+          open_top_nav_projects
 
-          page.within('.navbar-sub-nav') do
-            find_all('a', text: 'Projects').first.click
-
-            expect(page).to have_current_path(dashboard_projects_path)
+          within_top_nav do
+            click_link('Your projects')
           end
+
+          expect(page).to have_current_path(dashboard_projects_path)
         end
 
         context 'nav bar' do
           it 'shows admin dashboard links on bigger screen' do
-            pending_on_combined_menu_flag
-
             visit root_dashboard_path
+            open_top_nav
 
-            page.within '.navbar' do
-              expect(page).to have_link(text: 'Admin Area', href: admin_root_path, visible: true)
-              expect(page).to have_link(text: 'Leave Admin Mode', href: destroy_admin_session_path, visible: true)
-            end
+            link_text = Feature.enabled?(:combined_menu) ? 'Admin' : 'Admin Area'
+            expect(page).to have_link(text: link_text, href: admin_root_path, visible: true)
+            expect(page).to have_link(text: 'Leave Admin Mode', href: destroy_admin_session_path, visible: true)
           end
 
           it 'relocates admin dashboard links to dropdown list on smaller screen', :js do
-            pending_on_combined_menu_flag
+            skip('not applicable with :combined_menu feature flag enabled') if Feature.enabled?(:combined_menu)
 
             resize_screen_xs
             visit root_dashboard_path
 
-            page.within '.navbar' do
-              expect(page).not_to have_link(text: 'Leave Admin Mode', href: destroy_admin_session_path, visible: true)
-            end
+            expect(page).not_to have_link(text: 'Leave Admin Mode', href: destroy_admin_session_path, visible: true)
 
             find('.header-more').click
 
@@ -159,11 +153,11 @@ RSpec.describe 'Admin mode' do
           end
 
           it 'can leave admin mode', :js do
-            pending_on_combined_menu_flag
+            gitlab_disable_admin_mode
 
-            page.within('.navbar-sub-nav') do
-              click_on 'Leave Admin Mode'
+            open_top_nav
 
+            within_top_nav do
               expect(page).to have_link(href: new_admin_session_path)
             end
           end
@@ -179,16 +173,15 @@ RSpec.describe 'Admin mode' do
 
       it 'shows no admin mode buttons in navbar' do
         visit admin_root_path
+        open_top_nav
 
-        page.within('.navbar-sub-nav') do
-          expect(page).not_to have_link(href: new_admin_session_path)
-          expect(page).not_to have_link(href: destroy_admin_session_path)
-        end
+        expect(page).not_to have_link(href: new_admin_session_path)
+        expect(page).not_to have_link(href: destroy_admin_session_path)
       end
     end
   end
 
-  context 'with combined_menu: feature flag on' do
+  context 'with combined_menu feature flag on', :js do
     let(:needs_rewrite_for_combined_menu_flag_on) { true }
 
     before do
@@ -207,8 +200,4 @@ RSpec.describe 'Admin mode' do
 
     it_behaves_like 'combined_menu: feature flag examples'
   end
-
-  def pending_on_combined_menu_flag
-    pending 'https://gitlab.com/gitlab-org/gitlab/-/merge_requests/56587' if needs_rewrite_for_combined_menu_flag_on
-  end
 end

spec/features/dashboard/active_tab_spec.rb

@@ -2,6 +2,9 @@
 
 require 'spec_helper'
 
+# TODO: This entire spec file can be deleted once the combined_menu feature is fully rolled
+#   out and the flag is removed, because it will then be irrelevant (there will be no more tabs).
+#   Feature flag removal issue: https://gitlab.com/gitlab-org/gitlab/-/issues/324086
 RSpec.describe 'Dashboard Active Tab', :js do
   shared_examples 'combined_menu: feature flag examples' do
     before do
@@ -10,8 +13,6 @@ RSpec.describe 'Dashboard Active Tab', :js do
 
     shared_examples 'page has active tab' do |title|
       it "#{title} tab" do
-        pending_on_combined_menu_flag
-
         subject
 
         expect(page).to have_selector('.navbar-sub-nav li.active', count: 1)
@@ -32,27 +33,11 @@ RSpec.describe 'Dashboard Active Tab', :js do
     end
   end
 
-  context 'with combined_menu: feature flag on' do
-    let(:needs_rewrite_for_combined_menu_flag_on) { true }
-
-    before do
-      stub_feature_flags(combined_menu: true)
-    end
-
-    it_behaves_like 'combined_menu: feature flag examples'
-  end
-
   context 'with combined_menu feature flag off' do
-    let(:needs_rewrite_for_combined_menu_flag_on) { false }
-
     before do
       stub_feature_flags(combined_menu: false)
     end
 
     it_behaves_like 'combined_menu: feature flag examples'
   end
-
-  def pending_on_combined_menu_flag
-    pending 'https://gitlab.com/gitlab-org/gitlab/-/merge_requests/56587' if needs_rewrite_for_combined_menu_flag_on
-  end
 end

spec/features/dashboard/group_dashboard_with_external_authorization_service_spec.rb

@@ -4,6 +4,7 @@ require 'spec_helper'
 
 RSpec.describe 'The group dashboard' do
   include ExternalAuthorizationServiceHelpers
+  include Spec::Support::Helpers::Features::TopNavSpecHelpers
 
   let(:user) { create(:user) }
 
@@ -14,11 +15,11 @@ RSpec.describe 'The group dashboard' do
 
     describe 'The top navigation' do
       it 'has all the expected links' do
-        pending_on_combined_menu_flag
-
         visit dashboard_groups_path
 
-        within('.navbar') do
+        open_top_nav
+
+        within_top_nav do
           expect(page).to have_button('Projects')
           expect(page).to have_button('Groups')
           expect(page).to have_link('Activity')
@@ -28,12 +29,12 @@ RSpec.describe 'The group dashboard' do
       end
 
       it 'hides some links when an external authorization service is enabled' do
-        pending_on_combined_menu_flag
-
         enable_external_authorization_service_check
         visit dashboard_groups_path
 
-        within('.navbar') do
+        open_top_nav
+
+        within_top_nav do
           expect(page).to have_button('Projects')
           expect(page).to have_button('Groups')
           expect(page).not_to have_link('Activity')
@@ -44,7 +45,7 @@ RSpec.describe 'The group dashboard' do
     end
   end
 
-  context 'with combined_menu: feature flag on' do
+  context 'with combined_menu feature flag on', :js do
     let(:needs_rewrite_for_combined_menu_flag_on) { true }
 
     before do
@@ -63,8 +64,4 @@ RSpec.describe 'The group dashboard' do
 
     it_behaves_like 'combined_menu: feature flag examples'
   end
-
-  def pending_on_combined_menu_flag
-    pending 'https://gitlab.com/gitlab-org/gitlab/-/merge_requests/56587' if needs_rewrite_for_combined_menu_flag_on
-  end
 end

spec/features/dashboard/shortcuts_spec.rb

@@ -38,6 +38,10 @@ RSpec.describe 'Dashboard shortcuts', :js do
         find('body').send_keys([:shift, 'A'])
 
         check_page_title('Activity')
+
+        find('body').send_keys([:shift, 'L'])
+
+        check_page_title('Milestones')
       end
     end
 
@@ -69,7 +73,7 @@ RSpec.describe 'Dashboard shortcuts', :js do
     end
   end
 
-  context 'with combined_menu: feature flag on' do
+  context 'with combined_menu feature flag on' do
     before do
       stub_feature_flags(combined_menu: true)
     end

spec/features/frequently_visited_projects_and_groups_spec.rb

@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 RSpec.describe 'Frequently visited items', :js do
+  include Spec::Support::Helpers::Features::TopNavSpecHelpers
+
   let_it_be(:user) { create(:user) }
 
   shared_examples 'combined_menu: feature flag examples' do
@@ -14,9 +16,8 @@ RSpec.describe 'Frequently visited items', :js do
       let_it_be(:project) { create(:project, :public) }
 
       it 'increments localStorage counter when visiting the project' do
-        pending_on_combined_menu_flag
-
         visit project_path(project)
+        open_top_nav_projects
 
         frequent_projects = nil
 
@@ -34,9 +35,8 @@ RSpec.describe 'Frequently visited items', :js do
       let_it_be(:group) { create(:group, :public) }
 
       it 'increments localStorage counter when visiting the group' do
-        pending_on_combined_menu_flag
-
         visit group_path(group)
+        open_top_nav_groups
 
         frequent_groups = nil
 
@@ -51,7 +51,7 @@ RSpec.describe 'Frequently visited items', :js do
     end
   end
 
-  context 'with combined_menu: feature flag on' do
+  context 'with combined_menu feature flag on' do
     let(:needs_rewrite_for_combined_menu_flag_on) { true }
 
     before do
@@ -70,8 +70,4 @@ RSpec.describe 'Frequently visited items', :js do
 
     it_behaves_like 'combined_menu: feature flag examples'
   end
-
-  def pending_on_combined_menu_flag
-    pending 'https://gitlab.com/gitlab-org/gitlab/-/merge_requests/56587' if needs_rewrite_for_combined_menu_flag_on
-  end
 end

spec/features/projects/new_project_spec.rb

@@ -4,6 +4,7 @@ require 'spec_helper'
 
 RSpec.describe 'New project', :js do
   include Select2Helper
+  include Spec::Support::Helpers::Features::TopNavSpecHelpers
 
   shared_examples 'combined_menu: feature flag examples' do
     context 'as a user' do
@@ -45,34 +46,39 @@ RSpec.describe 'New project', :js do
         end
 
         it 'when in control it renders "project" in the new projects dropdown' do
-          pending_on_combined_menu_flag
-
           stub_experiments(new_repo: :control)
 
           visit new_project_path
 
-          find('#nav-projects-dropdown').click
-
-          page.within('#nav-projects-dropdown') do
-            expect(page).to have_selector('a', text: 'Create blank project')
-            expect(page).to have_selector('a', text: 'Import project')
-            expect(page).to have_no_selector('a', text: 'Create blank project/repository')
-            expect(page).to have_no_selector('a', text: 'Import project/repository')
+          open_top_nav_projects
+
+          within_top_nav do
+            if Feature.enabled?(:combined_menu)
+              expect(page).to have_selector('a', text: 'Create new project')
+              expect(page).to have_no_selector('a', text: 'Create blank project/repository')
+            else
+              expect(page).to have_selector('a', text: 'Create blank project')
+              expect(page).to have_selector('a', text: 'Import project')
+              expect(page).to have_no_selector('a', text: 'Create blank project/repository')
+              expect(page).to have_no_selector('a', text: 'Import project/repository')
+            end
           end
         end
 
         it 'when in candidate it renders "project/repository" in the new projects dropdown' do
-          pending_on_combined_menu_flag
-
           stub_experiments(new_repo: :candidate)
 
           visit new_project_path
 
-          find('#nav-projects-dropdown').click
+          open_top_nav_projects
 
-          page.within('#nav-projects-dropdown') do
-            expect(page).to have_selector('a', text: 'Create blank project/repository')
-            expect(page).to have_selector('a', text: 'Import project/repository')
+          within_top_nav do
+            if Feature.enabled?(:combined_menu)
+              expect(page).to have_selector('a', text: 'Create new project')
+            else
+              expect(page).to have_selector('a', text: 'Create blank project/repository')
+              expect(page).to have_selector('a', text: 'Import project/repository')
+            end
           end
         end
       end
@@ -412,7 +418,7 @@ RSpec.describe 'New project', :js do
     end
   end
 
-  context 'with combined_menu: feature flag on' do
+  context 'with combined_menu feature flag on' do
     let(:needs_rewrite_for_combined_menu_flag_on) { true }
 
     before do
@@ -431,8 +437,4 @@ RSpec.describe 'New project', :js do
 
     it_behaves_like 'combined_menu: feature flag examples'
   end
-
-  def pending_on_combined_menu_flag
-    pending 'https://gitlab.com/gitlab-org/gitlab/-/merge_requests/56587' if needs_rewrite_for_combined_menu_flag_on
-  end
 end

spec/support/helpers/features/top_nav_spec_helpers.rb

+# frozen_string_literal: true
+
+# These helpers help you interact within the Editor Lite (single-file editor, snippets, etc.).
+#
+module Spec
+  module Support
+    module Helpers
+      module Features
+        module TopNavSpecHelpers
+          def open_top_nav
+            return unless Feature.enabled?(:combined_menu)
+
+            find('.js-top-nav-dropdown-toggle').click
+          end
+
+          def within_top_nav
+            if Feature.enabled?(:combined_menu)
+              within('.js-top-nav-dropdown-menu') do
+                yield
+              end
+            else
+              within('.navbar-sub-nav') do
+                yield
+              end
+            end
+          end
+
+          def open_top_nav_projects
+            if Feature.enabled?(:combined_menu)
+              open_top_nav
+
+              within_top_nav do
+                click_button('Projects')
+              end
+            else
+              find('#nav-projects-dropdown').click
+            end
+          end
+
+          def open_top_nav_groups
+            return unless Feature.enabled?(:combined_menu)
+
+            open_top_nav
+
+            within_top_nav do
+              click_button('Groups')
+            end
+          end
+        end
+      end
+    end
+  end
+end

spec/support/helpers/login_helpers.rb

@@ -77,7 +77,11 @@ module LoginHelpers
 
   # Requires Javascript driver.
   def gitlab_disable_admin_mode
-    click_on 'Leave Admin Mode'
+    open_top_nav
+
+    within_top_nav do
+      click_on 'Leave Admin Mode'
+    end
   end
 
   private