go-sqlmock
go-sqlmock copied to clipboard
Change wording of error message for unset query return value
I forgot to call .WillReturnRows(...) on my ExpectQuery statement, which gave me the following error message:
Query 'INSERT INTO "Message" (.+) VALUES ($1, $2, $3),($4, $5, $6);$' with args [{Name: Ordinal:1 Value:1}], must return a database/sql/driver.Rows, but it was not set for expectation *sqlmock.ExpectedQuery as ExpectedQuery => expecting Query, QueryContext or QueryRow which:
- matches sql: 'INSERT .+'
- is with arguments:
0 - 1
This lead me to believe there was something wrong with the query regex, or the arguments. I changed the wording so the message would read:
No required return value of type database/sql/driver.Rows was set for expectation *sqlmock.ExpectedQuery as ExpectedQuery => expecting Query, QueryContext or QueryRow which:
- matches sql: 'INSERT .+'
- is with arguments:
0 - 1
I removed the query that was sent to sqlmock from the error message since it has nothing to do with the failure.
Codecov Report
Merging #105 into master will not change coverage. The diff coverage is
100%.
@@ Coverage Diff @@
## master #105 +/- ##
=======================================
Coverage 89.66% 89.66%
=======================================
Files 12 12
Lines 648 648
=======================================
Hits 581 581
Misses 47 47
Partials 20 20
| Impacted Files | Coverage Δ | |
|---|---|---|
| sqlmock.go | 90.14% <100%> (ø) |
:arrow_up: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update a7b3087...ebac755. Read the comment docs.
Hi, sorry for late reply, will check this when I can
well, I do not think it needs changes, the reason is available in the message. read your own comment, you were expecting an Query but instead, the SQL is for INSERT of course, rows will not be there. The reason why the SQL string is visible first in error is exactly for that reason, because people more often mix up ExpectExec with ExpectQuery.
In case if many more people will open issues with similar problems, I will not change it yet. There must be a proper reason, now to me it all looks like 1 user out of 1000 was confused, why should be 999 confused users instead of 1?