Improve selectors and simplify code

Add correct group comparison

Use plain args

Add waiting check

Add testcase link

Remove failure aggregate

Explicitly remove imported group first

Use sandbox group as target

app/assets/javascripts/import_entities/import_groups/components/import_table_row.vue

@@ -122,6 +122,7 @@ export default {
   <tr
     class="gl-border-gray-200 gl-border-0 gl-border-b-1 gl-border-solid"
     data-qa-selector="import_item"
+    :data-qa-source-group="group.full_path"
   >
     <td class="gl-p-4">
       <gl-link
@@ -166,6 +167,8 @@ export default {
             <gl-dropdown-item
               v-for="ns in availableNamespaces"
               :key="ns.full_path"
+              data-qa-selector="target_group_dropdown_item"
+              :data-qa-group-name="ns.full_path"
               @click="$emit('update-target-namespace', ns.full_path)"
             >
               {{ ns.full_path }}

qa/qa/page/group/bulk_import.rb

@@ -11,35 +11,35 @@ module QA
         view "app/assets/javascripts/import_entities/import_groups/components/import_table_row.vue" do
           element :import_item
           element :target_namespace_selector_dropdown
+          element :target_group_dropdown_item
           element :import_status_indicator
           element :import_group_button
         end
 
-        # Wait until list of groups has been loaded
-        #
-        # @return [void]
-        def wait_for_groups_to_load
-          has_element?(:import_group_table)
-        end
-
         # Import source group in to target group
         #
         # @param [String] source_group_name
         # @param [String] target_group_name
-        # @return [Boolean]
+        # @return [void]
         def import_group(source_group_name, target_group_name)
-          source_group = all_elements(:import_item, minimum: 1).detect { |el| el.has_text?(source_group_name) }
-          raise("Import entry for #{source_group_name} not found!") unless source_group
+          finished_loading?
 
-          within(source_group) do
+          within_element(:import_item, source_group: source_group_name) do
             click_element(:target_namespace_selector_dropdown)
-            find_button(target_group_name).click
+            click_element(:target_group_dropdown_item, group_name: target_group_name)
             click_element(:import_group_button)
-
-            wait_until(sleep_interval: 0.5, reload: false, raise_on_failure: false) do
-              find_element(:import_status_indicator).text == "Complete"
           end
         end
+
+        # Check if import page has a successfully imported group
+        #
+        # @param [String] source_group_name
+        # @param [Integer] wait
+        # @return [Boolean]
+        def has_imported_group?(source_group_name, wait: QA::Support::WaitForRequests::DEFAULT_MAX_WAIT_TIME)
+          within_element(:import_item, source_group: source_group_name) do
+            has_element?(:import_status_indicator, text: "Complete", wait: wait)
+          end
         end
       end
     end

qa/qa/specs/features/browser_ui/1_manage/group/bulk_import_group_spec.rb

@@ -7,61 +7,77 @@ module QA
       let!(:user) { Resource::User.fabricate_via_api! { |usr| usr.api_client = api_client } }
       let!(:personal_access_token) { Runtime::API::Client.new(user: user).personal_access_token }
 
-      let(:source_group) do
+      let!(:sandbox) do
         Resource::Sandbox.fabricate_via_api! do |group|
           group.api_client = api_client
-          group.path = "source-group-for-import-#{SecureRandom.hex(4)}"
         end
       end
 
-      let(:target_group) do
+      let!(:source_group) do
         Resource::Sandbox.fabricate_via_api! do |group|
           group.api_client = api_client
-          group.path = "target-group-for-import-#{SecureRandom.hex(4)}"
+          group.path = "source-group-for-import-#{SecureRandom.hex(4)}"
         end
       end
 
       let(:imported_group) do
-        Resource::Group.fabricate_via_api! do |group|
+        Resource::Group.new.tap do |group|
           group.api_client = api_client
-          group.sandbox = target_group
           group.path = source_group.path
+        end.reload!
+      rescue Resource::ApiFabricator::ResourceNotFoundError
+        nil
       end
+
+      # Return subset of fields for comparing groups
+      #
+      # @param [Resource::Group, nil] group
+      # @return [Hash]
+      def comparable_group(group)
+        group&.api_resource&.except(
+          :id,
+          :web_url,
+          :visibility,
+          :full_name,
+          :full_path,
+          :created_at,
+          :parent_id,
+          :runners_token
+        )
       end
 
       before do
         Runtime::Feature.enable(:bulk_import)
 
+        sandbox.add_member(user, Resource::Members::AccessLevel::MAINTAINER)
         source_group.add_member(user, Resource::Members::AccessLevel::MAINTAINER)
-        target_group.add_member(user, Resource::Members::AccessLevel::MAINTAINER)
 
         Flow::Login.sign_in(as: user)
         Page::Main::Menu.new.go_to_import_group
         Page::Group::New.new.connect_gitlab_instance(Runtime::Scenario.gitlab_address, personal_access_token)
       end
 
-      it "performs bulk group import from another gitlab instance" do
-        import = Page::Group::BulkImport.perform do |import_page|
-          import_page.wait_for_groups_to_load
-          import_page.import_group(source_group.path, target_group.path)
-        end
+      it(
+        "performs bulk group import from another gitlab instance",
+        testcase: "https://gitlab.com/gitlab-org/quality/testcases/-/issues/1785"
+      ) do
+        Page::Group::BulkImport.perform do |import_page|
+          import_page.import_group(source_group.path, sandbox.path)
 
           aggregate_failures do
-          expect(import).to be_truthy, "Group bulk import did not finish successfully"
-          expect(imported_group.path).to eq(source_group.path)
+            expect(import_page).to have_imported_group(source_group.path, wait: 60)
+            expect(comparable_group(imported_group)).to eq(comparable_group(source_group))
+          end
         end
       end
 
       after do
         Runtime::Feature.disable(:bulk_import)
 
-        source_group&.remove_via_api!
-        target_group&.remove_via_api!
-
-        # Imported group might not be immediately removed and 'user' is sole maintainer, so remove might fail
-        QA::Support::Retrier.retry_on_exception do
-          user&.remove_via_api!
-        end
+        # Add additional maintainer to imported group so user is not sole maintainer and can be deleted
+        imported_group&.add_member(Runtime::User.admin, Resource::Members::AccessLevel::MAINTAINER)
+        source_group.remove_via_api!
+        user.remove_via_api!
       end
     end
   end

qa/spec/spec_helper.rb

@@ -3,6 +3,7 @@
 require_relative '../qa'
 require 'rspec/retry'
 require 'rspec-parameterized'
+require 'active_support/core_ext/hash'
 
 if ENV['CI'] && QA::Runtime::Env.knapsack? && !ENV['NO_KNAPSACK']
   require 'knapsack'

qa/spec/specs/runner_spec.rb

 # frozen_string_literal: true
 
-require 'active_support/core_ext/hash'
-
 RSpec.describe QA::Specs::Runner do
   shared_examples 'excludes orchestrated, transient, and geo' do
     it 'excludes the orchestrated, transient, and geo tags, and includes default args' do