fix: resolve integration test transaction lifecycle issues#125
Merged
JacobCoffee merged 5 commits intomainfrom Nov 23, 2025
Merged
fix: resolve integration test transaction lifecycle issues#125JacobCoffee merged 5 commits intomainfrom
JacobCoffee merged 5 commits intomainfrom
Conversation
The db_session fixture uses a transaction context manager that auto-rolls back. Calling commit() within the transaction context closes the transaction and breaks subsequent queries. Fixed by relying on flush() for within-transaction visibility. - Remove commit() call after delete operation - Database queries see flushed changes within transaction - Test verifies cascade delete behavior correctly Fixes test: TestFullGuildLifecycleWithAllConfigs::test_full_guild_with_all_configs_lifecycle
Remove commit() call that closes transaction prematurely. Flush() provides sufficient visibility for verification queries within the same transaction context. Fixes test: TestFullGuildLifecycleWithAllConfigs::test_guild_with_multiple_sotags_and_users
Database integrity constraints are enforced during flush(), not just on commit(). Remove unnecessary commit() and rollback() calls - the fixture handles transaction cleanup. Fixes test: TestDatabaseIntegrity::test_duplicate_guild_id_rejected
Cascade delete behavior works within transactions after flush(). Remove commit() calls that close transaction prematurely. Fixes test: TestDatabaseIntegrity::test_cascade_delete_all_related_configs
The guilds list endpoint uses 'limit' and 'offset' parameters, not 'pageSize' and 'currentPage'. Update test to match API contract. This test was sending pageSize=5 but the endpoint ignored it and used the default limit of 20, causing the assertion to fail. Also remove unnecessary commit() call from test setup. Fixes test: TestAPIPaginationConsistency::test_pagination_offset_and_limit
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Sorry @JacobCoffee, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
|
🚅 Deployed to the byte-pr-125 environment in byte
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes 5 failing integration tests in
test_api_endpoints.pyby correcting transaction handling patterns.Root Cause
The
db_sessionfixture uses a transaction context manager that auto-rolls back:When tests called
await db_session.commit(), it closed the transaction prematurely while still inside the context manager, causing subsequent queries to fail with:Changes
Fixed 5 tests by removing unnecessary
commit()calls and relying onflush()for within-transaction visibility:pageSizetolimitparameter + removed commit()Testing
make cipassingTechnical Details
db_sessionfixture (auto-rollback ensures isolation)flush(), not justcommit()flush()within transactionsCommits
0e22872- fix: remove transaction commit from full guild lifecycle testc1fe78a- fix: remove transaction commit from multiple SO tags test4e74d54- fix: remove transaction commit from duplicate guild ID testeb9cd45- fix: remove transaction commits from cascade delete test05a848a- fix: use correct pagination parameter in list guilds test🤖 Generated with Claude Code
Co-Authored-By: Claude noreply@anthropic.com