Level / abstract-leveldown

An abstract prototype matching the leveldown API.
MIT License
146 stars 53 forks source link

Go through all t.throws, not checking messages correcly #280

Closed ralphtheninja closed 6 years ago

ralphtheninja commented 6 years ago

Using { name: 'Error', message: 'whatever' } doesn't check error messages correctly.

E.g. (from test/close-test.js):

t.throws(
  db.close.bind(db)
  , { name: 'Error', message: 'close() requires a callback argument' }
  , 'no-arg close() throws'
)

or

t.throws(
  db.close.bind(db)
  , { name: 'Error', message: 'random message here wooohoooo!!!!111oneone' }
  , 'no-arg close() throws'
)

Gives the same result. It checks that it throws, but doesn't care about the actual message.

ralphtheninja commented 6 years ago

Go throw? It's really too late :)

ralphtheninja commented 6 years ago

git grep -n -C2 't\.throws' -- test/

test/batch-test.js-13-exports.args = function (test, testCommon) {
test/batch-test.js-14-  test('test callback-less, 2-arg, batch() throws', function (t) {
test/batch-test.js:15:    t.throws(db.batch.bind(db, 'foo', {}), 'callback-less, 2-arg batch() throws')
test/batch-test.js-16-    t.end()
test/batch-test.js-17-  })
--
test/close-test.js-11-exports.close = function (test, testCommon) {
test/close-test.js-12-  test('test close()', function (t) {
test/close-test.js:13:    t.throws(
test/close-test.js-14-      db.close.bind(db)
test/close-test.js-15-      , { name: 'Error', message: 'close() requires a callback argument' }
test/close-test.js-16-      , 'no-arg close() throws'
test/close-test.js-17-    )
test/close-test.js:18:    t.throws(
test/close-test.js-19-      db.close.bind(db, 'foo')
test/close-test.js-20-      , { name: 'Error', message: 'close() requires a callback argument' }
--
test/del-test.js-12-exports.args = function (test, testCommon) {
test/del-test.js-13-  test('test argument-less del() throws', function (t) {
test/del-test.js:14:    t.throws(
test/del-test.js-15-      db.del.bind(db)
test/del-test.js-16-      , { name: 'Error', message: 'del() requires a callback argument' }
--
test/del-test.js-21-
test/del-test.js-22-  test('test callback-less, 1-arg, del() throws', function (t) {
test/del-test.js:23:    t.throws(
test/del-test.js-24-      db.del.bind(db, 'foo')
test/del-test.js-25-      , { name: 'Error', message: 'del() requires a callback argument' }
--
test/del-test.js-30-
test/del-test.js-31-  test('test callback-less, 3-arg, del() throws', function (t) {
test/del-test.js:32:    t.throws(
test/del-test.js-33-      db.del.bind(db, 'foo', {})
test/del-test.js-34-      , { name: 'Error', message: 'del() requires a callback argument' }
--
test/get-test.js-13-exports.args = function (test, testCommon) {
test/get-test.js-14-  test('test argument-less get() throws', function (t) {
test/get-test.js:15:    t.throws(
test/get-test.js-16-      db.get.bind(db)
test/get-test.js-17-      , { name: 'Error', message: 'get() requires a callback argument' }
--
test/get-test.js-22-
test/get-test.js-23-  test('test callback-less, 1-arg, get() throws', function (t) {
test/get-test.js:24:    t.throws(
test/get-test.js-25-      db.get.bind(db, 'foo')
test/get-test.js-26-      , { name: 'Error', message: 'get() requires a callback argument' }
--
test/get-test.js-31-
test/get-test.js-32-  test('test callback-less, 3-arg, get() throws', function (t) {
test/get-test.js:33:    t.throws(
test/get-test.js-34-      db.get.bind(db, 'foo', {})
test/get-test.js-35-      , { name: 'Error', message: 'get() requires a callback argument' }
--
test/iterator-test.js-12-  test('test argument-less iterator#next() throws', function (t) {
test/iterator-test.js-13-    var iterator = db.iterator()
test/iterator-test.js:14:    t.throws(
test/iterator-test.js-15-      iterator.next.bind(iterator)
test/iterator-test.js-16-      , { name: 'Error', message: 'next() requires a callback argument' }
--
test/iterator-test.js-23-    var iterator = db.iterator()
test/iterator-test.js-24-    iterator.next(function () {
test/iterator-test.js:25:      t.throws(
test/iterator-test.js-26-        iterator.end.bind(iterator)
test/iterator-test.js-27-        , { name: 'Error', message: 'end() requires a callback argument' }
--
test/iterator-test.js-34-  test('test argument-less iterator#end() throws', function (t) {
test/iterator-test.js-35-    var iterator = db.iterator()
test/iterator-test.js:36:    t.throws(
test/iterator-test.js-37-      iterator.end.bind(iterator)
test/iterator-test.js-38-      , { name: 'Error', message: 'end() requires a callback argument' }
--
test/open-test.js-6-  test('test database open no-arg throws', function (t) {
test/open-test.js-7-    var db = testCommon.factory()
test/open-test.js:8:    t.throws(
test/open-test.js-9-      db.open.bind(db)
test/open-test.js-10-      , { name: 'Error', message: 'open() requires a callback argument' }
--
test/open-test.js-16-  test('test callback-less, 1-arg, open() throws', function (t) {
test/open-test.js-17-    var db = testCommon.factory()
test/open-test.js:18:    t.throws(
test/open-test.js-19-      db.open.bind(db, {})
test/open-test.js-20-      , { name: 'Error', message: 'open() requires a callback argument' }
--
test/put-test.js-12-exports.args = function (test, testCommon) {
test/put-test.js-13-  test('test argument-less put() throws', function (t) {
test/put-test.js:14:    t.throws(
test/put-test.js-15-      db.put.bind(db)
test/put-test.js-16-      , { name: 'Error', message: 'put() requires a callback argument' }
--
test/put-test.js-21-
test/put-test.js-22-  test('test callback-less, 1-arg, put() throws', function (t) {
test/put-test.js:23:    t.throws(
test/put-test.js-24-      db.put.bind(db, 'foo')
test/put-test.js-25-      , { name: 'Error', message: 'put() requires a callback argument' }
--
test/put-test.js-30-
test/put-test.js-31-  test('test callback-less, 2-arg, put() throws', function (t) {
test/put-test.js:32:    t.throws(
test/put-test.js-33-      db.put.bind(db, 'foo', 'bar')
test/put-test.js-34-      , { name: 'Error', message: 'put() requires a callback argument' }
--
test/put-test.js-39-
test/put-test.js-40-  test('test callback-less, 3-arg, put() throws', function (t) {
test/put-test.js:41:    t.throws(
test/put-test.js-42-      db.put.bind(db, 'foo', 'bar', {})
test/put-test.js-43-      , { name: 'Error', message: 'put() requires a callback argument' }
vweevers commented 6 years ago

Ooh, good catch. Let's do this. For brevity, maybe we can use regexps: t.throws(fn, /Error: message/)