Skip to content

Fix CI to properly report test failures and refactor test fixtures#24

Merged
jason810496 merged 3 commits intofeature/add-transaction-classfrom
copilot/sub-pr-21-another-one
Jan 3, 2026
Merged

Fix CI to properly report test failures and refactor test fixtures#24
jason810496 merged 3 commits intofeature/add-transaction-classfrom
copilot/sub-pr-21-another-one

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Jan 3, 2026

Description

This PR addresses two issues with the CI and test setup:

1. CI Configuration Fix

The GitHub Actions workflow was masking test failures with continue-on-error: true, causing all integration tests to show green regardless of actual test results.

Changes:

  • Removed continue-on-error: true from test execution step in .github/workflows/codecov.yml to surface failures
  • Added if: always() to coverage upload step to preserve coverage artifacts for debugging failed runs

The CI now correctly fails when tests fail while still collecting coverage data and performing cleanup.

2. Test Fixture Refactoring

Refactored the test setup to remove dynamically generated lazy fixtures (pgmq_deps) and use dynamic parametrization based on the --driver CLI option.

Changes:

  • Removed @pgmq_deps decorator and lazy fixture approach from test files
  • Implemented pytest_generate_tests hook in conftest.py for dynamic parametrization
  • Updated all test files to use pgmq_all_variants fixture
  • Simplified pytest_collection_modifyitems by removing complex skip logic

Benefits:

  • Tests now generate only fixtures for the specified driver (5 sync fixtures for sync drivers, 3 async fixtures for async drivers) instead of generating all 8 and skipping most
  • Faster test execution with no unnecessary fixture generation
  • Cleaner, more maintainable test setup without dependency on pytest-lazy-fixture functionality

Status

  • In progress
  • Ready for review
  • Done

Checklist

  • Read the Contributing Guide
  • Passes tests
  • Linted ( we use pre-commit with ruff )
  • Updated documentation

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI changed the title [WIP] Add op module for transaction Fix CI to properly report test failures Jan 3, 2026
Copilot AI requested a review from jason810496 January 3, 2026 05:52
Copy link
Copy Markdown
Owner

@jason810496 jason810496 left a comment

Choose a reason for hiding this comment

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

@copilot

We should refactor our test setup by removing dynamic generate lazy fixture (pgmq_deps), since we just need to run the test for specific driver instead of having all eumerate of test cases and skip the rest of driver.

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Jan 3, 2026

@copilot

We should refactor our test setup by removing dynamic generate lazy fixture (pgmq_deps), since we just need to run the test for specific driver instead of having all eumerate of test case...

Refactored the test setup in commit 8404337. The changes:

  1. Removed dynamic lazy fixture generation (pgmq_deps)
  2. Replaced with pytest_generate_tests hook that dynamically parametrizes tests based on --driver CLI option
  3. When --driver=pg8000 is specified, only 5 sync fixtures run (pgmq_by_dsn, pgmq_by_engine, etc.)
  4. When --driver=asyncpg is specified, only 3 async fixtures run (pgmq_by_async_dsn, etc.)
  5. Removed unnecessary skip logic from pytest_collection_modifyitems

Tests now run only for the specified driver instead of generating all combinations and skipping most of them. The pytest-lazy-fixture dependency is still in pyproject.toml but no longer used in the code.

Copilot AI changed the title Fix CI to properly report test failures Fix CI to properly report test failures and refactor test fixtures Jan 3, 2026
Copilot AI requested a review from jason810496 January 3, 2026 06:09
@jason810496 jason810496 marked this pull request as ready for review January 3, 2026 06:15
@jason810496 jason810496 merged commit 698e8ec into feature/add-transaction-class Jan 3, 2026
jason810496 added a commit that referenced this pull request Jan 3, 2026
* Add transation friendly op staticmethod class

* Refactor PGMQueue methods to utilize PGMQOperation for extension checks, queue creation, validation, and message handling

- Replaced direct SQL execution with calls to PGMQOperation methods for checking pgmq and pg_partman extensions.
- Consolidated queue creation, validation, and message sending methods to use PGMQOperation for both sync and async operations.
- Updated method signatures to include optional session and commit parameters for better session management.
- Removed redundant sync and async methods, streamlining the codebase and improving maintainability.

* Add empty test_operation

* Move _validate_partition_interval to operations

