Deduplication analysis and refactoring of test_history_archiving.py

refactor.diff

diff --git a/test/integration/test_history_archiving.py b/test/integration/test_history_archiving.py
index 2bbcb91ccb..e12c193fdd 100644
--- a/test/integration/test_history_archiving.py
+++ b/test/integration/test_history_archiving.py
@@ -229,7 +229,7 @@ class TestHistoryArchivingWithExportRecord(PosixFileSourceSetup, IntegrationTest
         )
         self.dataset_populator.export_history_to_uri_async(history_id, target_uri, model_store_format)
         export_records = self.dataset_populator.get_history_export_tasks(history_id)
-        assert len(export_records) == 1
+        assert len(export_records) >= 1
         last_record = export_records[0]
         self.dataset_populator.wait_for_export_task_on_record(last_record)
         assert last_record["ready"] is True
@@ -238,29 +238,18 @@ class TestHistoryArchivingWithExportRecord(PosixFileSourceSetup, IntegrationTest
     def _export_history_to_short_term_storage(self, history_id):
         self.dataset_populator.download_history_to_store(history_id)
         export_records = self.dataset_populator.get_history_export_tasks(history_id)
-        assert len(export_records) == 1
+        assert len(export_records) >= 1
         last_record = export_records[0]
         self.dataset_populator.wait_for_export_task_on_record(last_record)
         assert last_record["ready"] is True
         return last_record
 
 
-class TestHistoryArchivingUserUpdateTime(PosixFileSourceSetup, IntegrationTestCase):
-    """Test user update_time handling during history archival."""
+class TestHistoryArchivingAdmin(TestHistoryArchivingWithExportRecord):
+    """Test admin-specific history archival behavior."""
 
-    dataset_populator: DatasetPopulator
-    task_based = True
     require_admin_user = True
 
-    @classmethod
-    def handle_galaxy_config_kwds(cls, config):
-        super().handle_galaxy_config_kwds(config)
-        IntegrationTestCase.handle_galaxy_config_kwds(config)
-
-    def setUp(self):
-        super().setUp()
-        self.dataset_populator = DatasetPopulator(self.galaxy_interactor)
-
     def test_user_archive_own_history_updates_update_time(self):
         """When a user archives their own history, their update_time should be updated."""
         history_name = f"for_archiving_{uuid4()}"
@@ -283,7 +272,7 @@ class TestHistoryArchivingUserUpdateTime(PosixFileSourceSetup, IntegrationTestCa
 
         # Archive with purge
         target_uri = f"gxfiles://posix_test/history_{history_id}"
-        export_record = self._export_history(history_id, target_uri)
+        export_record = self._export_history_to_permanent_storage(history_id, target_uri=target_uri)
         archive_response = self.dataset_populator.archive_history(
             history_id,
             export_record_id=export_record["id"],
@@ -324,7 +313,7 @@ class TestHistoryArchivingUserUpdateTime(PosixFileSourceSetup, IntegrationTestCa
 
         # Admin archives owner's history with purge
         target_uri = f"gxfiles://posix_test/history_{history_id}"
-        export_record = self._export_history(history_id, target_uri)
+        export_record = self._export_history_to_permanent_storage(history_id, target_uri=target_uri)
         archive_response = self.dataset_populator.archive_history(
             history_id,
             export_record_id=export_record["id"],
@@ -340,13 +329,3 @@ class TestHistoryArchivingUserUpdateTime(PosixFileSourceSetup, IntegrationTestCa
             f"Owner's update_time should NOT be updated when admin archives their history. "
             f"Got {owner.update_time}, expected {old_update_time}"
         )
-
-    def _export_history(self, history_id: str, target_uri: str):
-        """Export history and return export record."""
-        self.dataset_populator.export_history_to_uri_async(history_id, target_uri, ModelStoreFormat.ROCRATE_ZIP)
-        export_records = self.dataset_populator.get_history_export_tasks(history_id)
-        assert len(export_records) >= 1
-        last_record = export_records[0]
-        self.dataset_populator.wait_for_export_task_on_record(last_record)
-        assert last_record["ready"] is True
-        return last_record

transcript.md

Code Deduplication Analysis and Refactoring Session

Initial Request

User ran /jmchilton:dedup command to analyze the last commit for code duplication.

Analysis

The last commit (fdd6036cb4) added functionality to preserve a user's update_time when an admin archives their history. Files changed:

• lib/galaxy/celery/tasks.py
• lib/galaxy/managers/hdas.py
• lib/galaxy/model/__init__.py
• lib/galaxy/webapps/galaxy/services/histories.py
• test/integration/test_history_archiving.py

Duplication Found

Group 1: _export_history vs _export_history_to_permanent_storage

Type III (Near-miss duplication) - Two nearly identical implementations:

• _export_history_to_permanent_storage (lines 221-236)
• _export_history (lines 344-352) - new in this commit

Key differences:

Aspect	Version 1	Version 2
target_uri	Optional w/ default	Required
model_store_format	Configurable	Hardcoded ROCRATE_ZIP
export_records assertion	== 1	>= 1

Group 2: Test Class Setup Boilerplate

Type II (Structural duplication) - Both test classes had identical:

• dataset_populator: DatasetPopulator
• task_based = True
• handle_galaxy_config_kwds classmethod
• setUp method

User Request

Give me a plan that just universally changes those assertions to >= 1. Also do the refactoring that lets the second class subclass the first but change the name of the second class to TestHistoryArchivingAdmin.

Changes Made

1. Changed _export_history_to_permanent_storage assertion from == 1 to >= 1
2. Changed _export_history_to_short_term_storage assertion from == 1 to >= 1
3. Made TestHistoryArchivingUserUpdateTime subclass TestHistoryArchivingWithExportRecord
4. Renamed class to TestHistoryArchivingAdmin
5. Removed duplicate boilerplate (class attributes, handle_galaxy_config_kwds, setUp)
6. Removed duplicate _export_history method, updated calls to use inherited _export_history_to_permanent_storage

Result

Net reduction of ~21 lines while maintaining functionality. The subclass now only contains:

• require_admin_user = True (unique to this class)
• The two test methods specific to admin behavior

https://github.com/jmchilton/claude-commands/blob/main/dedup.md