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

Corrected test get list accounts #124

Closed
wants to merge 1 commit into from
Closed

Corrected test get list accounts #124

wants to merge 1 commit into from

Conversation

leedrum
Copy link

@leedrum leedrum commented Jun 21, 2024

  • Issue: the expectation of the test case does not match with the test in video
  • How to fix: Add condition OR to the SQL query

@techschool
Copy link
Owner

techschool commented Jun 22, 2024

This doesn't need to be corrected.
The code evolved over time when new lectures are added.
If you want to see the exact code as in the recorded video, you must look at the history:
https://github.com/techschool/simplebank/commits/master/db/sqlc/account_test.go
In the first commit, the code looks exactly as in the video.

@leedrum
Copy link
Author

leedrum commented Jul 16, 2024

I do not agree with that test because we don't cover the pagination cases.
btw, I think we can create another function to get a list account that doesn't depend on the owner.
I will close this PR.

@leedrum leedrum closed this Jul 16, 2024
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.

3 participants