From e6f96134e28d493b61584a74d8d210e24ab60cbf Mon Sep 17 00:00:00 2001 From: "greenkeeper[bot]" Date: Sat, 20 Oct 2018 12:54:20 +0000 Subject: [PATCH 01/21] Upgrade abstract-leveldown from ~5.0.0 to ~6.0.3 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index aaa1ad87..6e32b003 100644 --- a/package.json +++ b/package.json @@ -14,7 +14,7 @@ "prepublishOnly": "npm run dependency-check" }, "dependencies": { - "abstract-leveldown": "~5.0.0", + "abstract-leveldown": "~6.0.3", "bindings": "~1.5.0", "fast-future": "~1.0.2", "nan": "~2.13.2", From 2beee0083df31c140da332576ed9b21d4d87f8dc Mon Sep 17 00:00:00 2001 From: Vincent Weevers Date: Mon, 14 May 2018 09:08:08 +0300 Subject: [PATCH 02/21] Import and fix GC test from levelup --- test/iterator-gc-test.js | 68 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 test/iterator-gc-test.js diff --git a/test/iterator-gc-test.js b/test/iterator-gc-test.js new file mode 100644 index 00000000..feda357f --- /dev/null +++ b/test/iterator-gc-test.js @@ -0,0 +1,68 @@ +'use strict' + +const test = require('tape') +const testCommon = require('abstract-leveldown/testCommon') +const leveldown = require('..') +const sourceData = [] + +for (let i = 0; i < 1e3; i++) { + sourceData.push({ + type: 'put', + key: i.toString(), + value: Math.random().toString() + }) +} + +test('setUp', testCommon.setUp) + +// When you have a database open with an active iterator, but no references to +// the db, V8 will GC the database and you'll get an failed assert from LevelDB. +test('db without ref does not get GCed while iterating', function (t) { + t.plan(6) + + let db = leveldown(testCommon.location()) + + db.open(function (err) { + t.ifError(err, 'no open error') + + // Insert test data + db.batch(sourceData.slice(), function (err) { + t.ifError(err, 'no batch error') + + // Set highWaterMark to 0 so that we don't preemptively fetch. + const it = db.iterator({ highWaterMark: 0 }) + + // Remove reference + db = null + + if (global.gc) { + // This is the reliable way to trigger GC (and the bug if it exists). + // Useful for manual testing with "node --expose-gc". + global.gc() + iterate(it) + } else { + // But a timeout usually also allows GC to kick in. If not, the time + // between iterator ticks might. That's when "highWaterMark: 0" helps. + setTimeout(iterate.bind(null, it), 1000) + } + }) + }) + + function iterate (it) { + // No reference to db here, could be GCed. It shouldn't.. + testCommon.collectEntries(it, function (err, entries) { + t.ifError(err, 'no iterator error') + t.is(entries.length, sourceData.length, 'got data') + + // Because we also have a reference on the iterator. That's the fix. + t.ok(it.db, 'abstract iterator has reference to db') + + // Which as luck would have it, also allows us to properly end this test. + it.db.close(function (err) { + t.ifError(err, 'no close error') + }) + }) + } +}) + +test('tearDown', testCommon.tearDown) From ad2f8348e6ec770cfe00592dc6f518d6cd4af02d Mon Sep 17 00:00:00 2001 From: Lars-Magnus Skog Date: Thu, 21 Jun 2018 13:48:14 +0200 Subject: [PATCH 03/21] Remove redundant db.close() from test/approximate-size-test.js --- test/approximate-size-test.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/test/approximate-size-test.js b/test/approximate-size-test.js index d400cfa6..93b859b7 100644 --- a/test/approximate-size-test.js +++ b/test/approximate-size-test.js @@ -106,11 +106,7 @@ test('test approximateSize()', function (t) { t.equal(typeof size, 'number') // account for snappy compression, original would be ~100000 t.ok(size > 40000, 'size reports a reasonable amount (' + size + ')') - - db.close(function (err) { - t.error(err) - t.end() - }) + t.end() }) }) }) From 5fa429afa453578704aa573051e5f1678cf2f335 Mon Sep 17 00:00:00 2001 From: Lars-Magnus Skog Date: Fri, 13 Jul 2018 16:58:16 +0200 Subject: [PATCH 04/21] Update handling of location * Handle location in constructor at it was removed from abstract-leveldown * Pass a factory function to abstract tests * Use tempy in tests, removing need for cleanup * Use level-concat-iterator in tests --- leveldown.js | 8 ++++- package.json | 2 ++ test/approximate-size-test.js | 7 ++--- test/batch-test.js | 6 ++-- test/chained-batch-test.js | 6 ++-- test/close-test.js | 23 ++++----------- test/common.js | 9 ++++++ test/compact-range-test.js | 5 ++-- test/compression-test.js | 25 +++++++--------- test/del-test.js | 6 ++-- test/destroy-test.js | 10 ++++--- test/get-test.js | 6 ++-- test/getproperty-test.js | 5 ++-- test/iterator-gc-test.js | 8 ++--- test/iterator-range-test.js | 6 ++-- test/iterator-recursion-test.js | 5 ++-- test/iterator-test.js | 6 ++-- test/leak-tester-batch.js | 23 +++++++-------- test/leak-tester.js | 16 +++++----- test/leveldown-test.js | 16 ++++++++-- test/make.js | 52 +++++++++++++-------------------- test/open-test.js | 6 ++-- test/put-get-del-test.js | 6 ++-- test/put-test.js | 6 ++-- test/repair-test.js | 3 +- test/stack-blower.js | 31 +++++++++----------- 26 files changed, 146 insertions(+), 156 deletions(-) create mode 100644 test/common.js diff --git a/leveldown.js b/leveldown.js index 9a19c5b1..32f98c4f 100644 --- a/leveldown.js +++ b/leveldown.js @@ -10,7 +10,13 @@ function LevelDOWN (location) { return new LevelDOWN(location) } - AbstractLevelDOWN.call(this, location) + if (typeof location !== 'string') { + throw new Error('constructor requires a location string argument') + } + + AbstractLevelDOWN.call(this) + + this.location = location this.binding = binding(location) } diff --git a/package.json b/package.json index 6e32b003..2344711a 100644 --- a/package.json +++ b/package.json @@ -28,6 +28,7 @@ "du": "~0.1.0", "hallmark": "^0.1.0", "iota-array": "^1.0.0", + "level-concat-iterator": "^2.0.0", "level-community": "^3.0.0", "lexicographic-integer": "^1.1.0", "mkfiletree": "^1.0.1", @@ -41,6 +42,7 @@ "slump": "^3.0.0", "standard": "^12.0.0", "tape": "^4.10.0", + "tempy": "^0.2.1", "uuid": "^3.2.1", "verify-travis-appveyor": "^3.0.0" }, diff --git a/test/approximate-size-test.js b/test/approximate-size-test.js index 93b859b7..06dced35 100644 --- a/test/approximate-size-test.js +++ b/test/approximate-size-test.js @@ -1,13 +1,12 @@ const test = require('tape') -const leveldown = require('..') -const testCommon = require('abstract-leveldown/testCommon') +const testCommon = require('./common') var db test('setUp common for approximate size', testCommon.setUp) test('setUp db', function (t) { - db = leveldown(testCommon.location()) + db = testCommon.factory() db.open(t.end.bind(t)) }) @@ -67,7 +66,7 @@ test('test 1-arg + callback approximateSize() throws', function (t) { test('test custom _serialize*', function (t) { t.plan(4) - var db = leveldown(testCommon.location()) + var db = testCommon.factory() db._serializeKey = function (data) { return data } db.approximateSize = function (start, end, callback) { t.deepEqual(start, { foo: 'bar' }) diff --git a/test/batch-test.js b/test/batch-test.js index f56b353a..4594c48a 100644 --- a/test/batch-test.js +++ b/test/batch-test.js @@ -1,5 +1,5 @@ const test = require('tape') -const leveldown = require('../') -const abstract = require('abstract-leveldown/abstract/batch-test') +const testCommon = require('./common') +const abstract = require('abstract-leveldown/test/batch-test') -abstract.all(leveldown, test) +abstract.all(testCommon.factory, test) diff --git a/test/chained-batch-test.js b/test/chained-batch-test.js index 505dbd43..a8adb35d 100644 --- a/test/chained-batch-test.js +++ b/test/chained-batch-test.js @@ -1,5 +1,5 @@ const test = require('tape') -const leveldown = require('../') -const abstract = require('abstract-leveldown/abstract/chained-batch-test') +const testCommon = require('./common') +const abstract = require('abstract-leveldown/test/chained-batch-test') -abstract.all(leveldown, test) +abstract.all(testCommon.factory, test) diff --git a/test/close-test.js b/test/close-test.js index 967745b0..5e873e22 100644 --- a/test/close-test.js +++ b/test/close-test.js @@ -1,22 +1,9 @@ const test = require('tape') -const testCommon = require('abstract-leveldown/testCommon') -const leveldown = require('../') -const abstract = require('abstract-leveldown/abstract/close-test') +const testCommon = require('./common') +const abstract = require('abstract-leveldown/test/close-test') -module.exports.setUp = function () { - test('setUp', testCommon.setUp) -} +test('setUp', testCommon.setUp) -module.exports.close = abstract.close +abstract.close(testCommon.factory, test, testCommon) -module.exports.tearDown = function () { - test('tearDown', testCommon.tearDown) -} - -module.exports.all = function (leveldown) { - module.exports.setUp() - module.exports.close(leveldown, test) - module.exports.tearDown() -} - -module.exports.all(leveldown) +test('tearDown', testCommon.tearDown) diff --git a/test/common.js b/test/common.js new file mode 100644 index 00000000..d905eb6f --- /dev/null +++ b/test/common.js @@ -0,0 +1,9 @@ +const testCommon = require('abstract-leveldown/test/common') +const tempy = require('tempy') +const leveldown = require('..') + +testCommon.factory = function () { + return leveldown(tempy.directory()) +} + +module.exports = testCommon diff --git a/test/compact-range-test.js b/test/compact-range-test.js index 850637d0..9c1b55ee 100644 --- a/test/compact-range-test.js +++ b/test/compact-range-test.js @@ -1,13 +1,12 @@ const test = require('tape') -const testCommon = require('abstract-leveldown/testCommon') -const leveldown = require('../') +const testCommon = require('./common') var db test('setUp common', testCommon.setUp) test('setUp db', function (t) { - db = leveldown(testCommon.location()) + db = testCommon.factory() db.open(t.end.bind(t)) }) diff --git a/test/compression-test.js b/test/compression-test.js index cb63f37d..a3b831c2 100644 --- a/test/compression-test.js +++ b/test/compression-test.js @@ -1,14 +1,9 @@ -/* Copyright (c) 2012-2017 LevelUP contributors - * See list at - * MIT License - */ - -var async = require('async') -var du = require('du') -var delayed = require('delayed') -var common = require('abstract-leveldown/testCommon') -var leveldown = require('../') -var test = require('tape') +const async = require('async') +const du = require('du') +const delayed = require('delayed') +const testCommon = require('./common') +const leveldown = require('..') +const test = require('tape') var compressableData = Buffer.from(Array.apply(null, Array(1024 * 100)).map(function () { return 'aaaaaaaaaa' }).join('')) var multiples = 10 @@ -44,10 +39,10 @@ var cycle = function (db, compression, t, callback) { test('compression', function (t) { t.plan(4) - t.test('set up', common.setUp) + t.test('set up', testCommon.setUp) t.test('test data is compressed by default (db.put())', function (t) { - var db = leveldown(common.location()) + var db = testCommon.factory() db.open(function (err) { t.error(err) async.forEach(Array.apply(null, Array(multiples)).map(function (e, i) { @@ -59,7 +54,7 @@ test('compression', function (t) { }) t.test('test data is not compressed with compression=false on open() (db.put())', function (t) { - var db = leveldown(common.location()) + var db = testCommon.factory() db.open({ compression: false }, function (err) { t.error(err) async.forEach(Array.apply(null, Array(multiples)).map(function (e, i) { @@ -71,7 +66,7 @@ test('compression', function (t) { }) t.test('test data is compressed by default (db.batch())', function (t) { - var db = leveldown(common.location()) + var db = testCommon.factory() db.open(function (err) { t.error(err) db.batch(Array.apply(null, Array(multiples)).map(function (e, i) { diff --git a/test/del-test.js b/test/del-test.js index c3d49c14..8ca70133 100644 --- a/test/del-test.js +++ b/test/del-test.js @@ -1,5 +1,5 @@ const test = require('tape') -const leveldown = require('../') -const abstract = require('abstract-leveldown/abstract/del-test') +const testCommon = require('./common') +const abstract = require('abstract-leveldown/test/del-test') -abstract.all(leveldown, test) +abstract.all(testCommon.factory, test) diff --git a/test/destroy-test.js b/test/destroy-test.js index a80d33ea..7d8b44d0 100644 --- a/test/destroy-test.js +++ b/test/destroy-test.js @@ -1,5 +1,5 @@ const test = require('tape') -const testCommon = require('abstract-leveldown/testCommon') +const tempy = require('tempy') const fs = require('fs') const path = require('path') const mkfiletree = require('mkfiletree') @@ -26,7 +26,7 @@ test('test callback-less, 1-arg, destroy() throws', function (t) { test('test destroy non-existent directory', function (t) { t.plan(4) - var location = testCommon.location() + var location = tempy.directory() var parent = path.dirname(location) // For symmetry with the opposite test below. @@ -87,7 +87,8 @@ test('test destroy non leveldb directory', function (t) { }) }) -makeTest('test destroy() cleans and removes leveldb-only dir', function (db, t, done, location) { +makeTest('test destroy() cleans and removes leveldb-only dir', function (db, t, done) { + var location = db.location db.close(function (err) { t.ifError(err, 'no close error') @@ -100,7 +101,8 @@ makeTest('test destroy() cleans and removes leveldb-only dir', function (db, t, }) }) -makeTest('test destroy() cleans and removes only leveldb parts of a dir', function (db, t, done, location) { +makeTest('test destroy() cleans and removes only leveldb parts of a dir', function (db, t, done) { + var location = db.location fs.writeFileSync(path.join(location, 'foo'), 'FOO') db.close(function (err) { diff --git a/test/get-test.js b/test/get-test.js index 423268e7..50ed52a2 100644 --- a/test/get-test.js +++ b/test/get-test.js @@ -1,5 +1,5 @@ const test = require('tape') -const leveldown = require('../') -const abstract = require('abstract-leveldown/abstract/get-test') +const testCommon = require('./common') +const abstract = require('abstract-leveldown/test/get-test') -abstract.all(leveldown, test) +abstract.all(testCommon.factory, test) diff --git a/test/getproperty-test.js b/test/getproperty-test.js index 4fca9ee8..e9b8ce49 100644 --- a/test/getproperty-test.js +++ b/test/getproperty-test.js @@ -1,13 +1,12 @@ const test = require('tape') -const testCommon = require('abstract-leveldown/testCommon') -const leveldown = require('../') +const testCommon = require('./common') var db test('setUp common', testCommon.setUp) test('setUp db', function (t) { - db = leveldown(testCommon.location()) + db = testCommon.factory() db.open(t.end.bind(t)) }) diff --git a/test/iterator-gc-test.js b/test/iterator-gc-test.js index feda357f..51a3dbd0 100644 --- a/test/iterator-gc-test.js +++ b/test/iterator-gc-test.js @@ -1,8 +1,8 @@ 'use strict' const test = require('tape') -const testCommon = require('abstract-leveldown/testCommon') -const leveldown = require('..') +const collectEntries = require('level-concat-iterator') +const testCommon = require('./common') const sourceData = [] for (let i = 0; i < 1e3; i++) { @@ -20,7 +20,7 @@ test('setUp', testCommon.setUp) test('db without ref does not get GCed while iterating', function (t) { t.plan(6) - let db = leveldown(testCommon.location()) + let db = testCommon.factory() db.open(function (err) { t.ifError(err, 'no open error') @@ -50,7 +50,7 @@ test('db without ref does not get GCed while iterating', function (t) { function iterate (it) { // No reference to db here, could be GCed. It shouldn't.. - testCommon.collectEntries(it, function (err, entries) { + collectEntries(it, function (err, entries) { t.ifError(err, 'no iterator error') t.is(entries.length, sourceData.length, 'got data') diff --git a/test/iterator-range-test.js b/test/iterator-range-test.js index fa828e9b..d8595355 100644 --- a/test/iterator-range-test.js +++ b/test/iterator-range-test.js @@ -1,5 +1,5 @@ const test = require('tape') -const leveldown = require('..') -const abstract = require('abstract-leveldown/abstract/iterator-range-test') +const testCommon = require('./common') +const abstract = require('abstract-leveldown/test/iterator-range-test') -abstract.all(leveldown, test) +abstract.all(testCommon.factory, test) diff --git a/test/iterator-recursion-test.js b/test/iterator-recursion-test.js index 36c69a4f..71bd9edb 100644 --- a/test/iterator-recursion-test.js +++ b/test/iterator-recursion-test.js @@ -1,6 +1,5 @@ const test = require('tape') -const testCommon = require('abstract-leveldown/testCommon') -const leveldown = require('../') +const testCommon = require('./common') const fork = require('child_process').fork const path = require('path') @@ -42,7 +41,7 @@ test('try to create an iterator with a blown stack', function (t) { }) test('setUp db', function (t) { - db = leveldown(testCommon.location()) + db = testCommon.factory() db.open(function (err) { t.error(err) db.batch(sourceData, t.end.bind(t)) diff --git a/test/iterator-test.js b/test/iterator-test.js index d17c0a7a..0554b76f 100644 --- a/test/iterator-test.js +++ b/test/iterator-test.js @@ -1,12 +1,12 @@ const test = require('tape') -const leveldown = require('../') -const abstract = require('abstract-leveldown/abstract/iterator-test') +const testCommon = require('./common') +const abstract = require('abstract-leveldown/test/iterator-test') const make = require('./make') const iota = require('iota-array') const lexi = require('lexicographic-integer') const util = require('util') -abstract.all(leveldown, test) +abstract.all(testCommon.factory, test) make('iterator throws if key is not a string or buffer', function (db, t, done) { var keys = [null, undefined, 1, true, false] diff --git a/test/leak-tester-batch.js b/test/leak-tester-batch.js index 1a90e88a..6739a71e 100644 --- a/test/leak-tester-batch.js +++ b/test/leak-tester-batch.js @@ -3,12 +3,13 @@ const BUFFERS = false const CHAINED = false -var leveldown = require('..') -var crypto = require('crypto') -var assert = require('assert') -var writeCount = 0 -var rssBase -var db +const testCommon = require('./common') +const crypto = require('crypto') +const assert = require('assert') + +let writeCount = 0 +let rssBase +let db function print () { if (writeCount % 100 === 0) { @@ -73,10 +74,8 @@ var run = CHAINED print() } -leveldown.destroy('./leakydb', function () { - db = leveldown('./leakydb') - db.open({ xcacheSize: 0, xmaxOpenFiles: 10 }, function () { - rssBase = process.memoryUsage().rss - run() - }) +db = testCommon.factory() +db.open({ xcacheSize: 0, xmaxOpenFiles: 10 }, function () { + rssBase = process.memoryUsage().rss + run() }) diff --git a/test/leak-tester.js b/test/leak-tester.js index 44c8d5c8..504ae834 100644 --- a/test/leak-tester.js +++ b/test/leak-tester.js @@ -1,9 +1,9 @@ /* global gc */ -const BUFFERS = false +const testCommon = require('./common') +const crypto = require('crypto') -var leveldown = require('..') -var crypto = require('crypto') +var BUFFERS = false var putCount = 0 var getCount = 0 var rssBase @@ -42,10 +42,8 @@ function run () { } } -leveldown.destroy('./leakydb', function () { - db = leveldown('./leakydb') - db.open({ xcacheSize: 0, xmaxOpenFiles: 10 }, function () { - rssBase = process.memoryUsage().rss - run() - }) +db = testCommon.factory() +db.open({ xcacheSize: 0, xmaxOpenFiles: 10 }, function () { + rssBase = process.memoryUsage().rss + run() }) diff --git a/test/leveldown-test.js b/test/leveldown-test.js index 7c2d0d83..44df42c2 100644 --- a/test/leveldown-test.js +++ b/test/leveldown-test.js @@ -1,5 +1,15 @@ const test = require('tape') -const leveldown = require('../') -const abstract = require('abstract-leveldown/abstract/leveldown-test') +const testCommon = require('./common') +const abstract = require('abstract-leveldown/test/leveldown-test') +const leveldown = require('..') -abstract.args(leveldown, test) +abstract.args(testCommon.factory, test) + +test('test database creation non-string location throws', function (t) { + t.throws( + leveldown.bind(null, {}), + /constructor requires a location string argument/, + 'non-string location leveldown() throws' + ) + t.end() +}) diff --git a/test/make.js b/test/make.js index 80e9035d..d50d6397 100644 --- a/test/make.js +++ b/test/make.js @@ -1,40 +1,28 @@ const test = require('tape') -const testCommon = require('abstract-leveldown/testCommon') -const cleanup = testCommon.cleanup -const location = testCommon.location -const leveldown = require('../') +const testCommon = require('./common') function makeTest (name, testFn) { test(name, function (t) { - cleanup(function () { - var loc = location() - var db = leveldown(loc) - var done = function (close) { - if (close === false) { - cleanup(function (err) { - t.error(err, 'no error after cleanup') - t.end() - }) - return - } - db.close(function (err) { - t.notOk(err, 'no error from close()') - cleanup(function (err) { - t.error(err, 'no error after cleanup') - t.end() - }) - }) + var db = testCommon.factory() + var done = function (close) { + if (close === false) { + t.end() + return } - db.open(function (err) { - t.notOk(err, 'no error from open()') - db.batch([ - { type: 'put', key: 'one', value: '1' }, - { type: 'put', key: 'two', value: '2' }, - { type: 'put', key: 'three', value: '3' } - ], function (err) { - t.notOk(err, 'no error from batch()') - testFn(db, t, done, loc) - }) + db.close(function (err) { + t.error(err, 'no error from close()') + t.end() + }) + } + db.open(function (err) { + t.error(err, 'no error from open()') + db.batch([ + { type: 'put', key: 'one', value: '1' }, + { type: 'put', key: 'two', value: '2' }, + { type: 'put', key: 'three', value: '3' } + ], function (err) { + t.error(err, 'no error from batch()') + testFn(db, t, done) }) }) }) diff --git a/test/open-test.js b/test/open-test.js index 606be1a9..7d16bc89 100644 --- a/test/open-test.js +++ b/test/open-test.js @@ -1,5 +1,5 @@ const test = require('tape') -const leveldown = require('../') -const abstract = require('abstract-leveldown/abstract/open-test') +const testCommon = require('./common') +const abstract = require('abstract-leveldown/test/open-test') -abstract.all(leveldown, test) +abstract.all(testCommon.factory, test) diff --git a/test/put-get-del-test.js b/test/put-get-del-test.js index 55dec6cb..1317688b 100644 --- a/test/put-get-del-test.js +++ b/test/put-get-del-test.js @@ -1,5 +1,5 @@ const test = require('tape') -const leveldown = require('../') -const abstract = require('abstract-leveldown/abstract/put-get-del-test') +const testCommon = require('./common') +const abstract = require('abstract-leveldown/test/put-get-del-test') -abstract.all(leveldown, test) +abstract.all(testCommon.factory, test) diff --git a/test/put-test.js b/test/put-test.js index 87e73439..b7983230 100644 --- a/test/put-test.js +++ b/test/put-test.js @@ -1,5 +1,5 @@ const test = require('tape') -const leveldown = require('../') -const abstract = require('abstract-leveldown/abstract/put-test') +const testCommon = require('./common') +const abstract = require('abstract-leveldown/test/put-test') -abstract.all(leveldown, test) +abstract.all(testCommon.factory, test) diff --git a/test/repair-test.js b/test/repair-test.js index 8814571d..b3bce259 100644 --- a/test/repair-test.js +++ b/test/repair-test.js @@ -27,7 +27,8 @@ test('test repair non-existent directory returns error', function (t) { }) // a proxy indicator that RepairDB is being called and doing its thing -makeTest('test repair() compacts', function (db, t, done, location) { +makeTest('test repair() compacts', function (db, t, done) { + var location = db.location db.close(function (err) { t.notOk(err, 'no error') var files = fs.readdirSync(location) diff --git a/test/stack-blower.js b/test/stack-blower.js index c6cc825b..9d464b99 100644 --- a/test/stack-blower.js +++ b/test/stack-blower.js @@ -5,26 +5,23 @@ * iterator-recursion-test.js. To prevent tap from trying to run this test * directly, we check for a command-line argument. */ -const testCommon = require('abstract-leveldown/testCommon') -const leveldown = require('../') +const testCommon = require('./common') if (process.argv[2] === 'run') { - testCommon.cleanup(function () { - var db = leveldown(testCommon.location()) - var depth = 0 + var db = testCommon.factory() + var depth = 0 - db.open(function () { - function recurse () { - db.iterator({ start: '0' }) - depth++ - recurse() - } + db.open(function () { + function recurse () { + db.iterator({ start: '0' }) + depth++ + recurse() + } - try { - recurse() - } catch (e) { - process.send('Catchable error at depth ' + depth) - } - }) + try { + recurse() + } catch (e) { + process.send('Catchable error at depth ' + depth) + } }) } From 1e537656eaf1a9c88eca2bb109fb38c25f8f73e0 Mon Sep 17 00:00:00 2001 From: Lars-Magnus Skog Date: Fri, 13 Jul 2018 19:59:03 +0200 Subject: [PATCH 05/21] Invoke abstract tests from single function * Invoke abstract tests from single function * Options is actually now passed as options from abstract-leveldown and not a function --- chained-batch.js | 2 +- test/abstract-leveldown-test.js | 1 + test/batch-test.js | 5 ----- test/chained-batch-test.js | 5 ----- test/close-test.js | 9 --------- test/common.js | 13 +++++++------ test/del-test.js | 5 ----- test/get-test.js | 5 ----- test/iterator-range-test.js | 5 ----- test/iterator-test.js | 5 ----- test/leveldown-test.js | 4 ---- test/open-test.js | 5 ----- test/put-get-del-test.js | 5 ----- test/put-test.js | 5 ----- 14 files changed, 9 insertions(+), 65 deletions(-) create mode 100644 test/abstract-leveldown-test.js delete mode 100644 test/batch-test.js delete mode 100644 test/chained-batch-test.js delete mode 100644 test/close-test.js delete mode 100644 test/del-test.js delete mode 100644 test/get-test.js delete mode 100644 test/iterator-range-test.js delete mode 100644 test/open-test.js delete mode 100644 test/put-get-del-test.js delete mode 100644 test/put-test.js diff --git a/chained-batch.js b/chained-batch.js index fadc76d4..0b917672 100644 --- a/chained-batch.js +++ b/chained-batch.js @@ -19,7 +19,7 @@ ChainedBatch.prototype._clear = function (key) { } ChainedBatch.prototype._write = function (options, callback) { - this.binding.write(options, callback) + this.binding.write(callback) } util.inherits(ChainedBatch, AbstractChainedBatch) diff --git a/test/abstract-leveldown-test.js b/test/abstract-leveldown-test.js new file mode 100644 index 00000000..12888ea0 --- /dev/null +++ b/test/abstract-leveldown-test.js @@ -0,0 +1 @@ +require('abstract-leveldown/test')(require('./common')) diff --git a/test/batch-test.js b/test/batch-test.js deleted file mode 100644 index 4594c48a..00000000 --- a/test/batch-test.js +++ /dev/null @@ -1,5 +0,0 @@ -const test = require('tape') -const testCommon = require('./common') -const abstract = require('abstract-leveldown/test/batch-test') - -abstract.all(testCommon.factory, test) diff --git a/test/chained-batch-test.js b/test/chained-batch-test.js deleted file mode 100644 index a8adb35d..00000000 --- a/test/chained-batch-test.js +++ /dev/null @@ -1,5 +0,0 @@ -const test = require('tape') -const testCommon = require('./common') -const abstract = require('abstract-leveldown/test/chained-batch-test') - -abstract.all(testCommon.factory, test) diff --git a/test/close-test.js b/test/close-test.js deleted file mode 100644 index 5e873e22..00000000 --- a/test/close-test.js +++ /dev/null @@ -1,9 +0,0 @@ -const test = require('tape') -const testCommon = require('./common') -const abstract = require('abstract-leveldown/test/close-test') - -test('setUp', testCommon.setUp) - -abstract.close(testCommon.factory, test, testCommon) - -test('tearDown', testCommon.tearDown) diff --git a/test/common.js b/test/common.js index d905eb6f..4c55d8b0 100644 --- a/test/common.js +++ b/test/common.js @@ -1,9 +1,10 @@ -const testCommon = require('abstract-leveldown/test/common') +const test = require('tape') const tempy = require('tempy') const leveldown = require('..') -testCommon.factory = function () { - return leveldown(tempy.directory()) -} - -module.exports = testCommon +module.exports = require('abstract-leveldown/test/common')({ + test: test, + factory: function () { + return leveldown(tempy.directory()) + } +}) diff --git a/test/del-test.js b/test/del-test.js deleted file mode 100644 index 8ca70133..00000000 --- a/test/del-test.js +++ /dev/null @@ -1,5 +0,0 @@ -const test = require('tape') -const testCommon = require('./common') -const abstract = require('abstract-leveldown/test/del-test') - -abstract.all(testCommon.factory, test) diff --git a/test/get-test.js b/test/get-test.js deleted file mode 100644 index 50ed52a2..00000000 --- a/test/get-test.js +++ /dev/null @@ -1,5 +0,0 @@ -const test = require('tape') -const testCommon = require('./common') -const abstract = require('abstract-leveldown/test/get-test') - -abstract.all(testCommon.factory, test) diff --git a/test/iterator-range-test.js b/test/iterator-range-test.js deleted file mode 100644 index d8595355..00000000 --- a/test/iterator-range-test.js +++ /dev/null @@ -1,5 +0,0 @@ -const test = require('tape') -const testCommon = require('./common') -const abstract = require('abstract-leveldown/test/iterator-range-test') - -abstract.all(testCommon.factory, test) diff --git a/test/iterator-test.js b/test/iterator-test.js index 0554b76f..6e611ee8 100644 --- a/test/iterator-test.js +++ b/test/iterator-test.js @@ -1,13 +1,8 @@ -const test = require('tape') -const testCommon = require('./common') -const abstract = require('abstract-leveldown/test/iterator-test') const make = require('./make') const iota = require('iota-array') const lexi = require('lexicographic-integer') const util = require('util') -abstract.all(testCommon.factory, test) - make('iterator throws if key is not a string or buffer', function (db, t, done) { var keys = [null, undefined, 1, true, false] var pending = keys.length diff --git a/test/leveldown-test.js b/test/leveldown-test.js index 44df42c2..bf002307 100644 --- a/test/leveldown-test.js +++ b/test/leveldown-test.js @@ -1,10 +1,6 @@ const test = require('tape') -const testCommon = require('./common') -const abstract = require('abstract-leveldown/test/leveldown-test') const leveldown = require('..') -abstract.args(testCommon.factory, test) - test('test database creation non-string location throws', function (t) { t.throws( leveldown.bind(null, {}), diff --git a/test/open-test.js b/test/open-test.js deleted file mode 100644 index 7d16bc89..00000000 --- a/test/open-test.js +++ /dev/null @@ -1,5 +0,0 @@ -const test = require('tape') -const testCommon = require('./common') -const abstract = require('abstract-leveldown/test/open-test') - -abstract.all(testCommon.factory, test) diff --git a/test/put-get-del-test.js b/test/put-get-del-test.js deleted file mode 100644 index 1317688b..00000000 --- a/test/put-get-del-test.js +++ /dev/null @@ -1,5 +0,0 @@ -const test = require('tape') -const testCommon = require('./common') -const abstract = require('abstract-leveldown/test/put-get-del-test') - -abstract.all(testCommon.factory, test) diff --git a/test/put-test.js b/test/put-test.js deleted file mode 100644 index b7983230..00000000 --- a/test/put-test.js +++ /dev/null @@ -1,5 +0,0 @@ -const test = require('tape') -const testCommon = require('./common') -const abstract = require('abstract-leveldown/test/put-test') - -abstract.all(testCommon.factory, test) From c68a1417357b893dc2f7f379cce6773a7d640567 Mon Sep 17 00:00:00 2001 From: Lars-Magnus Skog Date: Sun, 15 Jul 2018 19:34:20 +0200 Subject: [PATCH 06/21] Use suite.common() in test/common.js --- test/common.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/common.js b/test/common.js index 4c55d8b0..14a4a904 100644 --- a/test/common.js +++ b/test/common.js @@ -1,8 +1,9 @@ const test = require('tape') const tempy = require('tempy') const leveldown = require('..') +const suite = require('abstract-leveldown/test') -module.exports = require('abstract-leveldown/test/common')({ +module.exports = suite.common({ test: test, factory: function () { return leveldown(tempy.directory()) From 2681961ee0a414450d9a17fd38034085ad1eb533 Mon Sep 17 00:00:00 2001 From: Vincent Weevers Date: Sun, 15 Jul 2018 16:38:29 +0200 Subject: [PATCH 07/21] Implement abstract _seek() instead of seek() --- iterator.js | 13 ++----------- test/iterator-test.js | 41 +++++++++++++++++++++++------------------ 2 files changed, 25 insertions(+), 29 deletions(-) diff --git a/iterator.js b/iterator.js index 9d412e56..1e99a819 100644 --- a/iterator.js +++ b/iterator.js @@ -13,18 +13,9 @@ function Iterator (db, options) { util.inherits(Iterator, AbstractIterator) -Iterator.prototype.seek = function (target) { - if (this._ended) { - throw new Error('cannot call seek() after end()') - } - if (this._nexting) { - throw new Error('cannot call seek() before next() has completed') - } - if (typeof target !== 'string' && !Buffer.isBuffer(target)) { - throw new Error('seek() requires a string or buffer key') - } +Iterator.prototype._seek = function (target) { if (target.length === 0) { - throw new Error('cannot seek() to an empty key') + throw new Error('cannot seek() to an empty target') } this.cache = null diff --git a/test/iterator-test.js b/test/iterator-test.js index 6e611ee8..e0399262 100644 --- a/test/iterator-test.js +++ b/test/iterator-test.js @@ -3,21 +3,21 @@ const iota = require('iota-array') const lexi = require('lexicographic-integer') const util = require('util') -make('iterator throws if key is not a string or buffer', function (db, t, done) { - var keys = [null, undefined, 1, true, false] - var pending = keys.length +make('iterator#seek throws if target is empty', function (db, t, done) { + var targets = [null, undefined, '', Buffer.alloc(0), []] + var pending = targets.length - keys.forEach(function (key) { - var error + targets.forEach(function (target) { var ite = db.iterator() + var error try { - ite.seek(key) - } catch (e) { - error = e + ite.seek(target) + } catch (err) { + error = err.message } - t.ok(error, 'had error from seek()') + t.is(error, 'cannot seek() to an empty target', 'got error') ite.end(end) }) @@ -86,8 +86,8 @@ make('reverse seek from invalid range', function (db, t, done) { ite.seek('zzz') ite.next(function (err, key, value) { t.error(err, 'no error') - t.same(key.toString(), 'two', 'end of iterator') - t.same(value.toString(), '2', 'end of iterator') + t.same(key.toString(), 'two') + t.same(value.toString(), '2') ite.end(done) }) }) @@ -132,17 +132,20 @@ make('iterator optimized for seek', function (db, t, done) { make('iterator seek before next has completed', function (db, t, done) { var ite = db.iterator() + var error + ite.next(function (err, key, value) { t.error(err, 'no error from next()') ite.end(done) }) - var error + try { ite.seek('two') - } catch (e) { - error = e + } catch (err) { + error = err.message } - t.ok(error, 'had error from seek() before next() has completed') + + t.is(error, 'cannot call seek() before next() has completed', 'got error') }) make('close db with open iterator', function (db, t, done) { @@ -170,12 +173,14 @@ make('iterator seek after end', function (db, t, done) { ite.end(function (err) { t.error(err, 'no error from end()') var error + try { ite.seek('two') - } catch (e) { - error = e + } catch (err) { + error = err.message } - t.ok(error, 'had error from seek() after end()') + + t.is(error, 'cannot call seek() after end()', 'got error') done() }) }) From c832f7e86c229f604c1dc0b65568150596b96284 Mon Sep 17 00:00:00 2001 From: Vincent Weevers Date: Sun, 15 Jul 2018 15:59:51 +0200 Subject: [PATCH 08/21] Implement abstract _serializeKey() and _serializeValue() With undefined behavior for nullish targets (range options and seek targets). Previously, nullish range options were ignored and seek(null) and seek(undefined) would throw an error. Now they translate to String(null) and String(undefined). This change makes it explicit that rocksdb only supports buffers and strings. Nullish targets do have a meaning in the ecosystem; that meaning should be given at a higher level like encoding-down. In other words, it isn't rocksdb's concern anymore. --- leveldown.js | 8 ++++++++ test/iterator-test.js | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/leveldown.js b/leveldown.js index 32f98c4f..7da510e9 100644 --- a/leveldown.js +++ b/leveldown.js @@ -30,6 +30,14 @@ LevelDOWN.prototype._close = function (callback) { this.binding.close(callback) } +LevelDOWN.prototype._serializeKey = function (key) { + return Buffer.isBuffer(key) ? key : String(key) +} + +LevelDOWN.prototype._serializeValue = function (value) { + return Buffer.isBuffer(value) ? value : String(value) +} + LevelDOWN.prototype._put = function (key, value, options, callback) { this.binding.put(key, value, options, callback) } diff --git a/test/iterator-test.js b/test/iterator-test.js index e0399262..60fee8f2 100644 --- a/test/iterator-test.js +++ b/test/iterator-test.js @@ -4,7 +4,7 @@ const lexi = require('lexicographic-integer') const util = require('util') make('iterator#seek throws if target is empty', function (db, t, done) { - var targets = [null, undefined, '', Buffer.alloc(0), []] + var targets = ['', Buffer.alloc(0), []] var pending = targets.length targets.forEach(function (target) { From e54fcf22fe2e0d74b43090cd05a6a7e58d69ee32 Mon Sep 17 00:00:00 2001 From: Vincent Weevers Date: Fri, 19 Oct 2018 11:06:30 +0200 Subject: [PATCH 09/21] Moved seek tests to abstract-leveldown --- test/iterator-test.js | 208 +----------------------------------------- 1 file changed, 4 insertions(+), 204 deletions(-) diff --git a/test/iterator-test.js b/test/iterator-test.js index 60fee8f2..3203eacd 100644 --- a/test/iterator-test.js +++ b/test/iterator-test.js @@ -1,8 +1,9 @@ const make = require('./make') -const iota = require('iota-array') -const lexi = require('lexicographic-integer') -const util = require('util') +// const iota = require('iota-array') // TODO: remove if unused +// const lexi = require('lexicographic-integer') // TODO: remove if unused +// This test isn't included in abstract-leveldown because +// the empty-check is currently performed by leveldown. make('iterator#seek throws if target is empty', function (db, t, done) { var targets = ['', Buffer.alloc(0), []] var pending = targets.length @@ -27,71 +28,6 @@ make('iterator#seek throws if target is empty', function (db, t, done) { } }) -make('iterator is seekable', function (db, t, done) { - var ite = db.iterator() - ite.seek('two') - ite.next(function (err, key, value) { - t.error(err, 'no error') - t.same(key.toString(), 'two', 'key matches') - t.same(value.toString(), '2', 'value matches') - ite.next(function (err, key, value) { - t.error(err, 'no error') - t.same(key, undefined, 'end of iterator') - t.same(value, undefined, 'end of iterator') - ite.end(done) - }) - }) -}) - -make('iterator is seekable with buffer', function (db, t, done) { - var ite = db.iterator() - ite.seek(Buffer.from('two')) - ite.next(function (err, key, value) { - t.error(err, 'no error from next()') - t.equal(key.toString(), 'two', 'key matches') - t.equal(value.toString(), '2', 'value matches') - ite.next(function (err, key, value) { - t.error(err, 'no error from next()') - t.equal(key, undefined, 'end of iterator') - t.equal(value, undefined, 'end of iterator') - ite.end(done) - }) - }) -}) - -make('reverse seek in the middle', function (db, t, done) { - var ite = db.iterator({ reverse: true, limit: 1 }) - ite.seek('three!') - ite.next(function (err, key, value) { - t.error(err, 'no error') - t.same(key.toString(), 'three', 'key matches') - t.same(value.toString(), '3', 'value matches') - ite.end(done) - }) -}) - -make('iterator invalid seek', function (db, t, done) { - var ite = db.iterator() - ite.seek('zzz') - ite.next(function (err, key, value) { - t.error(err, 'no error') - t.same(key, undefined, 'end of iterator') - t.same(value, undefined, 'end of iterator') - ite.end(done) - }) -}) - -make('reverse seek from invalid range', function (db, t, done) { - var ite = db.iterator({ reverse: true }) - ite.seek('zzz') - ite.next(function (err, key, value) { - t.error(err, 'no error') - t.same(key.toString(), 'two') - t.same(value.toString(), '2') - ite.end(done) - }) -}) - make('iterator optimized for seek', function (db, t, done) { var batch = db.batch() batch.put('a', 1) @@ -130,24 +66,6 @@ make('iterator optimized for seek', function (db, t, done) { }) }) -make('iterator seek before next has completed', function (db, t, done) { - var ite = db.iterator() - var error - - ite.next(function (err, key, value) { - t.error(err, 'no error from next()') - ite.end(done) - }) - - try { - ite.seek('two') - } catch (err) { - error = err.message - } - - t.is(error, 'cannot call seek() before next() has completed', 'got error') -}) - make('close db with open iterator', function (db, t, done) { var ite = db.iterator() var cnt = 0 @@ -165,121 +83,3 @@ make('close db with open iterator', function (db, t, done) { done(false) }) }) - -make('iterator seek after end', function (db, t, done) { - var ite = db.iterator() - ite.next(function (err, key, value) { - t.error(err, 'no error from next()') - ite.end(function (err) { - t.error(err, 'no error from end()') - var error - - try { - ite.seek('two') - } catch (err) { - error = err.message - } - - t.is(error, 'cannot call seek() after end()', 'got error') - done() - }) - }) -}) - -make('iterator seek respects range', function (db, t, done) { - db.batch(pairs(10), function (err) { - t.error(err, 'no error from batch()') - - var pending = 0 - - expect({ gt: '5' }, '4', undefined) - expect({ gt: '5' }, '5', undefined) - expect({ gt: '5' }, '6', '6') - - expect({ gte: '5' }, '4', undefined) - expect({ gte: '5' }, '5', '5') - expect({ gte: '5' }, '6', '6') - - expect({ start: '5' }, '4', undefined) - expect({ start: '5' }, '5', '5') - expect({ start: '5' }, '6', '6') - - expect({ lt: '5' }, '4', '4') - expect({ lt: '5' }, '5', undefined) - expect({ lt: '5' }, '6', undefined) - - expect({ lte: '5' }, '4', '4') - expect({ lte: '5' }, '5', '5') - expect({ lte: '5' }, '6', undefined) - - expect({ end: '5' }, '4', '4') - expect({ end: '5' }, '5', '5') - expect({ end: '5' }, '6', undefined) - - expect({ lt: '5', reverse: true }, '4', '4') - expect({ lt: '5', reverse: true }, '5', undefined) - expect({ lt: '5', reverse: true }, '6', undefined) - - expect({ lte: '5', reverse: true }, '4', '4') - expect({ lte: '5', reverse: true }, '5', '5') - expect({ lte: '5', reverse: true }, '6', undefined) - - expect({ start: '5', reverse: true }, '4', '4') - expect({ start: '5', reverse: true }, '5', '5') - expect({ start: '5', reverse: true }, '6', undefined) - - expect({ gt: '5', reverse: true }, '4', undefined) - expect({ gt: '5', reverse: true }, '5', undefined) - expect({ gt: '5', reverse: true }, '6', '6') - - expect({ gte: '5', reverse: true }, '4', undefined) - expect({ gte: '5', reverse: true }, '5', '5') - expect({ gte: '5', reverse: true }, '6', '6') - - expect({ end: '5', reverse: true }, '4', undefined) - expect({ end: '5', reverse: true }, '5', '5') - expect({ end: '5', reverse: true }, '6', '6') - - expect({ gt: '7', lt: '8' }, '7', undefined) - expect({ gte: '7', lt: '8' }, '7', '7') - expect({ gte: '7', lt: '8' }, '8', undefined) - expect({ gt: '7', lte: '8' }, '8', '8') - - function expect (range, target, expected) { - pending++ - var ite = db.iterator(range) - - ite.seek(target) - ite.next(function (err, key, value) { - t.error(err, 'no error from next()') - - var tpl = 'seek(%s) on %s yields %s' - var msg = util.format(tpl, target, util.inspect(range), expected) - - if (expected === undefined) { - t.equal(value, undefined, msg) - } else { - t.equal(value.toString(), expected, msg) - } - - ite.end(function (err) { - t.error(err, 'no error from end()') - if (!--pending) done() - }) - }) - } - }) -}) - -function pairs (length, opts) { - opts = opts || {} - return iota(length).filter(not(opts.not)).map(function (k) { - var key = opts.lex ? lexi.pack(k, 'hex') : '' + k - return { type: 'put', key: key, value: '' + k } - }) -} - -function not (n) { - if (typeof n === 'function') return function (k) { return !n(k) } - return function (k) { return k !== n } -} From 1341e5d20764e0c175389eb5038c80f7082440f7 Mon Sep 17 00:00:00 2001 From: Vincent Weevers Date: Fri, 19 Oct 2018 11:08:25 +0200 Subject: [PATCH 10/21] Remove unused iota-array and lexicographic-integer devDependencies --- package.json | 2 -- test/iterator-test.js | 2 -- 2 files changed, 4 deletions(-) diff --git a/package.json b/package.json index 2344711a..14372686 100644 --- a/package.json +++ b/package.json @@ -27,10 +27,8 @@ "dependency-check": "^3.3.0", "du": "~0.1.0", "hallmark": "^0.1.0", - "iota-array": "^1.0.0", "level-concat-iterator": "^2.0.0", "level-community": "^3.0.0", - "lexicographic-integer": "^1.1.0", "mkfiletree": "^1.0.1", "monotonic-timestamp": "~0.0.8", "nyc": "^14.0.0", diff --git a/test/iterator-test.js b/test/iterator-test.js index 3203eacd..22647b3b 100644 --- a/test/iterator-test.js +++ b/test/iterator-test.js @@ -1,6 +1,4 @@ const make = require('./make') -// const iota = require('iota-array') // TODO: remove if unused -// const lexi = require('lexicographic-integer') // TODO: remove if unused // This test isn't included in abstract-leveldown because // the empty-check is currently performed by leveldown. From e99e12f0481fb67c10a7bee5b8ea5752ab8e0166 Mon Sep 17 00:00:00 2001 From: Vincent Weevers Date: Sun, 21 Oct 2018 10:51:19 +0300 Subject: [PATCH 11/21] Add segfault tests * Restore (but skip) segfault test from abstract-leveldown * Prevent segfault when calling iterator() on non-open db --- leveldown.js | 5 +++++ test/segfault-test.js | 43 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) create mode 100644 test/segfault-test.js diff --git a/leveldown.js b/leveldown.js index 7da510e9..5e2b9e88 100644 --- a/leveldown.js +++ b/leveldown.js @@ -87,6 +87,11 @@ LevelDOWN.prototype.getProperty = function (property) { } LevelDOWN.prototype._iterator = function (options) { + if (this.status !== 'open') { + // Prevent segfault + throw new Error('cannot call iterator() before open()') + } + return new Iterator(this, options) } diff --git a/test/segfault-test.js b/test/segfault-test.js new file mode 100644 index 00000000..ec3f0fd4 --- /dev/null +++ b/test/segfault-test.js @@ -0,0 +1,43 @@ +const test = require('tape') +const testCommon = require('./common') + +// Open issue: https://github.com/Level/leveldown/issues/157 +test.skip('close() does not segfault if there is a pending write', function (t) { + t.plan(3) + + const db = testCommon.factory() + + db.open(function (err) { + t.ifError(err, 'no open error') + + // The "sync" option seems to be a reliable way to trigger a segfault, + // but is not necessarily the cause of that segfault. More likely, it + // exposes a race condition that's already there. + db.put('foo', 'bar', { sync: true }, function (err) { + // We never get here, due to segfault. + t.ifError(err, 'no put error') + }) + + db.close(function (err) { + // We never get here, due to segfault. + t.ifError(err, 'no close error') + }) + }) +}) + +// See https://github.com/Level/leveldown/issues/134 +test('iterator() does not segfault if db is not open', function (t) { + t.plan(2) + + const db = testCommon.factory() + + try { + db.iterator() + } catch (err) { + t.is(err.message, 'cannot call iterator() before open()') + } + + db.close(function (err) { + t.ifError(err, 'no close error') + }) +}) From 27bed202552da5ecbc2dbaec1ca1b0747c673ee7 Mon Sep 17 00:00:00 2001 From: Vincent Weevers Date: Sun, 21 Oct 2018 15:52:41 +0300 Subject: [PATCH 12/21] Remove unused 'key' argument from ChainedBatch#_clear --- chained-batch.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/chained-batch.js b/chained-batch.js index 0b917672..f0f14cfe 100644 --- a/chained-batch.js +++ b/chained-batch.js @@ -14,8 +14,8 @@ ChainedBatch.prototype._del = function (key) { this.binding.del(key) } -ChainedBatch.prototype._clear = function (key) { - this.binding.clear(key) +ChainedBatch.prototype._clear = function () { + this.binding.clear() } ChainedBatch.prototype._write = function (options, callback) { From cacddad95c7344674c1a9b25c2f704294ce197b6 Mon Sep 17 00:00:00 2001 From: Vincent Weevers Date: Sun, 21 Oct 2018 15:53:18 +0300 Subject: [PATCH 13/21] Make callback of makeTest an error-first callback * make callback of makeTest an error-first callback * normalize error assertions in tests that use makeTest --- test/cleanup-hanging-iterators-test.js | 6 +++--- test/destroy-test.js | 24 ++++++++++++------------ test/iterator-test.js | 23 ++++++++++++++--------- test/make.js | 11 +++++++---- test/repair-test.js | 12 +++++++++--- 5 files changed, 45 insertions(+), 31 deletions(-) diff --git a/test/cleanup-hanging-iterators-test.js b/test/cleanup-hanging-iterators-test.js index 09d84b57..e1e54092 100644 --- a/test/cleanup-hanging-iterators-test.js +++ b/test/cleanup-hanging-iterators-test.js @@ -5,11 +5,11 @@ makeTest('test ended iterator', function (db, t, done) { var it = db.iterator({ keyAsBuffer: false, valueAsBuffer: false }) it.next(function (err, key, value) { - t.notOk(err, 'no error from next()') + t.ifError(err, 'no error from next()') t.equal(key, 'one', 'correct key') t.equal(value, '1', 'correct value') it.end(function (err) { - t.notOk(err, 'no error from next()') + t.ifError(err, 'no error from end()') done() }) }) @@ -19,7 +19,7 @@ makeTest('test non-ended iterator', function (db, t, done) { // no end() call on our iterator, cleanup should crash Node if not handled properly var it = db.iterator({ keyAsBuffer: false, valueAsBuffer: false }) it.next(function (err, key, value) { - t.notOk(err, 'no error from next()') + t.ifError(err, 'no error from next()') t.equal(key, 'one', 'correct key') t.equal(value, '1', 'correct value') done() diff --git a/test/destroy-test.js b/test/destroy-test.js index 7d8b44d0..96b88748 100644 --- a/test/destroy-test.js +++ b/test/destroy-test.js @@ -34,7 +34,7 @@ test('test destroy non-existent directory', function (t) { // Cleanup to avoid conflicts with other tests rimraf(location, { glob: false }, function (err) { - t.ifError(err, 'no rimraf error') + t.ifError(err, 'no error from rimraf()') leveldown.destroy(location, function () { t.is(arguments.length, 0, 'no arguments returned on callback') @@ -69,17 +69,17 @@ test('test destroy non leveldb directory', function (t) { } mkfiletree.makeTemp('destroy-test', tree, function (err, dir) { - t.ifError(err, 'no close error') + t.ifError(err, 'no error from makeTemp()') leveldown.destroy(dir, function (err) { - t.ifError(err, 'no destroy error') + t.ifError(err, 'no error from destroy()') readfiletree(dir, function (err, actual) { - t.ifError(err, 'no read error') + t.ifError(err, 'no error from readfiletree()') t.deepEqual(actual, tree, 'directory remains untouched') mkfiletree.cleanUp(function (err) { - t.ifError(err, 'no cleanup error') + t.ifError(err, 'no error from cleanup()') t.end() }) }) @@ -90,13 +90,13 @@ test('test destroy non leveldb directory', function (t) { makeTest('test destroy() cleans and removes leveldb-only dir', function (db, t, done) { var location = db.location db.close(function (err) { - t.ifError(err, 'no close error') + t.ifError(err, 'no error from close()') leveldown.destroy(location, function (err) { - t.ifError(err, 'no destroy error') + t.ifError(err, 'no error from destroy()') t.notOk(fs.existsSync(location), 'directory completely removed') - done(false) + done(null, false) }) }) }) @@ -106,16 +106,16 @@ makeTest('test destroy() cleans and removes only leveldb parts of a dir', functi fs.writeFileSync(path.join(location, 'foo'), 'FOO') db.close(function (err) { - t.ifError(err, 'no close error') + t.ifError(err, 'no error from close()') leveldown.destroy(location, function (err) { - t.ifError(err, 'no destroy error') + t.ifError(err, 'no error from destroy()') readfiletree(location, function (err, tree) { - t.ifError(err, 'no read error') + t.ifError(err, 'no error from readfiletree()') t.deepEqual(tree, { 'foo': 'FOO' }, 'non-leveldb files left intact') - done(false) + done(null, false) }) }) }) diff --git a/test/iterator-test.js b/test/iterator-test.js index 22647b3b..40f59bfe 100644 --- a/test/iterator-test.js +++ b/test/iterator-test.js @@ -21,7 +21,7 @@ make('iterator#seek throws if target is empty', function (db, t, done) { }) function end (err) { - t.error(err, 'no error from end()') + t.ifError(err, 'no error from end()') if (!--pending) done() } }) @@ -37,23 +37,23 @@ make('iterator optimized for seek', function (db, t, done) { batch.put('g', 1) batch.write(function (err) { var ite = db.iterator() - t.error(err, 'no error from batch') + t.ifError(err, 'no error from batch()') ite.next(function (err, key, value) { - t.error(err, 'no error from next()') + t.ifError(err, 'no error from next()') t.equal(key.toString(), 'a', 'key matches') t.equal(ite.cache.length, 0, 'no cache') ite.next(function (err, key, value) { - t.error(err, 'no error from next()') + t.ifError(err, 'no error from next()') t.equal(key.toString(), 'b', 'key matches') t.ok(ite.cache.length > 0, 'has cached items') ite.seek('d') t.notOk(ite.cache, 'cache is removed') ite.next(function (err, key, value) { - t.error(err, 'no error from next()') + t.ifError(err, 'no error from next()') t.equal(key.toString(), 'd', 'key matches') t.equal(ite.cache.length, 0, 'no cache') ite.next(function (err, key, value) { - t.error(err, 'no error from next()') + t.ifError(err, 'no error from next()') t.equal(key.toString(), 'e', 'key matches') t.ok(ite.cache.length > 0, 'has cached items') ite.end(done) @@ -67,17 +67,22 @@ make('iterator optimized for seek', function (db, t, done) { make('close db with open iterator', function (db, t, done) { var ite = db.iterator() var cnt = 0 + var hadError = false + ite.next(function loop (err, key, value) { if (cnt++ === 0) { - t.error(err, 'no error from next()') + t.ifError(err, 'no error from next()') } else { t.equal(err.message, 'iterator has ended') + hadError = true } if (key !== undefined) { ite.next(loop) } }) db.close(function (err) { - t.error(err, 'no error from close()') - done(false) + t.ifError(err, 'no error from close()') + t.ok(hadError) + + done(null, false) }) }) diff --git a/test/make.js b/test/make.js index d50d6397..61fff6d3 100644 --- a/test/make.js +++ b/test/make.js @@ -4,24 +4,27 @@ const testCommon = require('./common') function makeTest (name, testFn) { test(name, function (t) { var db = testCommon.factory() - var done = function (close) { + var done = function (err, close) { + t.ifError(err, 'no error from done()') + if (close === false) { t.end() return } + db.close(function (err) { - t.error(err, 'no error from close()') + t.ifError(err, 'no error from close()') t.end() }) } db.open(function (err) { - t.error(err, 'no error from open()') + t.ifError(err, 'no error from open()') db.batch([ { type: 'put', key: 'one', value: '1' }, { type: 'put', key: 'two', value: '2' }, { type: 'put', key: 'three', value: '3' } ], function (err) { - t.error(err, 'no error from batch()') + t.ifError(err, 'no error from batch()') testFn(db, t, done) }) }) diff --git a/test/repair-test.js b/test/repair-test.js index b3bce259..6591b770 100644 --- a/test/repair-test.js +++ b/test/repair-test.js @@ -29,16 +29,22 @@ test('test repair non-existent directory returns error', function (t) { // a proxy indicator that RepairDB is being called and doing its thing makeTest('test repair() compacts', function (db, t, done) { var location = db.location + db.close(function (err) { - t.notOk(err, 'no error') + t.ifError(err, 'no error from close()') + var files = fs.readdirSync(location) t.ok(files.some(function (f) { return (/\.log$/).test(f) }), 'directory contains log file(s)') t.notOk(files.some(function (f) { return (/\.sst$/).test(f) }), 'directory does not contain sst file(s)') - leveldown.repair(location, function () { + + leveldown.repair(location, function (err) { + t.ifError(err, 'no error from repair()') + files = fs.readdirSync(location) t.notOk(files.some(function (f) { return (/\.log$/).test(f) }), 'directory does not contain log file(s)') t.ok(files.some(function (f) { return (/\.sst$/).test(f) }), 'directory contains sst file(s)') - done(false) + + done(null, false) }) }) }) From 61f59e229d75850765bd7a8a6e131fc0d1472108 Mon Sep 17 00:00:00 2001 From: Vincent Weevers Date: Sat, 24 Nov 2018 22:49:27 +0200 Subject: [PATCH 14/21] Serialize compactRange() arguments * handle errors in compactRange test * use batch() in compactRange test * serialize compactRange() start and end * copy type checks from approximateSize to compactRange --- leveldown.js | 16 +++++++++++- test/compact-range-test.js | 50 +++++++++++++++++++++++++++----------- 2 files changed, 51 insertions(+), 15 deletions(-) diff --git a/leveldown.js b/leveldown.js index 5e2b9e88..5d4f0d77 100644 --- a/leveldown.js +++ b/leveldown.js @@ -63,7 +63,7 @@ LevelDOWN.prototype.approximateSize = function (start, end, callback) { end == null || typeof start === 'function' || typeof end === 'function') { - throw new Error('approximateSize() requires valid `start`, `end` and `callback` arguments') + throw new Error('approximateSize() requires valid `start` and `end` arguments') } if (typeof callback !== 'function') { @@ -77,6 +77,20 @@ LevelDOWN.prototype.approximateSize = function (start, end, callback) { } LevelDOWN.prototype.compactRange = function (start, end, callback) { + if (start == null || + end == null || + typeof start === 'function' || + typeof end === 'function') { + throw new Error('compactRange() requires valid `start` and `end` arguments') + } + + if (typeof callback !== 'function') { + throw new Error('compactRange() requires a callback argument') + } + + start = this._serializeKey(start) + end = this._serializeKey(end) + this.binding.compactRange(start, end, callback) } diff --git a/test/compact-range-test.js b/test/compact-range-test.js index 9c1b55ee..d83263f2 100644 --- a/test/compact-range-test.js +++ b/test/compact-range-test.js @@ -15,20 +15,26 @@ test('test compactRange() frees disk space after key deletion', function (t) { var key2 = '000001' var val1 = Buffer.allocUnsafe(64).fill(1) var val2 = Buffer.allocUnsafe(64).fill(1) - db.put(key1, val1, function () { - db.put(key2, val2, function () { - db.compactRange(key1, key2, function () { - db.approximateSize('0', 'z', function (err, sizeAfterPuts) { - t.error(err, 'no error') - db.del(key1, function () { - db.del(key2, function () { - db.compactRange(key1, key2, function () { - db.approximateSize('0', 'z', function (err, sizeAfterCompact) { - t.error(err, 'no error') - t.ok(sizeAfterCompact < sizeAfterPuts) - t.end() - }) - }) + + db.batch().put(key1, val1).put(key2, val2).write(function (err) { + t.ifError(err, 'no batch put error') + + db.compactRange(key1, key2, function (err) { + t.ifError(err, 'no compactRange1 error') + + db.approximateSize('0', 'z', function (err, sizeAfterPuts) { + t.error(err, 'no approximateSize1 error') + + db.batch().del(key1).del(key2).write(function (err) { + t.ifError(err, 'no batch del error') + + db.compactRange(key1, key2, function (err) { + t.ifError(err, 'no compactRange2 error') + + db.approximateSize('0', 'z', function (err, sizeAfterCompact) { + t.error(err, 'no approximateSize2 error') + t.ok(sizeAfterCompact < sizeAfterPuts) + t.end() }) }) }) @@ -37,6 +43,22 @@ test('test compactRange() frees disk space after key deletion', function (t) { }) }) +test('test compactRange() serializes start and end', function (t) { + t.plan(3) + + var clone = Object.create(db) + var count = 0 + + clone._serializeKey = function (key) { + t.is(key, count++) + return db._serializeKey(key) + } + + clone.compactRange(0, 1, function (err) { + t.ifError(err, 'no compactRange error') + }) +}) + test('tearDown', function (t) { db.close(testCommon.tearDown.bind(null, t)) }) From f65d0bfae3945bdd9637034753444e96d01339f9 Mon Sep 17 00:00:00 2001 From: Lars-Magnus Skog Date: Sun, 16 Dec 2018 16:18:11 +0100 Subject: [PATCH 15/21] Remove 32 bits from appveyor --- appveyor.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/appveyor.yml b/appveyor.yml index 75800844..abfcf16f 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -9,7 +9,6 @@ environment: - nodejs_version: "10" platform: - - x86 - x64 install: From ce2f69611b96fbce3df7ab10563d46256caff4a2 Mon Sep 17 00:00:00 2001 From: Lars-Magnus Skog Date: Sun, 16 Dec 2018 23:24:35 +0100 Subject: [PATCH 16/21] Remove xcacheSize and xmaxOpenFiles from leak tests --- test/leak-tester-batch.js | 2 +- test/leak-tester.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/leak-tester-batch.js b/test/leak-tester-batch.js index 6739a71e..03315119 100644 --- a/test/leak-tester-batch.js +++ b/test/leak-tester-batch.js @@ -75,7 +75,7 @@ var run = CHAINED } db = testCommon.factory() -db.open({ xcacheSize: 0, xmaxOpenFiles: 10 }, function () { +db.open(function () { rssBase = process.memoryUsage().rss run() }) diff --git a/test/leak-tester.js b/test/leak-tester.js index 504ae834..c745d3b0 100644 --- a/test/leak-tester.js +++ b/test/leak-tester.js @@ -43,7 +43,7 @@ function run () { } db = testCommon.factory() -db.open({ xcacheSize: 0, xmaxOpenFiles: 10 }, function () { +db.open(function () { rssBase = process.memoryUsage().rss run() }) From ef4af1d447f66e6b1ddbc7e020e8da590457a8dc Mon Sep 17 00:00:00 2001 From: Vincent Weevers Date: Sun, 31 Mar 2019 23:53:06 +0200 Subject: [PATCH 17/21] Remove slump devDependency --- bench/db-bench.js | 2 +- package.json | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/bench/db-bench.js b/bench/db-bench.js index 8b9080af..b245a168 100755 --- a/bench/db-bench.js +++ b/bench/db-bench.js @@ -6,6 +6,7 @@ const du = require('du') const path = require('path') const argv = require('optimist').argv +const crypto = require('crypto') const options = { benchmark: argv.benchmark, @@ -20,7 +21,6 @@ const options = { throughputOutput: argv.throughputOutput } -const randomString = require('slump').string const keyTmpl = '0000000000000000' if (!options.useExisting) { diff --git a/package.json b/package.json index 14372686..cb91ef8f 100644 --- a/package.json +++ b/package.json @@ -37,7 +37,6 @@ "prebuild-ci": "^2.0.0", "readfiletree": "~0.0.1", "rimraf": "^2.6.1", - "slump": "^3.0.0", "standard": "^12.0.0", "tape": "^4.10.0", "tempy": "^0.2.1", From 0ea7a85994cb097a0ade9a116e26e3a53a0440f6 Mon Sep 17 00:00:00 2001 From: Vincent Weevers Date: Sun, 7 Apr 2019 12:01:16 +0200 Subject: [PATCH 18/21] Remove outdated benchmarks altogether --- bench/db-bench-plot.sh | 56 ------------------ bench/db-bench.js | 118 ------------------------------------- bench/memory.js | 98 ------------------------------ bench/write-random-plot.sh | 24 -------- bench/write-random.js | 59 ------------------- bench/write-sorted-plot.sh | 22 ------- bench/write-sorted.js | 59 ------------------- package.json | 3 +- 8 files changed, 1 insertion(+), 438 deletions(-) delete mode 100755 bench/db-bench-plot.sh delete mode 100755 bench/db-bench.js delete mode 100644 bench/memory.js delete mode 100755 bench/write-random-plot.sh delete mode 100644 bench/write-random.js delete mode 100755 bench/write-sorted-plot.sh delete mode 100644 bench/write-sorted.js diff --git a/bench/db-bench-plot.sh b/bench/db-bench-plot.sh deleted file mode 100755 index 4a8293db..00000000 --- a/bench/db-bench-plot.sh +++ /dev/null @@ -1,56 +0,0 @@ -#!/bin/sh - -gnuplot <= options.concurrency || totalWrites > options.num) return - - inProgress++ - - if (totalWrites % 100000 === 0) { - console.log('' + inProgress, totalWrites, Math.round(totalWrites / options.num * 100) + '%') - } - - if (totalWrites % 1000 === 0) { - elapsed = Date.now() - startTime - timesStream.write( - elapsed + - ',' + totalWrites + - ',' + totalBytes + - ',' + Math.floor(timesAccum / 1000) + - ',' + (Math.floor(((totalBytes / 1048576) / (elapsed / 1000)) * 100) / 100) + - '\n') - timesAccum = 0 - } - - var time = process.hrtime() - - db.put(make16CharPaddedKey(), randomString({ length: options.valueSize }), function (err) { - if (err) throw err - - totalBytes += keyTmpl.length + options.valueSize - timesAccum += process.hrtime(time)[1] - inProgress-- - process.nextTick(write) - }) - } - - for (var i = 0; i < options.concurrency; i++) { - write() - } -} - -setTimeout(function () { - db.open({ - errorIfExists: false, - createIfMissing: true, - cacheSize: options.cacheSize << 20, - writeBufferSize: options.writeBufferSize << 20 - }, function (err) { - if (err) throw err - start() - }) -}, 500) diff --git a/bench/memory.js b/bench/memory.js deleted file mode 100644 index c40bc31d..00000000 --- a/bench/memory.js +++ /dev/null @@ -1,98 +0,0 @@ -var leveldown = require('../') -var path = require('path') - -var addr = '1111111111111111111114oLvT2' - -var db = leveldown(path.join(process.env.HOME, 'iterleak.db')) -var records = { - 'w/a/14r6JPSJNzBXXJEM2jnmoybQCw3arseKuY/primary': '00', - 'w/a/17nJuKqjTyAeujSJnPCebpSTEz1v9kjNKg/primary': '00', - 'w/a/18cxWLCiJMESL34Ev1LJ2meGTgL14bAxfj/primary': '00', - 'w/a/18pghEAnqCRTrjd7iyUj6XNKmSNx4fAywB/primary': '00', - 'w/a/19EWPPzY6XkQeM7DxqJ4THbY3DGekRQKbt/primary': '00', - 'w/a/1DKDeLQyBCkV5aMG15XbAdpwwHnxqw9rvY/primary': '00', - 'w/a/1HSJAoU5TaGKhAJxwpTC1imiMM1ab8SFGW/primary': '00', - 'w/a/1HkeafxVvStf2Np6wxUjkTpCt1gTDJSLpi/primary': '00', - 'w/a/1Hr5JduPFdZ4n4dHBUqRoxLQEG4P93C658/primary': '00', - 'w/a/1KATodK9Ko8MchJZzDxLhjWz4d8oAuCqEh/primary': '00', - 'w/a/1NhRKhiAAJrmwXoLhL9dGG1z6oMiFGrxZ7/primary': '00', - 'w/a/1yTq3DpyUNqUCxDttczGjufbEBKAXMTSq/primary': '00', - 'w/w/primary': '00' -} - -db.open({ - createIfMissing: true, - errorIfExists: false, - compression: true, - cacheSize: 8 << 20, - writeBufferSize: 4 << 20, - maxOpenFiles: 8192 -}, function (err) { - if (err) throw err - - memory() - - var batch = db.batch() - Object.keys(records).forEach(function (key) { - var value = Buffer.from(records[key], 'hex') - batch.put(key, value) - }) - - batch.write(function (err) { - if (err) throw err - - // This will leak roughly 1mb per call. - setTimeout(function self () { - var i = 0 - ;(function next (err) { - if (err) throw err - if (i++ >= 10000) { - memory() - return setTimeout(self, 1000) - } - iterate(addr, next) - })() - }, 1000) - }) -}) - -function memory () { - var mem = process.memoryUsage() - console.log('Memory: rss=%dmb, js-heap=%d/%dmb native-heap=%dmb', - mb(mem.rss), - mb(mem.heapUsed), - mb(mem.heapTotal), - mb(mem.rss - mem.heapTotal)) -} - -function mb (size) { - return size / 1024 / 1024 | 0 -} - -function iterate (address, callback) { - var iter = db.iterator({ - gte: 'w/a/' + address, - lte: 'w/a/' + address + '~', - keys: true, - values: false, - fillCache: false, - keyAsBuffer: false - }) - - ;(function next () { - iter.next(function (err, key, value) { - if (err) { - return iter.end(function (e) { - if (e) throw e - callback(err) - }) - } - - if (key === undefined) { - return iter.end(callback) - } - - next() - }) - })() -} diff --git a/bench/write-random-plot.sh b/bench/write-random-plot.sh deleted file mode 100755 index f9294ae3..00000000 --- a/bench/write-random-plot.sh +++ /dev/null @@ -1,24 +0,0 @@ -#!/bin/sh - -gnuplot <= concurrency || totalWrites > entryCount) return - - var time = process.hrtime() - inProgress++ - - db.put(uuid.v4(), data, function (err) { - if (err) throw err - writeBuf += (Date.now() - startTime) + ',' + process.hrtime(time)[1] + '\n' - inProgress-- - process.nextTick(write) - }) - - process.nextTick(write) - } - - write() -}) diff --git a/bench/write-sorted-plot.sh b/bench/write-sorted-plot.sh deleted file mode 100755 index 24ee173a..00000000 --- a/bench/write-sorted-plot.sh +++ /dev/null @@ -1,22 +0,0 @@ -#!/bin/sh - -gnuplot <= concurrency || totalWrites > entryCount) return - - var time = process.hrtime() - inProgress++ - - db.put(timestamp(), data, function (err) { - if (err) throw err - writeBuf += (Date.now() - startTime) + ',' + process.hrtime(time)[1] + '\n' - inProgress-- - process.nextTick(write) - }) - - process.nextTick(write) - } - - write() -}) diff --git a/package.json b/package.json index cb91ef8f..cdfde2e1 100644 --- a/package.json +++ b/package.json @@ -10,7 +10,7 @@ "coverage": "nyc report --reporter=text-lcov | coveralls", "rebuild": "prebuild --compile", "hallmark": "hallmark --fix", - "dependency-check": "dependency-check . test/*.js bench/*.js", + "dependency-check": "dependency-check . test/*.js", "prepublishOnly": "npm run dependency-check" }, "dependencies": { @@ -32,7 +32,6 @@ "mkfiletree": "^1.0.1", "monotonic-timestamp": "~0.0.8", "nyc": "^14.0.0", - "optimist": "~0.6.1", "prebuild": "^8.0.0", "prebuild-ci": "^2.0.0", "readfiletree": "~0.0.1", From c50d820cd00e4fb706380f7d0710f644cea6a462 Mon Sep 17 00:00:00 2001 From: Vincent Weevers Date: Sun, 7 Apr 2019 12:26:40 +0200 Subject: [PATCH 19/21] Use tempy in open-read-only-test --- test/open-read-only-test.js | 82 +++++++++++++++---------------------- 1 file changed, 34 insertions(+), 48 deletions(-) diff --git a/test/open-read-only-test.js b/test/open-read-only-test.js index c05ed5e8..5c8961b1 100644 --- a/test/open-read-only-test.js +++ b/test/open-read-only-test.js @@ -1,39 +1,36 @@ 'use strict' const test = require('tape') -const leveldown = require('../') -const testCommon = require('abstract-leveldown/testCommon') +const leveldown = require('..') +const tempy = require('tempy') const fs = require('fs') const path = require('path') -var location = testCommon.location() +const location = tempy.directory() // This is used because it's not sufficient on windows to set a parent folder as readonly -function chmodFilesSync (dir, mode) { - var files = fs.readdirSync(dir) - files.forEach(function (file) { - var fullPath = path.join(dir, file) - fs.chmodSync(fullPath, mode) +function chmodRecursive (mode) { + fs.readdirSync(location).forEach(function (file) { + fs.chmodSync(path.join(location, file), mode) }) + fs.chmodSync(location, mode) } -test('setUp', function (t) { - // just in case we thew an error last time and don't have perms to remove db - if (fs.existsSync(location)) { - fs.chmodSync(location, 0o755) - chmodFilesSync(location, 0o755) - } - testCommon.setUp(t) -}) +function factory (mode) { + if (mode != null) chmodRecursive(mode) + return leveldown(location) +} test('test write to read/write database', function (t) { - var db = leveldown(location) + const db = factory() + db.open(function (err) { - t.error(err) + t.ifError(err, 'no error from open()') + db.put('my key', 'my value', function (err) { - t.error(err, 'no write error') + t.ifError(err, 'no error from put()') db.get('my key', function (err, value) { - t.error(err, 'no read error') + t.ifError(err, 'no error from get()') t.equal(value.toString(), 'my value', 'correct value') db.close(t.end.bind(t)) }) @@ -42,9 +39,8 @@ test('test write to read/write database', function (t) { }) test('test throw error reading read-only database', function (t) { - chmodFilesSync(location, 0o555) - fs.chmodSync(location, 0o555) - var db = leveldown(location) + const db = factory(0o555) + db.open(function (err) { t.ok(err, 'should get error reading read only database') t.ok(/IO Error/i.test(err && err.message), 'should get io error') @@ -53,13 +49,13 @@ test('test throw error reading read-only database', function (t) { }) test('test read from a read-only database if readOnly is true', function (t) { - chmodFilesSync(location, 0o555) - fs.chmodSync(location, 0o555) - var db = leveldown(location) + const db = factory(0o555) + db.open({ readOnly: true }, function (err) { - t.error(err) + t.ifError(err, 'no error from open()') + db.get('my key', function (err, value) { - t.error(err, 'no read error') + t.ifError(err, 'no error from get()') t.equal(value.toString(), 'my value', 'correct value') db.close(t.end.bind(t)) }) @@ -67,9 +63,8 @@ test('test read from a read-only database if readOnly is true', function (t) { }) test('test throw error reading read-only database if readOnly is false', function (t) { - chmodFilesSync(location, 0o555) - fs.chmodSync(location, 0o555) - var db = leveldown(location) + const db = factory(0o555) + db.open({ readOnly: false }, function (err) { t.ok(err, 'should get error reading read only database') t.ok(/IO Error/i.test(err && err.message), 'should get io error') @@ -78,11 +73,11 @@ test('test throw error reading read-only database if readOnly is false', functio }) test('test throw error putting data to read-only db if readOnly is true', function (t) { - chmodFilesSync(location, 0o555) - fs.chmodSync(location, 0o555) - var db = leveldown(location) + const db = factory(0o555) + db.open({ readOnly: true }, function (err) { - t.error(err) + t.ifError(err, 'no error from open()') + db.put('my key', 'my value', function (err) { t.ok(err, 'should get write error') t.ok(/Not supported operation in read only mode/i.test(err && err.message), 'should get io error') @@ -92,11 +87,11 @@ test('test throw error putting data to read-only db if readOnly is true', functi }) test('test throw error deleting data from read-only db if readOnly is true', function (t) { - chmodFilesSync(location, 0o555) - fs.chmodSync(location, 0o555) - var db = leveldown(location) + const db = factory(0o555) + db.open({ readOnly: true }, function (err) { - t.error(err) + t.ifError(err, 'no error from open()') + db.del('my key', function (err) { t.ok(err, 'should get write error') t.ok(/Not supported operation in read only mode/i.test(err && err.message), 'should get io error') @@ -104,12 +99,3 @@ test('test throw error deleting data from read-only db if readOnly is true', fun }) }) }) - -test('tearDown', function (t) { - // just in case we thew an error last time and don't have perms to remove db - if (fs.existsSync(location)) { - fs.chmodSync(location, 0o755) - chmodFilesSync(location, 0o755) - } - testCommon.tearDown(t) -}) From 31f83a67e1731364d3fcfbca8f15e0e112863b07 Mon Sep 17 00:00:00 2001 From: Vincent Weevers Date: Sun, 7 Apr 2019 12:40:16 +0200 Subject: [PATCH 20/21] Update comment in segfault-test --- test/segfault-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/segfault-test.js b/test/segfault-test.js index ec3f0fd4..e35210cc 100644 --- a/test/segfault-test.js +++ b/test/segfault-test.js @@ -1,7 +1,7 @@ const test = require('tape') const testCommon = require('./common') -// Open issue: https://github.com/Level/leveldown/issues/157 +// See https://github.com/Level/leveldown/issues/157, not yet ported to rocksdb. test.skip('close() does not segfault if there is a pending write', function (t) { t.plan(3) From dd18a9890f13b8b047a6004e1cfd618e5fd8ae90 Mon Sep 17 00:00:00 2001 From: Vincent Weevers Date: Fri, 26 Apr 2019 17:25:16 +0200 Subject: [PATCH 21/21] Fix asynchronicity of empty chained batch --- src/batch.cc | 20 +++++++------------- src/batch.h | 3 ++- src/batch_async.cc | 4 +++- 3 files changed, 12 insertions(+), 15 deletions(-) diff --git a/src/batch.cc b/src/batch.cc index 88572352..1472c3fc 100644 --- a/src/batch.cc +++ b/src/batch.cc @@ -130,19 +130,13 @@ NAN_METHOD(Batch::Clear) { NAN_METHOD(Batch::Write) { Batch* batch = ObjectWrap::Unwrap(info.Holder()); - if (batch->hasData) { - Nan::Callback *callback = - new Nan::Callback(v8::Local::Cast(info[0])); - BatchWriteWorker* worker = new BatchWriteWorker(batch, callback); - // persist to prevent accidental GC - v8::Local _this = info.This(); - worker->SaveToPersistent("batch", _this); - Nan::AsyncQueueWorker(worker); - } else { - LD_RUN_CALLBACK("rocksdb::batch.write", - v8::Local::Cast(info[0]), - 0, NULL); - } + Nan::Callback *callback = + new Nan::Callback(v8::Local::Cast(info[0])); + BatchWriteWorker* worker = new BatchWriteWorker(batch, callback); + // persist to prevent accidental GC + v8::Local _this = info.This(); + worker->SaveToPersistent("batch", _this); + Nan::AsyncQueueWorker(worker); } } // namespace leveldown diff --git a/src/batch.h b/src/batch.h index 49d166c2..d6bcbc17 100644 --- a/src/batch.h +++ b/src/batch.h @@ -22,11 +22,12 @@ class Batch : public Nan::ObjectWrap { ~Batch (); rocksdb::Status Write (); + bool hasData; // keep track of whether we're writing data or not + private: leveldown::Database* database; rocksdb::WriteOptions* options; rocksdb::WriteBatch* batch; - bool hasData; // keep track of whether we're writing data or not static NAN_METHOD(New); static NAN_METHOD(Put); diff --git a/src/batch_async.cc b/src/batch_async.cc index fbb56f4e..db3846f7 100644 --- a/src/batch_async.cc +++ b/src/batch_async.cc @@ -16,7 +16,9 @@ BatchWriteWorker::BatchWriteWorker ( BatchWriteWorker::~BatchWriteWorker () {} void BatchWriteWorker::Execute () { - SetStatus(batch->Write()); + if (batch->hasData) { + SetStatus(batch->Write()); + } } } // namespace leveldown