Skip to content
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

Merged
merged 5 commits into from
May 3, 2021
Merged

Fix sqlalchemy query error of test_postgres.py (Fix #538) #539

merged 5 commits into from
May 3, 2021

Conversation

YasushiMiyata
Copy link
Contributor

@YasushiMiyata YasushiMiyata commented Apr 26, 2021

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 use candidate.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

  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • I have updated the CHANGELOG.rst accordingly.

@YasushiMiyata
Copy link
Contributor Author

This fixs issue #538 and pytest on github was successful. However, the error occurs in the process of "test (macos-latest, 3.6)";

Warning: Cache service responded with 503 during chunk upload.
events.js:187
      throw er; // Unhandled 'error' event

Can I fix it myself? Please support.

@YasushiMiyata YasushiMiyata marked this pull request as ready for review April 27, 2021 08:27
@senwu
Copy link
Collaborator

senwu commented Apr 27, 2021

This fixs issue #538 and pytest on github was successful. However, the error occurs in the process of "test (macos-latest, 3.6)";

Warning: Cache service responded with 503 during chunk upload.
events.js:187
      throw er; // Unhandled 'error' event

Can I fix it myself? Please support.

Please fix it. Thanks!!!

@lukehsiao
Copy link
Contributor

@YasushiMiyata I just re-ran the jobs and it appears to have passed. I don't think that error was caused by you.

@YasushiMiyata
Copy link
Contributor Author

@senwu , @lukehsiao Thank you for checking and your comments. I also found my request passed.
The previous error may be something temporary unavailable on network.

@@ -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()
Copy link
Contributor

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?

Copy link
Contributor Author

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-commenter
Copy link

codecov-commenter commented Apr 28, 2021

Codecov Report

Merging #539 (642389f) into master (433a75d) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           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           
Flag Coverage Δ
unittests 86.02% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants