go-sqlmock icon indicating copy to clipboard operation
go-sqlmock copied to clipboard

Change wording of error message for unset query return value

Open MasterCarl opened this issue 7 years ago • 3 comments
trafficstars

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.

MasterCarl avatar Jan 05 '18 00:01 MasterCarl

Codecov Report

Merging #105 into master will not change coverage. The diff coverage is 100%.

Impacted file tree graph

@@           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 data Powered by Codecov. Last update a7b3087...ebac755. Read the comment docs.

codecov-io avatar Jan 05 '18 01:01 codecov-io

Hi, sorry for late reply, will check this when I can

l3pp4rd avatar Feb 19 '18 18:02 l3pp4rd

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?

l3pp4rd avatar Apr 22 '18 16:04 l3pp4rd