Skip to content

Commit 698e8ec

Browse files
Copilotjason810496
Copilot
and
jason810496
authored
Fix CI to properly report test failures and refactor test fixtures (#24)
* Initial plan * Fix CI configuration to properly report test failures Co-authored-by: jason810496 <[email protected]> * Refactor test fixtures to remove lazy fixtures and use dynamic parametrization Co-authored-by: jason810496 <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: jason810496 <[email protected]>
1 parent ec8bfd8 commit 698e8ec

5 files changed

Lines changed: 104 additions & 143 deletions

File tree

‎.github/workflows/codecov.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,8 @@ jobs:
5959
export COVERAGE_FILE=".coverage.py${{ matrix.python-version }}.${{ matrix.driver }}"
6060
6161
uv run pytest tests --driver=${{ matrix.driver }} --db-name=${DB_NAME} --cov=pgmq_sqlalchemy.queue --cov-report=xml:coverage-py${{ matrix.python-version }}-${{ matrix.driver }}.xml
62-
continue-on-error: true
6362
- name: Upload coverage artifact
63+
if: always()
6464
uses: actions/upload-artifact@v4
6565
with:
6666
name: coverage-py${{ matrix.python-version }}-${{ matrix.driver }}

‎tests/conftest.py‎

Lines changed: 51 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -29,59 +29,47 @@ def pytest_addoption(parser):
2929
)
3030

3131

32-
def pytest_collection_modifyitems(config, items):
33-
"""Modify test collection to skip tests not matching the --driver option."""
34-
driver_from_cli = config.getoption("--driver")
35-
36-
if not driver_from_cli:
37-
# No driver specified, run all tests
38-
return
39-
40-
# Determine if the specified driver is sync or async
41-
is_async_driver = driver_from_cli in ASYNC_DRIVERS
42-
is_sync_driver = driver_from_cli in SYNC_DRIVERS
43-
44-
if not is_async_driver and not is_sync_driver:
45-
# Invalid driver
46-
return
47-
48-
# Filter out tests that don't match the specified driver
49-
skip_marker = pytest.mark.skip(reason=f"Test uses different driver (--driver={driver_from_cli} specified)")
50-
51-
for item in items:
52-
# Parse the test name to extract driver info
53-
# Format is usually: test_name[fixture_name-driver_name]
54-
item_id = item.nodeid
55-
56-
# Check if the test has a specific driver in its ID
57-
# Extract driver name from test ID (e.g., test_name[pgmq_by_dsn-psycopg2])
58-
if '[' in item_id and ']' in item_id:
59-
# Extract the part between brackets
60-
bracket_content = item_id[item_id.find('[')+1:item_id.find(']')]
61-
62-
# Check for async fixtures by name (more precise than string matching)
63-
is_async_test = any(async_fixture in bracket_content for async_fixture in ASYNC_FIXTURE_NAMES)
64-
65-
# Skip async tests if sync driver specified
66-
if is_sync_driver and is_async_test:
67-
item.add_marker(skip_marker)
68-
continue
69-
70-
# Skip sync tests if async driver specified
71-
if is_async_driver and not is_async_test:
72-
item.add_marker(skip_marker)
73-
continue
74-
75-
# Check if any known driver is in the bracket content
76-
# Sort drivers by length (descending) to match longer names first (e.g., psycopg2cffi before psycopg2)
77-
sorted_drivers = sorted(SYNC_DRIVERS + ASYNC_DRIVERS, key=len, reverse=True)
78-
for driver in sorted_drivers:
79-
if f"-{driver}]" in item_id or f"-{driver}-" in bracket_content:
80-
# This test is for a specific driver
81-
if driver != driver_from_cli:
82-
# Skip if it doesn't match the CLI driver
83-
item.add_marker(skip_marker)
84-
break
32+
def pytest_generate_tests(metafunc):
33+
"""
34+
Dynamically generate test parametrization based on CLI options.
35+
36+
This allows us to parametrize fixtures based on the --driver option.
37+
"""
38+
if "pgmq_all_variants" in metafunc.fixturenames:
39+
driver_from_cli = metafunc.config.getoption("--driver")
40+
41+
# Define sync and async fixture variants
42+
sync_fixtures = [
43+
'pgmq_by_dsn',
44+
'pgmq_by_engine',
45+
'pgmq_by_session_maker',
46+
'pgmq_by_dsn_and_engine',
47+
'pgmq_by_dsn_and_session_maker',
48+
]
49+
50+
async_fixtures = [
51+
'pgmq_by_async_dsn',
52+
'pgmq_by_async_engine',
53+
'pgmq_by_async_session_maker',
54+
]
55+
56+
# Determine which fixtures to use
57+
if not driver_from_cli:
58+
# No driver specified, use all fixtures
59+
fixture_params = sync_fixtures + async_fixtures
60+
elif driver_from_cli in ASYNC_DRIVERS:
61+
# Async driver specified
62+
fixture_params = async_fixtures
63+
else:
64+
# Sync driver specified
65+
fixture_params = sync_fixtures
66+
67+
# Parametrize the test
68+
metafunc.parametrize(
69+
"pgmq_all_variants",
70+
fixture_params,
71+
indirect=True
72+
)
8573

8674

8775
@pytest.fixture(scope="module")
@@ -113,7 +101,7 @@ def get_sa_db(request):
113101
return os.getenv("SQLALCHEMY_DB", "postgres")
114102

115103

116-
@pytest.fixture(scope="function", params=SYNC_DRIVERS)
104+
@pytest.fixture(scope="function")
117105
def get_dsn(
118106
request: FixtureRequest,
119107
get_sa_host,
@@ -122,19 +110,20 @@ def get_dsn(
122110
get_sa_password,
123111
get_sa_db,
124112
):
125-
"""Get DSN for sync drivers."""
113+
"""Get DSN for sync drivers based on CLI option."""
126114
driver_from_cli = request.config.getoption("--driver")
127115

128-
# Use CLI driver if specified, otherwise use parametrized driver
116+
# Use CLI driver if specified and it's a sync driver
129117
if driver_from_cli and driver_from_cli in SYNC_DRIVERS:
130118
driver = driver_from_cli
131119
else:
132-
driver = request.param
120+
# Default to first sync driver if no CLI option or invalid
121+
driver = SYNC_DRIVERS[0]
133122

134123
return f"postgresql+{driver}://{get_sa_user}:{get_sa_password}@{get_sa_host}:{get_sa_port}/{get_sa_db}"
135124

136125

137-
@pytest.fixture(scope="function", params=ASYNC_DRIVERS)
126+
@pytest.fixture(scope="function")
138127
def get_async_dsn(
139128
request: FixtureRequest,
140129
get_sa_host,
@@ -143,14 +132,15 @@ def get_async_dsn(
143132
get_sa_password,
144133
get_sa_db,
145134
):
146-
"""Get DSN for async drivers."""
135+
"""Get DSN for async drivers based on CLI option."""
147136
driver_from_cli = request.config.getoption("--driver")
148137

149-
# Use CLI driver if specified, otherwise use parametrized driver
138+
# Use CLI driver if specified and it's an async driver
150139
if driver_from_cli and driver_from_cli in ASYNC_DRIVERS:
151140
driver = driver_from_cli
152141
else:
153-
driver = request.param
142+
# Default to first async driver if no CLI option or invalid
143+
driver = ASYNC_DRIVERS[0]
154144

155145
return f"postgresql+{driver}://{get_sa_user}:{get_sa_password}@{get_sa_host}:{get_sa_port}/{get_sa_db}"
156146

‎tests/fixture_deps.py‎

Lines changed: 28 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -6,64 +6,47 @@
66
from pgmq_sqlalchemy import PGMQueue
77
from tests._utils import check_queue_exists
88

9-
LAZY_FIXTURES = [
10-
pytest.lazy_fixture("pgmq_by_dsn"),
11-
pytest.lazy_fixture("pgmq_by_async_dsn"),
12-
pytest.lazy_fixture("pgmq_by_engine"),
13-
pytest.lazy_fixture("pgmq_by_async_engine"),
14-
pytest.lazy_fixture("pgmq_by_session_maker"),
15-
pytest.lazy_fixture("pgmq_by_async_session_maker"),
16-
pytest.lazy_fixture("pgmq_by_dsn_and_engine"),
17-
pytest.lazy_fixture("pgmq_by_dsn_and_session_maker"),
18-
]
19-
20-
pgmq_deps = pytest.mark.parametrize(
21-
"pgmq_fixture",
22-
LAZY_FIXTURES,
23-
)
24-
"""
25-
Decorator that allows a test function to receive a PGMQueue instance as a parameter.
26-
27-
Usage:
28-
29-
```
30-
from tests.fixture_deps import pgmq_deps
31-
32-
@pgmq_deps
33-
def test_create_queue(pgmq_fixture,db_session):
34-
pgmq:PGMQueue = pgmq_fixture
35-
# test code here
36-
```
9+
PGMQ_WITH_QUEUE = Tuple[PGMQueue, str]
3710

38-
Note:
39-
`pytest` version should < 8.0.0,
40-
or `pytest-lazy-fixture` will not work
41-
ref: https://github.com/TvoroG/pytest-lazy-fixture/issues/65
42-
"""
4311

44-
PGMQ_WITH_QUEUE = Tuple[PGMQueue, str]
12+
@pytest.fixture(scope="function")
13+
def pgmq_all_variants(request: pytest.FixtureRequest) -> PGMQueue:
14+
"""
15+
Fixture that parametrizes tests across all appropriate PGMQueue initialization methods.
16+
17+
When --driver is specified, only fixtures matching that driver type (sync/async) are used.
18+
Without --driver, all fixtures are used.
19+
20+
The parametrization is handled by pytest_generate_tests in conftest.py.
21+
22+
Usage:
23+
def test_something(pgmq_all_variants):
24+
pgmq: PGMQueue = pgmq_all_variants
25+
# test code here
26+
"""
27+
# The param is set by pytest_generate_tests via indirect parametrization
28+
return request.getfixturevalue(request.param)
4529

4630

47-
@pytest.fixture(scope="function", params=LAZY_FIXTURES)
48-
def pgmq_setup_teardown(request: pytest.FixtureRequest, db_session) -> PGMQ_WITH_QUEUE:
31+
@pytest.fixture(scope="function")
32+
def pgmq_setup_teardown(pgmq_all_variants: PGMQueue, db_session) -> PGMQ_WITH_QUEUE:
4933
"""
5034
Fixture that provides a PGMQueue instance with a unique temporary queue with setup and teardown.
5135
5236
Args:
53-
request (pytest.FixtureRequest): The pytest fixture request object.
37+
pgmq_all_variants (PGMQueue): The PGMQueue instance (parametrized across all variants).
5438
db_session (sqlalchemy.orm.Session): The SQLAlchemy session object.
5539
5640
Yields:
5741
tuple[PGMQueue,str]: A tuple containing the PGMQueue instance and the name of the temporary queue.
5842
5943
Usage:
60-
@pgmq_setup_teardown
6144
def test_something(pgmq_setup_teardown):
6245
pgmq, queue_name = pgmq_setup_teardown
6346
# test code here
6447
6548
"""
66-
pgmq = request.param
49+
pgmq = pgmq_all_variants
6750
queue_name = f"test_queue_{uuid.uuid4().hex}"
6851
assert check_queue_exists(db_session, queue_name) is False
6952
pgmq.create_queue(queue_name)
@@ -73,28 +56,27 @@ def test_something(pgmq_setup_teardown):
7356
assert check_queue_exists(db_session, queue_name) is False
7457

7558

76-
@pytest.fixture(scope="function", params=LAZY_FIXTURES)
59+
@pytest.fixture(scope="function")
7760
def pgmq_partitioned_setup_teardown(
78-
request: pytest.FixtureRequest, db_session
61+
pgmq_all_variants: PGMQueue, db_session
7962
) -> PGMQ_WITH_QUEUE:
8063
"""
8164
Fixture that provides a PGMQueue instance with a unique temporary partitioned queue with setup and teardown.
8265
8366
Args:
84-
request (pytest.FixtureRequest): The pytest fixture request object.
67+
pgmq_all_variants (PGMQueue): The PGMQueue instance (parametrized across all variants).
8568
db_session (sqlalchemy.orm.Session): The SQLAlchemy session object.
8669
8770
Yields:
8871
tuple[PGMQueue,str]: A tuple containing the PGMQueue instance and the name of the temporary queue.
8972
9073
Usage:
91-
@pgmq_setup_teardown
92-
def test_something(pgmq_setup_teardown):
93-
pgmq, queue_name = pgmq_setup_teardown
74+
def test_something(pgmq_partitioned_setup_teardown):
75+
pgmq, queue_name = pgmq_partitioned_setup_teardown
9476
# test code here
9577
9678
"""
97-
pgmq: PGMQueue = request.param
79+
pgmq: PGMQueue = pgmq_all_variants
9880
queue_name = f"test_queue_{uuid.uuid4().hex}"
9981
assert check_queue_exists(db_session, queue_name) is False
10082
pgmq.create_partitioned_queue(queue_name)

‎tests/test_construct_pgmq.py‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
11
import pytest
22
from pgmq_sqlalchemy import PGMQueue
33

4-
from tests.fixture_deps import pgmq_deps
4+
from tests.fixture_deps import pgmq_all_variants
55

66

7-
@pgmq_deps
8-
def test_construct_pgmq(pgmq_fixture):
9-
pgmq: PGMQueue = pgmq_fixture
7+
def test_construct_pgmq(pgmq_all_variants):
8+
pgmq: PGMQueue = pgmq_all_variants
109
assert pgmq is not None
1110

1211

0 commit comments

Comments
 (0)