From 2d36d61183925d88457a6fa36715b50913477ef7 Mon Sep 17 00:00:00 2001 From: Lenore Kubie Date: Fri, 25 Sep 2026 17:19:22 +0000 Subject: [PATCH 1/7] test(integration): add poll_until predicate; fix inventory storage-location search flake --- .../integration/collections/test_inventory.py | 8 +++++-- tests/integration/utils/wait.py | 20 ++++++++++++---- tests/unit/utils/test_wait.py | 24 +++++++++++++++++++ 3 files changed, 46 insertions(+), 6 deletions(-) create mode 100644 tests/unit/utils/test_wait.py diff --git a/tests/integration/collections/test_inventory.py b/tests/integration/collections/test_inventory.py index 44881f355..46f4bb4e5 100644 --- a/tests/integration/collections/test_inventory.py +++ b/tests/integration/collections/test_inventory.py @@ -185,12 +185,16 @@ def search_scoped(**kwargs): ] filter_results = poll_until( - lambda: search_scoped(storage_location=[StorageLocationFilter(name=unit.name)]) + lambda: search_scoped(storage_location=[StorageLocationFilter(name=unit.name)]), + predicate=lambda results: {f"INV{p.id}" for p in results} == expected_ids, ) assert {f"INV{p.id}" for p in filter_results} == expected_ids # The full StorageLocation object from a lookup remains accepted. - object_results = poll_until(lambda: search_scoped(storage_location=unit)) + object_results = poll_until( + lambda: search_scoped(storage_location=unit), + predicate=lambda results: {f"INV{p.id}" for p in results} == expected_ids, + ) assert {f"INV{p.id}" for p in object_results} == expected_ids diff --git a/tests/integration/utils/wait.py b/tests/integration/utils/wait.py index 2809e1082..f8ec7af7b 100644 --- a/tests/integration/utils/wait.py +++ b/tests/integration/utils/wait.py @@ -2,15 +2,27 @@ from collections.abc import Callable -def poll_until(fetch: Callable[[], list], *, timeout: float = 30.0, interval: float = 1.0) -> list: - """Poll ``fetch`` until it returns a non-empty result or the timeout elapses. +def poll_until( + fetch: Callable[[], list], + *, + predicate: Callable[[list], bool] | None = None, + timeout: float = 30.0, + interval: float = 1.0, +) -> list: + """Poll ``fetch`` until the result satisfies ``predicate`` or the timeout elapses. Search-index-backed endpoints lag behind seeding; tests asserting on fresh - seeds poll instead of assuming immediate visibility. Returns the last result. + seeds poll instead of assuming immediate visibility. With no ``predicate``, + any non-empty result ends polling. Returns the last result. + + Pass a ``predicate`` when the assertion needs the complete expected set: a + non-empty but partially indexed result satisfies the default check while + remaining items are still becoming visible. """ deadline = time.monotonic() + timeout while True: result = fetch() - if result or time.monotonic() >= deadline: + ready = predicate(result) if predicate is not None else bool(result) + if ready or time.monotonic() >= deadline: return result time.sleep(interval) diff --git a/tests/unit/utils/test_wait.py b/tests/unit/utils/test_wait.py new file mode 100644 index 000000000..8cac1da6b --- /dev/null +++ b/tests/unit/utils/test_wait.py @@ -0,0 +1,24 @@ +from tests.integration.utils.wait import poll_until + + +def test_poll_until_returns_first_non_empty_by_default(): + """Test that polling stops at the first non-empty result with no predicate.""" + calls = iter([[], ["a"], ["a", "b"]]) + assert poll_until(lambda: next(calls), interval=0) == ["a"] + + +def test_poll_until_predicate_waits_for_satisfaction(): + """Test that polling continues until the predicate accepts the result.""" + calls = iter([["a"], ["a", "b"], ["a", "b"]]) + result = poll_until( + lambda: next(calls), + predicate=lambda result: len(result) == 2, + interval=0, + ) + assert result == ["a", "b"] + + +def test_poll_until_returns_last_result_on_timeout(): + """Test that an unsatisfied predicate returns the last result after the timeout.""" + result = poll_until(lambda: [], timeout=0.01, interval=0.005) + assert result == [] From 3c458e05b311e51f1d45376378ca39adcb775c36 Mon Sep 17 00:00:00 2001 From: Lenore Kubie Date: Fri, 25 Sep 2026 17:21:49 +0000 Subject: [PATCH 2/7] docs(testing): document poll_until predicate for complete-set search asserts --- tests/integration/TESTING.md | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/tests/integration/TESTING.md b/tests/integration/TESTING.md index d57423536..3ab3b0927 100644 --- a/tests/integration/TESTING.md +++ b/tests/integration/TESTING.md @@ -108,6 +108,26 @@ def test_hydrate_project(client: Albert, seed_prefix: str, seeded_projects: list assert projects, "Expected at least one project in search results" ``` +`poll_until` stops at the first **non-empty** result by default. Our NoSQL store plus +search index is eventually consistent, so freshly seeded items become visible one by one +and a partial page is normal during indexing. If the assertion needs the **complete** +expected set (exact-equality asserts against your fixture's ids), pass a `predicate` so +polling continues until the set is whole; without it the test flakes the moment one item +indexes before the rest: + +```python +expected_ids = {f"INV{lot.inventory_id}" for lot in seeded_lots if ...} +results = poll_until( + lambda: search_scoped(...), + predicate=lambda results: {f"INV{p.id}" for p in results} == expected_ids, +) +assert {f"INV{p.id}" for p in results} == expected_ids +``` + +Even with a predicate, a rare timeout is expected behavior of an eventually consistent +index. Treat it as a re-run, not a code change — but write the predicate to describe the +full expectation so only genuine lag can trip it. + Also: - Never assert **exact counts** of unscoped `search()`/`get_all()` results; other workers @@ -156,7 +176,7 @@ group's files with `-n 4` to catch cross-worker races. - [ ] `pytestmark = pytest.mark.xdist_group("...")` chosen per Rule 1 (or deliberately unmarked because only `client` is used) - [ ] No mutation of `seeded_*` entities; private entities cleaned up in `try/finally` -- [ ] Search assertions scoped, id-filtered, and wrapped in `poll_until` +- [ ] Search assertions scoped, id-filtered, and wrapped in `poll_until` (with a `predicate` when asserting a complete expected set) - [ ] No exact-count asserts on global queries - [ ] Names of created entities include `seed_prefix` or `TEST - ` - [ ] Ran the file plus its group with `-n 4` locally From 09c600720d78bc828bd986c78d4cad6fdddb6416 Mon Sep 17 00:00:00 2001 From: Lenore Kubie Date: Fri, 25 Sep 2026 17:31:37 +0000 Subject: [PATCH 3/7] refactor(tests): move wait.py to shared tests/utils to respect suite boundary --- tests/integration/TESTING.md | 4 ++-- tests/integration/utils/wait.py | 29 +++-------------------------- tests/unit/utils/test_wait.py | 2 +- tests/utils/__init__.py | 0 tests/utils/wait.py | 28 ++++++++++++++++++++++++++++ 5 files changed, 34 insertions(+), 29 deletions(-) create mode 100644 tests/utils/__init__.py create mode 100644 tests/utils/wait.py diff --git a/tests/integration/TESTING.md b/tests/integration/TESTING.md index 3ab3b0927..5a3435e02 100644 --- a/tests/integration/TESTING.md +++ b/tests/integration/TESTING.md @@ -90,10 +90,10 @@ For any search-based assertion: 1. Scope the query with `text=`/`name=` set to `seed_prefix` (reduces noise). 2. **Filter results to the ids owned by the fixture** (correctness). -3. Wrap the fetch in `poll_until` from `tests/integration/utils/wait.py` (index lag). +3. Wrap the fetch in `poll_until` from `tests/utils/wait.py` (index lag). ```python -from tests.integration.utils.wait import poll_until +from tests.utils.wait import poll_until def test_hydrate_project(client: Albert, seed_prefix: str, seeded_projects: list[Project]): diff --git a/tests/integration/utils/wait.py b/tests/integration/utils/wait.py index f8ec7af7b..c84597ab5 100644 --- a/tests/integration/utils/wait.py +++ b/tests/integration/utils/wait.py @@ -1,28 +1,5 @@ -import time -from collections.abc import Callable +"""Backwards-compatible re-export; the helper lives in ``tests.utils.wait``.""" +from tests.utils.wait import poll_until -def poll_until( - fetch: Callable[[], list], - *, - predicate: Callable[[list], bool] | None = None, - timeout: float = 30.0, - interval: float = 1.0, -) -> list: - """Poll ``fetch`` until the result satisfies ``predicate`` or the timeout elapses. - - Search-index-backed endpoints lag behind seeding; tests asserting on fresh - seeds poll instead of assuming immediate visibility. With no ``predicate``, - any non-empty result ends polling. Returns the last result. - - Pass a ``predicate`` when the assertion needs the complete expected set: a - non-empty but partially indexed result satisfies the default check while - remaining items are still becoming visible. - """ - deadline = time.monotonic() + timeout - while True: - result = fetch() - ready = predicate(result) if predicate is not None else bool(result) - if ready or time.monotonic() >= deadline: - return result - time.sleep(interval) +__all__ = ["poll_until"] diff --git a/tests/unit/utils/test_wait.py b/tests/unit/utils/test_wait.py index 8cac1da6b..21c1ec747 100644 --- a/tests/unit/utils/test_wait.py +++ b/tests/unit/utils/test_wait.py @@ -1,4 +1,4 @@ -from tests.integration.utils.wait import poll_until +from tests.utils.wait import poll_until def test_poll_until_returns_first_non_empty_by_default(): diff --git a/tests/utils/__init__.py b/tests/utils/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/utils/wait.py b/tests/utils/wait.py new file mode 100644 index 000000000..f8ec7af7b --- /dev/null +++ b/tests/utils/wait.py @@ -0,0 +1,28 @@ +import time +from collections.abc import Callable + + +def poll_until( + fetch: Callable[[], list], + *, + predicate: Callable[[list], bool] | None = None, + timeout: float = 30.0, + interval: float = 1.0, +) -> list: + """Poll ``fetch`` until the result satisfies ``predicate`` or the timeout elapses. + + Search-index-backed endpoints lag behind seeding; tests asserting on fresh + seeds poll instead of assuming immediate visibility. With no ``predicate``, + any non-empty result ends polling. Returns the last result. + + Pass a ``predicate`` when the assertion needs the complete expected set: a + non-empty but partially indexed result satisfies the default check while + remaining items are still becoming visible. + """ + deadline = time.monotonic() + timeout + while True: + result = fetch() + ready = predicate(result) if predicate is not None else bool(result) + if ready or time.monotonic() >= deadline: + return result + time.sleep(interval) From a5866e8e5a9322beae6d246d64097de5e66d9583 Mon Sep 17 00:00:00 2001 From: Lenore Kubie Date: Fri, 25 Sep 2026 17:54:52 +0000 Subject: [PATCH 4/7] test(integration): scope polls to asserted items; unify wait import path Polls filtered to any seeded id stop at the first partially indexed page, so assertions on a specific item or a complete set can flake. Filter each fetched page to the exact item under assertion so the default non-empty check implies the assertion; keep predicates only for true complete-set asserts (parameters id lists, inventory created_by three-way equality). Move all callers to tests.utils.wait and drop the re-export shim. --- AGENTS.md | 2 +- .../collections/test_attributes.py | 2 +- .../integration/collections/test_btinsight.py | 2 +- .../collections/test_custom_templates.py | 2 +- .../collections/test_data_columns.py | 2 +- .../collections/test_data_templates.py | 2 +- tests/integration/collections/test_files.py | 2 +- .../integration/collections/test_inventory.py | 35 ++++++++++++------- .../integration/collections/test_notebooks.py | 8 ++--- .../collections/test_parameter_groups.py | 7 ++-- .../collections/test_parameters.py | 5 ++- .../collections/test_product_design.py | 8 ++--- .../integration/collections/test_projects.py | 2 +- tests/integration/collections/test_reports.py | 10 +++--- .../integration/collections/test_synthesis.py | 2 +- tests/integration/collections/test_tasks.py | 2 +- tests/integration/collections/test_teams.py | 2 +- .../integration/collections/test_workflows.py | 2 +- .../integration/collections/test_worksheet.py | 2 +- tests/integration/utils/wait.py | 5 --- 20 files changed, 53 insertions(+), 51 deletions(-) delete mode 100644 tests/integration/utils/wait.py diff --git a/AGENTS.md b/AGENTS.md index 93056359c..64b03ef7e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -168,7 +168,7 @@ Exception: when a backend caps page size below `DEFAULT_LIMIT` (1000), set `limi - Shared `seeded_*` fixtures are read-only; update/delete tests create private entities and clean up in `try/finally`. - Search assertions must be scoped to `seed_prefix`, filtered to the fixture's ids, and - wrapped in `poll_until` (`tests/integration/utils/wait.py`); `text`/`name` params are + wrapped in `poll_until` (`tests/utils/wait.py`); `text`/`name` params are fuzzy full-text queries, and other workers delete their seeds mid-run. - No exact-count asserts on unscoped queries. - Seed helpers live in `tests/integration/seeding.py`; new seed entities are appended diff --git a/tests/integration/collections/test_attributes.py b/tests/integration/collections/test_attributes.py index c0ece31f6..24b638dfd 100644 --- a/tests/integration/collections/test_attributes.py +++ b/tests/integration/collections/test_attributes.py @@ -18,7 +18,7 @@ from albert.resources.inventory import InventoryItem from albert.resources.lots import Lot from albert.resources.parameter_groups import DataType, EnumValidationValue, Operator -from tests.integration.utils.wait import poll_until +from tests.utils.wait import poll_until pytestmark = pytest.mark.xdist_group("inventory") diff --git a/tests/integration/collections/test_btinsight.py b/tests/integration/collections/test_btinsight.py index 0df349e15..a480ce976 100644 --- a/tests/integration/collections/test_btinsight.py +++ b/tests/integration/collections/test_btinsight.py @@ -2,7 +2,7 @@ from albert import Albert from albert.resources.btinsight import BTInsight, BTInsightCategory, BTInsightRegistry -from tests.integration.utils.wait import poll_until +from tests.utils.wait import poll_until pytestmark = pytest.mark.xdist_group("bt") diff --git a/tests/integration/collections/test_custom_templates.py b/tests/integration/collections/test_custom_templates.py index 7516e6419..f3203b514 100644 --- a/tests/integration/collections/test_custom_templates.py +++ b/tests/integration/collections/test_custom_templates.py @@ -9,7 +9,7 @@ _CustomTemplateDataUnion, ) from albert.resources.users import User -from tests.integration.utils.wait import poll_until +from tests.utils.wait import poll_until pytestmark = pytest.mark.xdist_group("customtemplates") diff --git a/tests/integration/collections/test_data_columns.py b/tests/integration/collections/test_data_columns.py index 8f7819119..daaaa0ce0 100644 --- a/tests/integration/collections/test_data_columns.py +++ b/tests/integration/collections/test_data_columns.py @@ -2,7 +2,7 @@ from albert import Albert from albert.resources.data_columns import DataColumn -from tests.integration.utils.wait import poll_until +from tests.utils.wait import poll_until pytestmark = pytest.mark.xdist_group("datatemplates") diff --git a/tests/integration/collections/test_data_templates.py b/tests/integration/collections/test_data_templates.py index 365f3a2fa..2596f248f 100644 --- a/tests/integration/collections/test_data_templates.py +++ b/tests/integration/collections/test_data_templates.py @@ -25,7 +25,7 @@ from albert.resources.tags import Tag from albert.resources.units import Unit from albert.resources.users import User -from tests.integration.utils.wait import poll_until +from tests.utils.wait import poll_until pytestmark = pytest.mark.xdist_group("datatemplates") diff --git a/tests/integration/collections/test_files.py b/tests/integration/collections/test_files.py index a155343b0..8e3e80ee1 100644 --- a/tests/integration/collections/test_files.py +++ b/tests/integration/collections/test_files.py @@ -7,7 +7,7 @@ from albert import Albert from albert.exceptions import NotFoundError from albert.resources.files import FileNamespace -from tests.integration.utils.wait import poll_until +from tests.utils.wait import poll_until def test_file_round_trip(client: Albert): diff --git a/tests/integration/collections/test_inventory.py b/tests/integration/collections/test_inventory.py index 46f4bb4e5..6cf3a617d 100644 --- a/tests/integration/collections/test_inventory.py +++ b/tests/integration/collections/test_inventory.py @@ -22,7 +22,7 @@ from albert.resources.storage_locations import StorageLocation, StorageLocationFilter from albert.resources.tags import Tag from albert.resources.users import User -from tests.integration.utils.wait import poll_until +from tests.utils.wait import poll_until pytestmark = pytest.mark.xdist_group("inventory") @@ -62,6 +62,8 @@ def filter_seeded(items): return [item for item in items if normalize_inv_id(item.id) in seeded_ids] def scoped_search(*, created_by=None, updated_by=None): + # Every seeded item is created by static_user and matches text=seed_prefix, so + # the complete scoped set is seeded_ids; wait for all of it before comparing. return poll_until( lambda: filter_seeded( list( @@ -72,7 +74,10 @@ def scoped_search(*, created_by=None, updated_by=None): max_items=100, ) ) - ) + ), + predicate=lambda results: ( + {normalize_inv_id(item.id) for item in results} == seeded_ids + ), ) results = poll_until( @@ -101,17 +106,17 @@ def scoped_search(*, created_by=None, updated_by=None): assert test_item.created and test_item.created.at from_created_at = test_item.created.at.date().isoformat() recently_created = poll_until( - lambda: filter_seeded( - list( - client.inventory.search( - text=seed_prefix, - from_created_at=from_created_at, - max_items=100, - ) + lambda: [ + item + for item in client.inventory.search( + text=seed_prefix, + from_created_at=from_created_at, + max_items=100, ) - ) + if normalize_inv_id(item.id) == test_item.id + ] ) - assert test_item.id in {normalize_inv_id(item.id) for item in recently_created} + assert recently_created, "Expected the seeded item in from_created_at search results" hydrated_by_creator = poll_until( lambda: filter_seeded( @@ -130,9 +135,13 @@ def scoped_search(*, created_by=None, updated_by=None): assert facets search_hits = poll_until( - lambda: filter_seeded(list(client.inventory.search(text=test_item.name, max_items=10))) + lambda: [ + item + for item in client.inventory.search(text=test_item.name, max_items=10) + if normalize_inv_id(item.id) == test_item.id + ] ) - hit = next(item for item in search_hits if normalize_inv_id(item.id) == test_item.id) + hit = search_hits[0] assert hit.manufacturer is not None company_name = ( test_item.company.name if isinstance(test_item.company, Company) else test_item.company diff --git a/tests/integration/collections/test_notebooks.py b/tests/integration/collections/test_notebooks.py index 154f4d1fd..bab8dd0b7 100644 --- a/tests/integration/collections/test_notebooks.py +++ b/tests/integration/collections/test_notebooks.py @@ -15,7 +15,7 @@ ) from albert.resources.projects import Project from tests.integration.seeding import generate_notebook_block_seeds, generate_notebook_seeds -from tests.integration.utils.wait import poll_until +from tests.utils.wait import poll_until pytestmark = pytest.mark.xdist_group("projects") @@ -111,18 +111,16 @@ def test_update_block_content_raises_exception(client: Albert, seeded_notebook: def test_search(client: Albert, seed_prefix: str, seeded_notebooks: list[Notebook]): """Test search finds seeded notebook block content scoped to the seed project.""" nb = seeded_notebooks[0] - seeded_ids = {n.id for n in seeded_notebooks} hits = poll_until( lambda: [ hit for hit in client.notebooks.search( text=seed_prefix, project_id=nb.parent_id, max_items=50 ) - if hit.notebook_id in seeded_ids and hit.block_id + if hit.notebook_id == nb.id and hit.block_id ] ) - assert hits, "Expected at least one notebook search hit" - assert any(hit.notebook_id == nb.id for hit in hits) + assert hits, "Expected seeded notebook in search results" def test_get_block_by_id(client: Albert, seeded_notebooks: list[Notebook]): diff --git a/tests/integration/collections/test_parameter_groups.py b/tests/integration/collections/test_parameter_groups.py index 370f34aa1..d56cca922 100644 --- a/tests/integration/collections/test_parameter_groups.py +++ b/tests/integration/collections/test_parameter_groups.py @@ -13,7 +13,7 @@ ) from albert.resources.tags import Tag from albert.resources.units import Unit -from tests.integration.utils.wait import poll_until +from tests.utils.wait import poll_until pytestmark = pytest.mark.xdist_group("datatemplates") @@ -85,7 +85,6 @@ def test_parameter_group_search( parameter = pg.parameters[0].name assert tag and parameter - seeded_ids = {item.id for item in seeded_parameter_groups} hits = poll_until( lambda: [ hit @@ -112,11 +111,11 @@ def test_parameter_group_search( additional_field=["owner", "tags", "createdByName"], max_items=50, ) - if hit.id in seeded_ids + if hit.id == pg.id ] ) assert_valid_parameter_groups(results, ParameterGroupSearchItem) - assert pg.id in {hit.id for hit in results} + assert results[0].id == pg.id def test_hydrate_pg(client: Albert, seed_prefix: str, seeded_parameter_groups): diff --git a/tests/integration/collections/test_parameters.py b/tests/integration/collections/test_parameters.py index 318221180..260726198 100644 --- a/tests/integration/collections/test_parameters.py +++ b/tests/integration/collections/test_parameters.py @@ -4,7 +4,7 @@ from albert.client import Albert from albert.resources.parameters import Parameter -from tests.integration.utils.wait import poll_until +from tests.utils.wait import poll_until pytestmark = pytest.mark.xdist_group("datatemplates") @@ -35,6 +35,7 @@ def test_parameter_get_all_by_ids(client: Albert, seeded_parameters: list[Parame ids = [x.id for x in seeded_parameters] results = poll_until( lambda: [p for p in client.parameters.get_all(ids=ids, max_items=10) if p.id in set(ids)], + predicate=lambda results: len(results) == len(ids), timeout=15.0, interval=1.0, ) @@ -53,6 +54,7 @@ def test_get_by_ids(client: Albert, seeded_parameters: list[Parameter]): ids = [x.id for x in seeded_parameters] results = poll_until( lambda: [p for p in client.parameters.get_by_ids(ids=ids) if p.id in set(ids)], + predicate=lambda results: len(results) == len(ids), timeout=15.0, interval=1.0, ) @@ -74,6 +76,7 @@ def test_get_by_ids_omits_unknown_ids(client: Albert, seeded_parameters: list[Pa for p in client.parameters.get_by_ids(ids=[*known_ids, "PRM0"]) if p.id in set(known_ids) ], + predicate=lambda results: len(results) == len(known_ids), timeout=15.0, interval=1.0, ) diff --git a/tests/integration/collections/test_product_design.py b/tests/integration/collections/test_product_design.py index 403f5a7d6..79cc8b4a4 100644 --- a/tests/integration/collections/test_product_design.py +++ b/tests/integration/collections/test_product_design.py @@ -4,7 +4,7 @@ from albert.resources.inventory import InventoryItem from albert.resources.product_design import UnpackedProductDesign from albert.resources.worksheets import Worksheet -from tests.integration.utils.wait import poll_until +from tests.utils.wait import poll_until pytestmark = pytest.mark.xdist_group("sheets") @@ -16,7 +16,7 @@ def test_search( seeded_worksheet: Worksheet, ): """Test search finds the seeded formula on the product design grid within its project.""" - seeded_ids = {p.id for p in seeded_products} + product = seeded_products[0] hits = poll_until( lambda: [ hit @@ -25,11 +25,11 @@ def test_search( project_id=seeded_worksheet.project_id, max_items=50, ) - if hit.id in seeded_ids + if hit.id == product.id ] ) assert hits, "Expected seeded formula in product design search results" - assert seeded_products[0].id in {hit.id for hit in hits} + assert hits[0].id == product.id def test_get_unpacked(client: Albert, seeded_products: list[InventoryItem]): diff --git a/tests/integration/collections/test_projects.py b/tests/integration/collections/test_projects.py index 46e22f9b7..15bb44783 100644 --- a/tests/integration/collections/test_projects.py +++ b/tests/integration/collections/test_projects.py @@ -8,7 +8,7 @@ from albert.resources.acls import ACL, AccessControlLevel from albert.resources.attachments import Attachment from albert.resources.projects import DocumentSearchItem, Project, ProjectSearchItem -from tests.integration.utils.wait import poll_until +from tests.utils.wait import poll_until pytestmark = pytest.mark.xdist_group("projects") diff --git a/tests/integration/collections/test_reports.py b/tests/integration/collections/test_reports.py index aaf4a3b3b..183603b48 100644 --- a/tests/integration/collections/test_reports.py +++ b/tests/integration/collections/test_reports.py @@ -12,7 +12,7 @@ FullAnalyticalReport, ) from albert.resources.tasks import BaseTask -from tests.integration.utils.wait import poll_until +from tests.utils.wait import poll_until pytestmark = pytest.mark.xdist_group("tasks") @@ -24,7 +24,6 @@ def test_search_reports( ): """Test searching reports finds seeded reports scoped to their project.""" expected = seeded_reports[0] - seeded_ids = {r.id for r in seeded_reports} hits = poll_until( lambda: [ hit @@ -33,13 +32,12 @@ def test_search_reports( project_id=expected.project_id, max_items=50, ) - if hit.id in seeded_ids + if hit.id == expected.id ] ) - hit_ids = {hit.id for hit in hits} - assert expected.id in hit_ids + assert hits, "Expected seeded report in search results" - hit = next(item for item in hits if item.id == expected.id) + hit = hits[0] assert hit.name is not None assert seed_prefix in hit.name assert hit.project_id == expected.project_id diff --git a/tests/integration/collections/test_synthesis.py b/tests/integration/collections/test_synthesis.py index 08f5ec91f..eb59ae8d8 100644 --- a/tests/integration/collections/test_synthesis.py +++ b/tests/integration/collections/test_synthesis.py @@ -5,7 +5,7 @@ from albert import Albert from albert.exceptions import AlbertException from albert.resources.notebooks import Notebook -from tests.integration.utils.wait import poll_until +from tests.utils.wait import poll_until pytestmark = pytest.mark.xdist_group("projects") diff --git a/tests/integration/collections/test_tasks.py b/tests/integration/collections/test_tasks.py index cd7c703fe..78acf91dc 100644 --- a/tests/integration/collections/test_tasks.py +++ b/tests/integration/collections/test_tasks.py @@ -26,7 +26,7 @@ from albert.resources.worker_jobs import WorkerJob from albert.resources.workflows import Workflow from tests.integration.utils.metadata import change_metadata, make_metadata_update_assertions -from tests.integration.utils.wait import poll_until +from tests.utils.wait import poll_until pytestmark = pytest.mark.xdist_group("tasks") diff --git a/tests/integration/collections/test_teams.py b/tests/integration/collections/test_teams.py index d65bdd714..b08efebfc 100644 --- a/tests/integration/collections/test_teams.py +++ b/tests/integration/collections/test_teams.py @@ -7,7 +7,7 @@ from albert.exceptions import AlbertException, NotFoundError from albert.resources.teams import Team, TeamMember from albert.resources.users import User -from tests.integration.utils.wait import poll_until +from tests.utils.wait import poll_until pytestmark = pytest.mark.xdist_group("teams") diff --git a/tests/integration/collections/test_workflows.py b/tests/integration/collections/test_workflows.py index 7184efa53..ace5c516c 100644 --- a/tests/integration/collections/test_workflows.py +++ b/tests/integration/collections/test_workflows.py @@ -4,7 +4,7 @@ from albert.resources.facet import FacetItem from albert.resources.parameters import ParameterCategory from albert.resources.workflows import Workflow, WorkflowSearchItem -from tests.integration.utils.wait import poll_until +from tests.utils.wait import poll_until pytestmark = pytest.mark.xdist_group("tasks") diff --git a/tests/integration/collections/test_worksheet.py b/tests/integration/collections/test_worksheet.py index 4c5245b16..027b3fd62 100644 --- a/tests/integration/collections/test_worksheet.py +++ b/tests/integration/collections/test_worksheet.py @@ -3,7 +3,7 @@ from albert import Albert from albert.resources.inventory import InventoryItem from albert.resources.worksheets import Worksheet -from tests.integration.utils.wait import poll_until +from tests.utils.wait import poll_until pytestmark = pytest.mark.xdist_group("sheets") diff --git a/tests/integration/utils/wait.py b/tests/integration/utils/wait.py deleted file mode 100644 index c84597ab5..000000000 --- a/tests/integration/utils/wait.py +++ /dev/null @@ -1,5 +0,0 @@ -"""Backwards-compatible re-export; the helper lives in ``tests.utils.wait``.""" - -from tests.utils.wait import poll_until - -__all__ = ["poll_until"] From 7e22b9ca338affcf9d186897edb0527fcdf197b5 Mon Sep 17 00:00:00 2001 From: Lenore Kubie Date: Fri, 25 Sep 2026 17:54:58 +0000 Subject: [PATCH 5/7] test(utils): retry transient fetch errors in poll_until A single 5xx/429 from fetch no longer fails the poll; polling continues until the deadline and re-raises the last exception if the deadline expires on an erroring attempt. Cover the behavior plus the last-partial-result-on-timeout path in unit tests. --- tests/unit/utils/test_wait.py | 40 ++++++++++++++++++++++++++++++++++- tests/utils/wait.py | 17 +++++++++++---- 2 files changed, 52 insertions(+), 5 deletions(-) diff --git a/tests/unit/utils/test_wait.py b/tests/unit/utils/test_wait.py index 21c1ec747..93dc8fd4d 100644 --- a/tests/unit/utils/test_wait.py +++ b/tests/unit/utils/test_wait.py @@ -1,3 +1,5 @@ +import pytest + from tests.utils.wait import poll_until @@ -20,5 +22,41 @@ def test_poll_until_predicate_waits_for_satisfaction(): def test_poll_until_returns_last_result_on_timeout(): """Test that an unsatisfied predicate returns the last result after the timeout.""" - result = poll_until(lambda: [], timeout=0.01, interval=0.005) + result = poll_until(lambda: [], predicate=lambda result: False, timeout=0.01, interval=0.005) assert result == [] + + +def test_poll_until_returns_last_partial_result_on_timeout(): + """Test that a never-satisfied predicate returns the latest partial result.""" + partials = [["a"], ["a", "b"], ["a", "b", "c"]] + calls = iter(partials) + result = poll_until( + lambda: next(calls, partials[-1]), + predicate=lambda result: len(result) == 4, + timeout=0.05, + interval=0.01, + ) + assert result == ["a", "b", "c"] + + +def test_poll_until_recovers_from_transient_fetch_error(): + """Test that a one-off fetch exception does not fail the poll.""" + outcomes = iter([RuntimeError("transient 503"), ["a"]]) + + def fetch(): + outcome = next(outcomes) + if isinstance(outcome, Exception): + raise outcome + return outcome + + assert poll_until(fetch, interval=0) == ["a"] + + +def test_poll_until_reraises_fetch_error_after_timeout(): + """Test that a fetch that always raises propagates the exception after the timeout.""" + + def fetch(): + raise RuntimeError("persistent 503") + + with pytest.raises(RuntimeError, match="persistent 503"): + poll_until(fetch, timeout=0.01, interval=0.005) diff --git a/tests/utils/wait.py b/tests/utils/wait.py index f8ec7af7b..d9f3b9036 100644 --- a/tests/utils/wait.py +++ b/tests/utils/wait.py @@ -18,11 +18,20 @@ def poll_until( Pass a ``predicate`` when the assertion needs the complete expected set: a non-empty but partially indexed result satisfies the default check while remaining items are still becoming visible. + + Exceptions raised by ``fetch`` (for example a transient 5xx or 429) do not + end polling: the poll retries until the deadline and re-raises the last + exception if the deadline expires on an erroring attempt. """ deadline = time.monotonic() + timeout while True: - result = fetch() - ready = predicate(result) if predicate is not None else bool(result) - if ready or time.monotonic() >= deadline: - return result + try: + result = fetch() + except Exception: + if time.monotonic() >= deadline: + raise + else: + ready = predicate(result) if predicate is not None else bool(result) + if ready or time.monotonic() >= deadline: + return result time.sleep(interval) From 968869a8a69d81754601c88da2e4166beea07815 Mon Sep 17 00:00:00 2001 From: Lenore Kubie Date: Fri, 25 Sep 2026 17:54:58 +0000 Subject: [PATCH 6/7] docs(testing): state the poll stop rule and honest timeout guidance Whatever stops the polling must imply the assertion that follows. A timeout usually means indexing lag, but a repeat failure means the predicate or the code is wrong. --- tests/integration/TESTING.md | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/tests/integration/TESTING.md b/tests/integration/TESTING.md index 5a3435e02..21b7212f9 100644 --- a/tests/integration/TESTING.md +++ b/tests/integration/TESTING.md @@ -110,10 +110,13 @@ def test_hydrate_project(client: Albert, seed_prefix: str, seeded_projects: list `poll_until` stops at the first **non-empty** result by default. Our NoSQL store plus search index is eventually consistent, so freshly seeded items become visible one by one -and a partial page is normal during indexing. If the assertion needs the **complete** -expected set (exact-equality asserts against your fixture's ids), pass a `predicate` so -polling continues until the set is whole; without it the test flakes the moment one item -indexes before the rest: +and a partial page is normal during indexing. One rule decides how to poll: **whatever +stops the polling must imply the assertion that follows.** When asserting on a specific +item, filter the fetched page to that item's id, so a non-empty result means the item is +visible. When the assertion needs the **complete** expected set (exact-equality or +exact-count asserts against your fixture's ids), pass a `predicate` so polling continues +until the set is whole; without it the test flakes the moment one item indexes before +the rest: ```python expected_ids = {f"INV{lot.inventory_id}" for lot in seeded_lots if ...} @@ -125,8 +128,8 @@ assert {f"INV{p.id}" for p in results} == expected_ids ``` Even with a predicate, a rare timeout is expected behavior of an eventually consistent -index. Treat it as a re-run, not a code change — but write the predicate to describe the -full expectation so only genuine lag can trip it. +index. A timeout usually means indexing lag, so re-run once. A repeat failure means the +predicate or the code under test is wrong; investigate it like any other failure. Also: From 2367409dd5a47f93d530928c07c88c448d28479b Mon Sep 17 00:00:00 2001 From: "claude[bot]" <41898282+claude[bot]@users.noreply.github.com> Date: Mon, 28 Sep 2026 07:54:45 +0000 Subject: [PATCH 7/7] fix(tests): narrow poll_until exception handling to AlbertServerError - `except Exception` in poll_until stalled for the full timeout on programming errors (AttributeError, ValidationError) and 4xx errors that should propagate immediately; narrow to AlbertServerError (5xx) - Remove "429" from the docstring since 429 maps to AlbertClientError and is not retried by this helper - Update unit tests: use InternalServerError instead of RuntimeError in transient-error tests; add a new test asserting non-server errors propagate immediately without polling - Update tests/unit/TESTING.md: remove poll_until from the banned tests/integration/ import list; note it lives in tests/utils/ Co-authored-by: Prasad <212564756+prasad-albert@users.noreply.github.com> Co-Authored-By: Claude Sonnet 4.6 --- tests/unit/TESTING.md | 2 +- tests/unit/utils/test_wait.py | 42 ++++++++++++++++++++++++++++++----- tests/utils/wait.py | 10 +++++---- 3 files changed, 44 insertions(+), 10 deletions(-) diff --git a/tests/unit/TESTING.md b/tests/unit/TESTING.md index 56e5f4abf..e0d4396f9 100644 --- a/tests/unit/TESTING.md +++ b/tests/unit/TESTING.md @@ -32,7 +32,7 @@ The conftest guards enforce the first two mechanically: - **Open a network connection.** `_block_network` raises on any real `connect`. - **Read credentials or `.env`.** `_strip_albert_env` removes every `ALBERT_*` variable. - **Import from `tests/integration/`** (`client`, `seeded_*`, `static_*`, `seed_prefix`, - `poll_until`, `seeding`). + `seeding`). Shared, network-free helpers live in `tests/utils/` (e.g. `poll_until`). - **Encode assumed server behavior.** A fake response may be shaped like a real one, but the assertion is about what the SDK sends or how it reacts, never "the API returns X". - **Assert trivia.** That a Pydantic field exists, that a default is `None`, that a diff --git a/tests/unit/utils/test_wait.py b/tests/unit/utils/test_wait.py index 93dc8fd4d..9cd6a7406 100644 --- a/tests/unit/utils/test_wait.py +++ b/tests/unit/utils/test_wait.py @@ -1,8 +1,26 @@ +import json + import pytest +import requests +from albert.exceptions import InternalServerError from tests.utils.wait import poll_until +def _make_server_error() -> InternalServerError: + req = requests.PreparedRequest() + req.method = "GET" + req.url = "https://example.com/api/v3/x" + req.body = None + resp = requests.Response() + resp.status_code = 500 + resp.reason = "Internal Server Error" + resp.request = req + resp._content = json.dumps({"errors": "transient failure"}).encode() + resp.encoding = "utf-8" + return InternalServerError(resp) + + def test_poll_until_returns_first_non_empty_by_default(): """Test that polling stops at the first non-empty result with no predicate.""" calls = iter([[], ["a"], ["a", "b"]]) @@ -40,8 +58,8 @@ def test_poll_until_returns_last_partial_result_on_timeout(): def test_poll_until_recovers_from_transient_fetch_error(): - """Test that a one-off fetch exception does not fail the poll.""" - outcomes = iter([RuntimeError("transient 503"), ["a"]]) + """Test that a one-off AlbertServerError does not fail the poll.""" + outcomes = iter([_make_server_error(), ["a"]]) def fetch(): outcome = next(outcomes) @@ -53,10 +71,24 @@ def fetch(): def test_poll_until_reraises_fetch_error_after_timeout(): - """Test that a fetch that always raises propagates the exception after the timeout.""" + """Test that a fetch that always raises AlbertServerError propagates after the timeout.""" def fetch(): - raise RuntimeError("persistent 503") + raise _make_server_error() - with pytest.raises(RuntimeError, match="persistent 503"): + with pytest.raises(InternalServerError): poll_until(fetch, timeout=0.01, interval=0.005) + + +def test_poll_until_does_not_catch_non_server_errors(): + """Test that non-AlbertServerError exceptions propagate immediately without polling.""" + calls = [0] + + def fetch(): + calls[0] += 1 + raise AttributeError("lambda typo") + + with pytest.raises(AttributeError, match="lambda typo"): + poll_until(fetch, timeout=30.0, interval=0) + + assert calls[0] == 1 diff --git a/tests/utils/wait.py b/tests/utils/wait.py index d9f3b9036..ac965b877 100644 --- a/tests/utils/wait.py +++ b/tests/utils/wait.py @@ -1,6 +1,8 @@ import time from collections.abc import Callable +from albert.exceptions import AlbertServerError + def poll_until( fetch: Callable[[], list], @@ -19,15 +21,15 @@ def poll_until( non-empty but partially indexed result satisfies the default check while remaining items are still becoming visible. - Exceptions raised by ``fetch`` (for example a transient 5xx or 429) do not - end polling: the poll retries until the deadline and re-raises the last - exception if the deadline expires on an erroring attempt. + ``AlbertServerError`` exceptions raised by ``fetch`` (for example a transient + 5xx) do not end polling: the poll retries until the deadline and re-raises the + last exception if the deadline expires on an erroring attempt. """ deadline = time.monotonic() + timeout while True: try: result = fetch() - except Exception: + except AlbertServerError: if time.monotonic() >= deadline: raise else: