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 query command's support for DML #120

Merged
merged 2 commits into from
Jul 8, 2020

Conversation

tsibley
Copy link
Contributor

@tsibley tsibley commented Jul 8, 2020

See commit messages for details. I ran into this while investigating another feature/issue.

The failing test I added in the previous commit now passes.

Diff best viewed with whitespace ignored (git diff/show -w), as most of
the change is indentation for the new "with" block.
Comment on lines +1129 to +1131
assert db.execute_returning_dicts("select * from dogs") == [
{"id": 1, "age": 5, "name": "Cleo"},
]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In other tests, the order of the expected vs. gotten values in the asserts is reversed. However, pytest expects that the LHS is the gotten value and the RHS is the expected value and its error/debugging messages reflect that.

Copy link
Owner

Choose a reason for hiding this comment

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

I had no idea! I've been habitually doing it the other way round because 1 = a is a syntax error whereas a = 1 is not (a typo for a == 1).

It sounds like I'm going to have to break that habit and switch to actual == expected. Thanks for letting me know!

@simonw
Copy link
Owner

simonw commented Jul 8, 2020

This is an excellent fix, thanks!

@simonw simonw merged commit f8277d0 into simonw:master Jul 8, 2020
simonw added a commit that referenced this pull request Jul 8, 2020
simonw added a commit that referenced this pull request Jul 8, 2020
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