* Address code review feedback: fix SQL injection, QueueMetrics bug, reduce boilerplate (#22)

* Initial plan

* Fix SQL injection vulnerability and QueueMetrics bug in operation.py

Co-authored-by: jason810496 <[email protected]>

* Refactor queue.py to reduce boilerplate with _execute_operation helper

Co-authored-by: jason810496 <[email protected]>

* Add comprehensive tests for PGMQOperation class

Co-authored-by: jason810496 <[email protected]>

* Fix json import and send_batch parameter handling

Co-authored-by: jason810496 <[email protected]>

* Fix test exception handling to be more specific

Co-authored-by: jason810496 <[email protected]>

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: jason810496 <[email protected]>

* Improve test coverage for operation module and fix SQL parameterization bugs (#23)

* Initial plan

* Add comprehensive test coverage for PGMQOperation methods

- Added tests for delete_batch (sync and async)
- Added tests for archive_batch (sync and async)
- Added tests for purge (sync and async)
- Added tests for read_with_poll (sync and async)
- Added tests for drop_queue (sync and async)
- Added tests for check_pg_partman_ext (sync and async)
- Added tests for create_partitioned_queue (sync and async)
- Added tests for time-based partitioned queues

Note: Tests currently fail due to SQL parameterization bug in operation.py where mixed parameter styles (:param and %(param)s) cause syntax errors. This was introduced in commit 849a8f1.

Co-authored-by: jason810496 <[email protected]>

* Fix SQL parameterization bugs and improve partition interval validation

- Fixed SQL injection fix by using CAST(:param AS type) instead of :param::type
- Fixed delete_batch and archive_batch to return message IDs instead of boolean
- Improved partition interval validation to accept numeric strings
- Added validation to sync create_partitioned_queue for consistency
- Fixed queue name length in partitioned queue tests
- All new tests now pass successfully

Co-authored-by: jason810496 <[email protected]>

* Fix send_batch array formatting and validate_queue_name test

- Fixed send_batch to properly escape JSON in PostgreSQL array literal format
- Fixed test_validate_queue_name to handle both ProgrammingError and InternalError
- All 103 operation tests now pass successfully

Co-authored-by: jason810496 <[email protected]>

* Address code review feedback

- Move imports to top of file (time, InternalError)
- Add documentation comment explaining array literal escaping approach
- All tests still passing

Co-authored-by: jason810496 <[email protected]>

* Improve import ordering and documentation

- Fix import grouping (stdlib imports together)
- Enhance documentation for array literal escaping with security notes
- All tests still passing

Co-authored-by: jason810496 <[email protected]>

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: jason810496 <[email protected]>

* Fix PGMQueue with _execute_operation for rest of the methods

* Fix Jsonb format

* 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]>

* Fix JSONB parameter binding to prevent psycopg2 dict adaptation errors (#25)

* Initial plan

* Fix JSONB handling with bindparams for send and send_batch operations

Co-authored-by: jason810496 <[email protected]>

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: jason810496 <[email protected]>

* Fix partition interval validation for negative numeric strings (#26)

* Initial plan

* Add read_archive methods to PGMQOperation and PGMQueue

Co-authored-by: jason810496 <[email protected]>

* Fix partition interval validation for negative numeric strings

Co-authored-by: jason810496 <[email protected]>

* Apply ruff formatting fixes

Co-authored-by: jason810496 <[email protected]>

* Remove read_archive methods and tests per spec clarification

Co-authored-by: jason810496 <[email protected]>

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: jason810496 <[email protected]>

* Add pytest-asyncio

* Fix ambiguous column references in delete and archive SQL statements (#27)

* Initial plan

* Fix ambiguous column references in delete_batch and archive_batch

Co-authored-by: jason810496 <[email protected]>

* Add aliases to delete and archive statements to avoid ambiguity

Co-authored-by: jason810496 <[email protected]>

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: jason810496 <[email protected]>

* Fix ambiguous error manually

Fix type hint

* Fix bindparams usage

Fix typ hint

* Try fix text to varchar wrong cast error, remove drop_archive test

* Fix text type casting

* Fix sqlalchemy.exc.ArgumentError

* Fix pgmq.delete_bactch and pgmq.archive_batch

* Fix missing pgmq_all_variants fixture import in test_queue.py (#28)

* Initial plan

* Fix pgmq_all_variants fixture import in test_queue.py

Co-authored-by: jason810496 <[email protected]>

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: jason810496 <[email protected]>

* Improve test coverage for `op` module based on review feedback (#29)

* Initial plan

* Add tests for _execute_operation with provided session and read_with_poll without vt

Co-authored-by: jason810496 <[email protected]>

* Add test for async _execute_operation path when session is None

Co-authored-by: jason810496 <[email protected]>

* Remove fragile line number references from test comments

Co-authored-by: jason810496 <[email protected]>

* Clarify comment in test_read_with_poll_without_vt

Co-authored-by: jason810496 <[email protected]>

* Address final code review feedback: improve test structure and add cleanup verification

Co-authored-by: jason810496 <[email protected]>

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: jason810496 <[email protected]>

---------

Co-authored-by: Copilot <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants