test: address PR review feedback

- Parametrize tests with multiple assertions into single-assert-per-case
- Remove redundant service_locator._configuration = None (handled by autouse fixture)
- Inject mocked API clients to avoid accessing private _api_client attribute
- Move drop() calls into finally blocks for cleanup on test failure
- Always include is_running_in_ipython flag in get_system_info (True/False)
- Remove duplicate assert in test_create_same_hmac
- Remove unnecessary type: ignore comment

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Author: vdusek

src/apify/_utils.py

@@ -21,8 +21,7 @@ def get_system_info() -> dict:
         'os': sys.platform,
     }
 
-    if is_running_in_ipython():
-        system_info['is_running_in_ipython'] = True
+    system_info['is_running_in_ipython'] = is_running_in_ipython()
 
     return system_info
 

tests/integration/test_dataset.py

@@ -82,16 +82,17 @@ async def test_same_references_in_named_dataset(apify_token: str, monkeypatch: p
 
     async with Actor:
         dataset_by_name_1 = await Actor.open_dataset(name=dataset_name)
-        dataset_by_name_2 = await Actor.open_dataset(name=dataset_name)
-        assert dataset_by_name_1 is dataset_by_name_2
-
-        dataset_1_metadata = await dataset_by_name_1.get_metadata()
-        dataset_by_id_1 = await Actor.open_dataset(id=dataset_1_metadata.id)
-        dataset_by_id_2 = await Actor.open_dataset(id=dataset_1_metadata.id)
-        assert dataset_by_id_1 is dataset_by_name_1
-        assert dataset_by_id_2 is dataset_by_id_1
-
-        await dataset_by_name_1.drop()
+        try:
+            dataset_by_name_2 = await Actor.open_dataset(name=dataset_name)
+            assert dataset_by_name_1 is dataset_by_name_2
+
+            dataset_1_metadata = await dataset_by_name_1.get_metadata()
+            dataset_by_id_1 = await Actor.open_dataset(id=dataset_1_metadata.id)
+            dataset_by_id_2 = await Actor.open_dataset(id=dataset_1_metadata.id)
+            assert dataset_by_id_1 is dataset_by_name_1
+            assert dataset_by_id_2 is dataset_by_id_1
+        finally:
+            await dataset_by_name_1.drop()
 
 
 async def test_force_cloud(

tests/integration/test_key_value_store.py

@@ -46,16 +46,17 @@ async def test_same_references_in_named_kvs(apify_token: str, monkeypatch: pytes
 
     async with Actor:
         kvs_by_name_1 = await Actor.open_key_value_store(name=kvs_name)
-        kvs_by_name_2 = await Actor.open_key_value_store(name=kvs_name)
-        assert kvs_by_name_1 is kvs_by_name_2
-
-        kvs_1_metadata = await kvs_by_name_1.get_metadata()
-        kvs_by_id_1 = await Actor.open_key_value_store(id=kvs_1_metadata.id)
-        kvs_by_id_2 = await Actor.open_key_value_store(id=kvs_1_metadata.id)
-        assert kvs_by_id_1 is kvs_by_name_1
-        assert kvs_by_id_2 is kvs_by_id_1
-
-        await kvs_by_name_1.drop()
+        try:
+            kvs_by_name_2 = await Actor.open_key_value_store(name=kvs_name)
+            assert kvs_by_name_1 is kvs_by_name_2
+
+            kvs_1_metadata = await kvs_by_name_1.get_metadata()
+            kvs_by_id_1 = await Actor.open_key_value_store(id=kvs_1_metadata.id)
+            kvs_by_id_2 = await Actor.open_key_value_store(id=kvs_1_metadata.id)
+            assert kvs_by_id_1 is kvs_by_name_1
+            assert kvs_by_id_2 is kvs_by_id_1
+        finally:
+            await kvs_by_name_1.drop()
 
 
 async def test_set_and_get_value_in_same_run(key_value_store_apify: KeyValueStore) -> None:

tests/integration/test_request_queue.py

@@ -1003,13 +1003,16 @@ async def default_handler(context: BasicCrawlingContext) -> None:
         # Make sure all requests were handled.
         assert crawler.statistics.state.requests_finished == requests
 
-    # Check the request queue stats
-    await asyncio.sleep(10)  # Wait to be sure that metadata are updated
+    try:
+        # Check the request queue stats
+        await asyncio.sleep(10)  # Wait to be sure that metadata are updated
+
+        metadata = cast('ApifyRequestQueueMetadata', await rq.get_metadata())
+        Actor.log.info(f'{metadata.stats=}')
+        assert metadata.stats.write_count == requests * expected_write_count_per_request
 
-    metadata = cast('ApifyRequestQueueMetadata', await rq.get_metadata())
-    Actor.log.info(f'{metadata.stats=}')
-    assert metadata.stats.write_count == requests * expected_write_count_per_request
-    await rq.drop()
+    finally:
+        await rq.drop()
 
 
 async def test_cache_initialization(apify_token: str, monkeypatch: pytest.MonkeyPatch) -> None:
@@ -1217,16 +1220,17 @@ async def test_same_references_in_named_rq(apify_token: str, monkeypatch: pytest
 
     async with Actor:
         rq_by_name_1 = await Actor.open_request_queue(name=rq_name)
-        rq_by_name_2 = await Actor.open_request_queue(name=rq_name)
-        assert rq_by_name_1 is rq_by_name_2
-
-        rq_1_metadata = await rq_by_name_1.get_metadata()
-        rq_by_id_1 = await Actor.open_request_queue(id=rq_1_metadata.id)
-        rq_by_id_2 = await Actor.open_request_queue(id=rq_1_metadata.id)
-        assert rq_by_id_1 is rq_by_name_1
-        assert rq_by_id_2 is rq_by_id_1
-
-        await rq_by_name_1.drop()
+        try:
+            rq_by_name_2 = await Actor.open_request_queue(name=rq_name)
+            assert rq_by_name_1 is rq_by_name_2
+
+            rq_1_metadata = await rq_by_name_1.get_metadata()
+            rq_by_id_1 = await Actor.open_request_queue(id=rq_1_metadata.id)
+            rq_by_id_2 = await Actor.open_request_queue(id=rq_1_metadata.id)
+            assert rq_by_id_1 is rq_by_name_1
+            assert rq_by_id_2 is rq_by_id_1
+        finally:
+            await rq_by_name_1.drop()
 
 
 async def test_request_queue_deduplication(
@@ -1330,63 +1334,63 @@ async def test_concurrent_processing_simulation(apify_token: str, monkeypatch: p
     )
     async with Actor:
         rq = await Actor.open_request_queue()
+        try:
+            for i in range(20):
+                await rq.add_request(f'https://example.com/concurrent/{i}')
 
-        for i in range(20):
-            await rq.add_request(f'https://example.com/concurrent/{i}')
-
-        total_count = await rq.get_total_count()
-        assert total_count == 20
+            total_count = await rq.get_total_count()
+            assert total_count == 20
 
-        async def worker() -> int:
-            processed = 0
-            request_counter = 0
+            async def worker() -> int:
+                processed = 0
+                request_counter = 0
 
-            while request := await rq.fetch_next_request():
-                await asyncio.sleep(0.01)
+                while request := await rq.fetch_next_request():
+                    await asyncio.sleep(0.01)
 
-                if request_counter % 5 == 0 and request_counter > 0:
-                    await rq.reclaim_request(request)
-                else:
-                    await rq.mark_request_as_handled(request)
-                    processed += 1
+                    if request_counter % 5 == 0 and request_counter > 0:
+                        await rq.reclaim_request(request)
+                    else:
+                        await rq.mark_request_as_handled(request)
+                        processed += 1
 
-                request_counter += 1
+                    request_counter += 1
 
-            return processed
+                return processed
 
-        workers = [worker() for _ in range(3)]
-        results = await asyncio.gather(*workers)
+            workers = [worker() for _ in range(3)]
+            results = await asyncio.gather(*workers)
 
-        total_processed = sum(results)
+            total_processed = sum(results)
 
-        assert total_processed > 0
-        assert len(results) == 3
+            assert total_processed > 0
+            assert len(results) == 3
 
-        handled_after_workers = await rq.get_handled_count()
-        assert handled_after_workers == total_processed
+            handled_after_workers = await rq.get_handled_count()
+            assert handled_after_workers == total_processed
 
-        total_after_workers = await rq.get_total_count()
-        assert total_after_workers == 20
+            total_after_workers = await rq.get_total_count()
+            assert total_after_workers == 20
 
-        remaining_count = 0
-        while not await rq.is_finished():
-            request = await rq.fetch_next_request()
-            if request:
-                remaining_count += 1
-                await rq.mark_request_as_handled(request)
-            else:
-                break
-
-        final_handled = await rq.get_handled_count()
-        final_total = await rq.get_total_count()
-        assert final_handled == 20
-        assert final_total == 20
-        assert total_processed + remaining_count == 20
+            remaining_count = 0
+            while not await rq.is_finished():
+                request = await rq.fetch_next_request()
+                if request:
+                    remaining_count += 1
+                    await rq.mark_request_as_handled(request)
+                else:
+                    break
 
-        is_finished = await rq.is_finished()
-        assert is_finished is True
+            final_handled = await rq.get_handled_count()
+            final_total = await rq.get_total_count()
+            assert final_handled == 20
+            assert final_total == 20
+            assert total_processed + remaining_count == 20
 
-        await rq.drop()
+            is_finished = await rq.is_finished()
+            assert is_finished is True
+        finally:
+            await rq.drop()
 
 
 async def test_rq_isolation(apify_token: str, monkeypatch: pytest.MonkeyPatch) -> None:
@@ -1399,22 +1403,22 @@ async def test_rq_isolation(apify_token: str, monkeypatch: pytest.MonkeyPatch) -
     async with Actor:
         rq1 = await Actor.open_request_queue(name=rq_name_1)
         rq2 = await Actor.open_request_queue(name=rq_name_2)
+        try:
+            assert rq1 is not rq2
 
-        assert rq1 is not rq2
-
-        await rq1.add_request('https://example.com/queue1-request')
-        await rq2.add_request('https://example.com/queue2-request')
-
-        req1 = await rq1.fetch_next_request()
-        req2 = await rq2.fetch_next_request()
+            await rq1.add_request('https://example.com/queue1-request')
+            await rq2.add_request('https://example.com/queue2-request')
 
-        assert req1 is not None
-        assert 'queue1' in req1.url
-        assert req2 is not None
-        assert 'queue2' in req2.url
+            req1 = await rq1.fetch_next_request()
+            req2 = await rq2.fetch_next_request()
 
-        await rq1.mark_request_as_handled(req1)
-        await rq2.mark_request_as_handled(req2)
+            assert req1 is not None
+            assert 'queue1' in req1.url
+            assert req2 is not None
+            assert 'queue2' in req2.url
 
-        await rq1.drop()
-        await rq2.drop()
+            await rq1.mark_request_as_handled(req1)
+            await rq2.mark_request_as_handled(req2)
+        finally:
+            await rq1.drop()
+            await rq2.drop()

tests/unit/actor/test_actor_helpers.py

@@ -296,7 +296,7 @@ async def test_remote_method_with_invalid_timeout(
     async with Actor:
         actor_method = getattr(Actor, actor_method_name)
         with pytest.raises(ValueError, match='Invalid timeout'):
-            await actor_method(entity_id, timeout='invalid')  # type: ignore[arg-type]
+            await actor_method(entity_id, timeout='invalid')
 
 
 async def test_abort_with_status_message(

tests/unit/storage_clients/test_apify_dataset_client.py

@@ -8,24 +8,30 @@
 from apify.storage_clients._apify._dataset_client import ApifyDatasetClient
 
 
-def _make_dataset_client() -> ApifyDatasetClient:
+def _make_dataset_client(api_client: AsyncMock | None = None) -> tuple[ApifyDatasetClient, AsyncMock]:
     """Create an ApifyDatasetClient with a mocked API client."""
-    api_client = AsyncMock()
-    return ApifyDatasetClient(api_client=api_client, api_public_base_url='', lock=asyncio.Lock())
+    if api_client is None:
+        api_client = AsyncMock()
+
+    return ApifyDatasetClient(
+        api_client=api_client,
+        api_public_base_url='',
+        lock=asyncio.Lock(),
+    ), api_client
 
 
 async def test_purge_raises_not_implemented() -> None:
     """Test that purge() raises NotImplementedError."""
-    client = _make_dataset_client()
+    client, _ = _make_dataset_client()
     with pytest.raises(NotImplementedError, match='Purging datasets is not supported'):
         await client.purge()
 
 
 async def test_drop_calls_api_delete() -> None:
     """Test that drop() delegates to the API client."""
-    client = _make_dataset_client()
+    client, api_client = _make_dataset_client()
     await client.drop()
-    client._api_client.delete.assert_awaited_once()  # ty: ignore[possibly-missing-attribute]
+    api_client.delete.assert_awaited_once()
 
 
 async def test_deprecated_api_public_base_url() -> None: