Skip to content

Conversation

@kapouer
Copy link
Contributor

@kapouer kapouer commented Mar 12, 2021

Fixes #128

@kapouer kapouer force-pushed the master branch 3 times, most recently from 0ecb5ea to 03b8699 Compare March 12, 2021 17:03
@kapouer
Copy link
Contributor Author

kapouer commented Mar 12, 2021

I'm not sure how to correctly use tape to test that the thrown error is TypeError

@codecov-io
Copy link

Codecov Report

Merging #129 (03b8699) into master (05badcd) will increase coverage by 1.52%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #129      +/-   ##
==========================================
+ Coverage   91.74%   93.26%   +1.52%     
==========================================
  Files           4        4              
  Lines         206      208       +2     
==========================================
+ Hits          189      194       +5     
+ Misses         17       14       -3     
Impacted Files Coverage Δ
index.js 90.90% <100.00%> (+15.90%) ⬆️

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 05badcd...03b8699. Read the comment docs.

@sehrope
Copy link
Contributor

sehrope commented Mar 12, 2021

You can use a regex in the second arg of t.throws(...) to validate the thrown error:

t.test('setTypeParser should throw when oid is not an integer', function (t) {
    t.throws(function () {
      setTypeParser(null, function () {})
    }, /^TypeError: oid must be an integer/)    // <-- Add this
    t.throws(function () {
      setTypeParser('a', function () {})
    }, /^TypeError: oid must be an integer/)    // <-- Add this
    t.end()
  })

@kapouer
Copy link
Contributor Author

kapouer commented Mar 12, 2021

Ha, ok. Done and force-pushed.

@bendrucker bendrucker changed the title Throw an error when oid is missing (Fix #128) Throw an error when oid is missing Mar 12, 2021
@bendrucker
Copy link
Collaborator

Perfect, thanks!

@bendrucker bendrucker merged commit 8594bc6 into brianc:master Mar 12, 2021
@sehrope
Copy link
Contributor

sehrope commented Mar 12, 2021

LGTM.

@bendrucker Should add a note that the only thing this might break is someone setting a parser using the string representation of an int for the oid value. That would have worked before as the prop lookup of the oid gets stringified but I don't think it's a big deal to break that use case. A note should be fine as it's undocumented behavior and anyone would see it at app startup too.

@bendrucker
Copy link
Collaborator

Good catch! This is going to go out in a major release that's months overdue here anyway. Will make sure to note it in the main pg changelog.

@bendrucker bendrucker added this to the 4.0.0 milestone Mar 12, 2021
aqeelat added a commit to aqeelat/node-pg-types that referenced this pull request Apr 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

please throw when oid is nullish

4 participants