Skip to content

refactor(tests): use db_session_with_containers in test_storage_key_loader#35766

Merged
asukaminato0721 merged 3 commits into
langgenius:mainfrom
guangyang1206:refactor/test-storage-key-db-session-32860
May 2, 2026
Merged

refactor(tests): use db_session_with_containers in test_storage_key_loader#35766
asukaminato0721 merged 3 commits into
langgenius:mainfrom
guangyang1206:refactor/test-storage-key-db-session-32860

Conversation

@guangyang1206
Copy link
Copy Markdown
Contributor

…oader (#32860)

Convert TestStorageKeyLoader from unittest.TestCase to a pytest class that receives the db_session_with_containers fixture as a parameter, removing the direct db.session() call pattern flagged in #32860.

Changes:

  • Remove unittest.TestCase inheritance and setUp/tearDown
  • Add db_session_with_containers as a parameter on every test method
  • Convert per-test DB setup helpers to @staticmethod helpers that accept a session argument, keeping helpers co-located with the tests
  • Preserve all existing test scenarios (local file, remote URL, tool file, mixed methods, empty list, tenant isolation, session isolation, etc.)
  • Add flask_req_ctx_with_containers via @pytest.mark.usefixtures

Part of #32860

Description

Important

  1. Make sure you have read our contribution guidelines
  2. Ensure there is an associated issue and you have been assigned to it
  3. Use the correct syntax to link this PR: Fixes #<issue number>.

Summary

Part of #32860 — converts TestStorageKeyLoader from a unittest.TestCase subclass that calls db.session() directly to a pytest class that receives the db_session_with_containers fixture as a parameter.

Changes

Before After
class TestStorageKeyLoader(unittest.TestCase) class TestStorageKeyLoader (plain pytest class)
setUp(self): self.session = db.session() Each test method accepts db_session_with_containers: Session
tearDown(self): self.session.close() Session lifecycle handled by the fixture
Per-test self._create_* methods that used self.session @staticmethod helpers that take session as a parameter

Why this matters

db.session() bypasses the transaction rollback managed by db_session_with_containers, which can leave test data in the database between tests and cause flakiness. The fixture ensures each test runs in an isolated transaction that is rolled back after completion.

All existing test scenarios preserved

  • test_load_storage_keys_local_file
  • test_load_storage_keys_remote_url
  • test_load_storage_keys_tool_file
  • test_load_storage_keys_mixed_methods
  • test_load_storage_keys_empty_list
  • test_load_storage_keys_ignores_legacy_file_tenant_id
  • test_load_storage_keys_missing_file_id
  • test_load_storage_keys_nonexistent_upload_file_records
  • test_load_storage_keys_nonexistent_tool_file_records
  • test_load_storage_keys_invalid_uuid
  • test_load_storage_keys_batch_efficiency
  • test_load_storage_keys_tenant_isolation
  • test_load_storage_keys_mixed_tenant_batch
  • test_load_storage_keys_duplicate_file_ids
  • test_load_storage_keys_session_isolation

Checklist

…oader (langgenius#32860)

Convert TestStorageKeyLoader from unittest.TestCase to a pytest class that
receives the db_session_with_containers fixture as a parameter, removing the
direct db.session() call pattern flagged in langgenius#32860.

Changes:
- Remove unittest.TestCase inheritance and setUp/tearDown
- Add db_session_with_containers as a parameter on every test method
- Convert per-test DB setup helpers to @staticmethod helpers that accept
  a session argument, keeping helpers co-located with the tests
- Preserve all existing test scenarios (local file, remote URL, tool file,
  mixed methods, empty list, tenant isolation, session isolation, etc.)
- Add flask_req_ctx_with_containers via @pytest.mark.usefixtures

Part of langgenius#32860
@dosubot dosubot Bot added size:XS This PR changes 0-9 lines, ignoring generated files. refactor labels May 2, 2026
@asukaminato0721 asukaminato0721 enabled auto-merge May 2, 2026 09:29
@autofix-ci autofix-ci Bot requested review from Yeuoly and crazywoola as code owners May 2, 2026 13:05
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 2, 2026

Pyrefly Diff

base → PR
--- /tmp/pyrefly_base.txt	2026-05-02 15:48:22.807276324 +0000
+++ /tmp/pyrefly_pr.txt	2026-05-02 15:48:15.079202106 +0000
@@ -5528,10 +5528,6 @@
    --> tests/unit_tests/factories/test_build_from_mapping.py:132:12
 ERROR Object of class `NoneType` has no attribute `storage_key` [missing-attribute]
    --> tests/unit_tests/factories/test_build_from_mapping.py:164:12
-ERROR `in` is not supported between `Literal['file']` and `None` [not-iterable]
-   --> tests/unit_tests/factories/test_file_factory.py:282:16
-ERROR `in` is not supported between `Literal['.txt']` and `None` [not-iterable]
-   --> tests/unit_tests/factories/test_file_factory.py:283:16
 ERROR Argument `datetime` is not assignable to parameter `created_at` with type `Decimal | bool | bytes | float | int | str | None` in function `fields.file_fields.FileWithSignedUrl.__init__` [bad-argument-type]
   --> tests/unit_tests/fields/test_file_fields.py:55:20
 ERROR `dict[str, str | None]` is not assignable to TypedDict key `site` with type `list[dict[str, Any]] | list[dict[str, str]] | str` [bad-typed-dict-key]

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors the containerized integration tests for StorageKeyLoader to use pytest-style tests that receive db_session_with_containers, eliminating the direct db.session() pattern and aligning with the container test fixture lifecycle.

Changes:

  • Convert TestStorageKeyLoader from unittest.TestCase (setUp/tearDown) to a plain pytest test class.
  • Thread db_session_with_containers: Session through all test methods and update helper builders to accept an explicit Session.
  • Minor formatting-only whitespace adjustments in a stress-test TypedDict helper file.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
api/tests/test_containers_integration_tests/factories/test_storage_key_loader.py Reworks tests to pytest + db_session_with_containers, refactoring per-test setup into session-accepting helper methods.
scripts/stress-test/common/config_helper.py Adds blank lines inside several TypedDict definitions (formatting-only).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread scripts/stress-test/common/config_helper.py
@dosubot dosubot Bot added the lgtm This PR has been approved by a maintainer label May 2, 2026
@asukaminato0721 asukaminato0721 added this pull request to the merge queue May 2, 2026
Merged via the queue into langgenius:main with commit 3708e3e May 2, 2026
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm This PR has been approved by a maintainer refactor size:XS This PR changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants