-
Notifications
You must be signed in to change notification settings - Fork 78
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix sqlalchemy query error of test_postgres.py (Fix #538) #539
Conversation
This fixs issue #538 and pytest on github was successful. However, the error occurs in the process of "test (macos-latest, 3.6)";
Can I fix it myself? Please support. |
Please fix it. Thanks!!! |
@YasushiMiyata I just re-ran the jobs and it appears to have passed. I don't think that error was caused by you. |
@senwu , @lukehsiao Thank you for checking and your comments. I also found my request passed. |
tests/test_postgres.py
Outdated
@@ -116,7 +116,8 @@ def test_cand_gen_cascading_delete(database_session): | |||
|
|||
# Test that deletion of a Candidate does not delete the Mention | |||
x = session.query(PartTemp).first() | |||
session.query(PartTemp).filter_by(id=x.id).delete(synchronize_session="fetch") | |||
d = session.query(PartTemp).filter_by(id=x.id).first() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: can we call this variable candidate
or cand
rather than d
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think candidate
is better than d
. I correct it.
As a side note, I was intended that d
denotes delete row
of PartTemp
table. However, this d
also means delete candidate
. So, I update d
to candidate
. Thank you for your suggestion!
Codecov Report
@@ Coverage Diff @@
## master #539 +/- ##
=======================================
Coverage 86.02% 86.02%
=======================================
Files 92 92
Lines 4773 4773
Branches 899 899
=======================================
Hits 4106 4106
Misses 476 476
Partials 191 191
Flags with carried forward coverage won't be shown. Click here to find out more. |
Description of the problems or issues
Is your pull request related to a problem? Please describe.
See #538
Does your pull request fix any issue.
See #538
Description of the proposed changes
Fix sqlalchemy query building on
tests/test_postgres.py::test_cand_gen_cascading_delete
.Before this fixing, this test code tried delete query for deleting one data from PartTemp table, but sqlalchemy create unexecutable query to refer
candidate.id
from PartTemp table although this query does not usecandidate.id
.Fix is changing query builder codes not to refer
candidate.id
.Test plan
Execute
tests/test_postgres.py::test_cand_gen_cascading_delete
.Checklist