make testCommon.js the default value for testCommon parameter + remove testBuffer

[ralphtheninja]

Closes #163

Also removes testBuffer in put-get-del-test.js

[ralphtheninja]

Note that the order of testBuffer and testCommon parameters was swapped. So you can do:

Use default testBuffer and default testCommon

abstract.all(leveldown, test)

Use custom testBuffer and default testCommon

abstract.all(leveldown, test, testBuffer)

Or custom testBuffer and custom testCommon

abstract.all(leveldown, test, testBuffer, testCommon)

[ralphtheninja]

I chose this approach because leveldown, leveldown-hyper, rocksdb, memdown and medeadown all use testCommon from abstract-leveldown but also pass in a custom buffer.

[vweevers]

Didn't we have the intention to remove testBuffer?

[ralphtheninja]

@vweevers Yeah, but I got cold feet. What if it's important for implementations to specify their custom buffers?

[ralphtheninja]

I'd still like to remove it though :)

[vweevers]

I'm also not 100% sure. Mainly because we don't know the original motivation or use case (if there ever was one).

The only case I can think of is if they do some kind of prefixing or encoding, for example, add my-db~ to every key. Then with a custom testBuffer they could test edge cases, like putting a buffer that is the same as the prefix. They should probably have their own tests for that though.

[ralphtheninja]

I'll do a grep of this over the most common dependents to get some more information. Lets leave this hanging a little bit.

[ralphtheninja]

See complete search log at the bottom.

Summary 1

• asyncstorage-down passes in simple string buffer to abstract put-get-del-test
• jsondown reads its own test.js file and passes to abstract put-get-del-test
• azureleveldown has copied the abstract test, i.e. not using tests from abstract-leveldown
• rocksdb reads data/testdata.bin and passes to abstract put-get-del-test (usage comes from leveldown)
• riakdown reads custom testdata.bin (actually copied from leveldown, which is a PNG image) and passes to abstract put-get-del-test
• leveldown reads data/testdata.bin and passes to abstract put-get-del-test (PNG image)
• sqldown passes in simple string buffer to abstract put-get-del-test
• level.js passes in simple string buffer to abstract put-get-del-test
• memdown passes in custom buffer (base64 encoded) and passes to abstract put-get-del-test
• leveldown-hyper reads data/testdata.bin and passes to abstract put-get-del-test (usage comes from leveldown)
• redisdown passes in simple string buffer to abstract put-get-del-test
• medeadown reads its own test.js file and passes to abstract put-get-del-test
• mysqldown reads custom testdata.bin (actually copied from leveldown, which is a PNG image) and passes to abstract put-get-del-test
• fruitdown passes in simple string buffer to abstract put-get-del-test
• localstorage-down passes in simple string buffer to abstract put-get-del-test

Summary 2

1. asyncstorage-down, sqldown, level.js, redisdown, fruitdown, localstorage-down all pass in Buffer('foo') (or Buffer('hello'))
2. jsondown and medeadown reads its own test.js file and passes to abstract put-get-del-test
3. rocksdb, riakdown, leveldown, leveldown-hyper and mysqldown reads a PNG file and passes to abstract put-get-del-test
4. memdown passes in custom buffer (base64 encoded) to abstract put-get-del-test
5. azureleveldown has copied the abstract test, i.e. not using tests from abstract-leveldown

Conclusion

1 just pass a string and seem to have no meaning.

2 and 3 are essentially doing the same thing, only difference from 1 is length of buffer (as I see it) and the actual file content doesn't have any real meaning.

4 is basically the same as 1 with the only difference that the buffer is generated from a base64 encoded string

5 doesn't use abstract put-get-del-test

No one seems to be doing any funky stuff so we should be good by removing testBuffer completely. However, if we want we could add a test which reads a file (e.g. the PNG or whatever) to cover a larger buffer.

Search output

asyncstorage-down/test/test.js:5-var testCommon = require('./testCommon');
asyncstorage-down/test/test.js:6:var testBuffer = new Buffer('hello');
asyncstorage-down/test/test.js:7-
--
asyncstorage-down/test/test.js:12-require('abstract-leveldown/abstract/get-test').all(asyncstorage, tape, testCommon);
asyncstorage-down/test/test.js:13:require('abstract-leveldown/abstract/put-get-del-test').all(asyncstorage, tape, testCommon, testBuffer);
asyncstorage-down/test/test.js:14-require('abstract-leveldown/abstract/close-test').close(asyncstorage, tape, testCommon);
jsondown/test.js:3-  jsonDOWN = require("./jsondown"),
jsondown/test.js:4:  testBuffer = require("fs").readFileSync(__filename),
jsondown/test.js:5-  db;
--
jsondown/test.js:18-
jsondown/test.js:19:require("abstract-leveldown/abstract/put-get-del-test").all(jsonDOWN, test, testCommon, testBuffer);
jsondown/test.js:20-
azureleveldown/abstract/put-get-del-test.js:4-var db
azureleveldown/abstract/put-get-del-test.js:5:  , testBuffer
azureleveldown/abstract/put-get-del-test.js:6-  , test
--
azureleveldown/abstract/put-get-del-test.js:122-    // Buffer key
azureleveldown/abstract/put-get-del-test.js:123:    makePutGetDelSuccessfulTest('Buffer key', testBuffer, 'foo')
azureleveldown/abstract/put-get-del-test.js:124-  }
--
azureleveldown/abstract/put-get-del-test.js:157-  // standard Buffer value
azureleveldown/abstract/put-get-del-test.js:158:  makePutGetDelSuccessfulTest('Buffer value', 'foo', testBuffer)
azureleveldown/abstract/put-get-del-test.js:159-
--
azureleveldown/abstract/put-get-del-test.js:172-module.exports.all = function (leveldown, testFunc, testCommon, buffer, BufferType) {
azureleveldown/abstract/put-get-del-test.js:173:  testBuffer = buffer
azureleveldown/abstract/put-get-del-test.js:174-  test = testFunc
rocksdb/test/put-get-del-test.js:5-    , path       = require('path')
rocksdb/test/put-get-del-test.js:6:    , testBuffer = fs.readFileSync(path.join(__dirname, 'data/testdata.bin'))
rocksdb/test/put-get-del-test.js:7-    , abstract   = require('abstract-leveldown/abstract/put-get-del-test')
rocksdb/test/put-get-del-test.js:8-
rocksdb/test/put-get-del-test.js:9:abstract.all(leveldown, test, testCommon, testBuffer)
rocksdb/test/put-get-del-test.js:10-
riakdown/test.js:2-var testCommon = require('./testCommon');
riakdown/test.js:3:var testBuffer = require('fs').readFileSync('./testdata.bin');
riakdown/test.js:4-var RiakDOWN = require('./');
--
riakdown/test.js:16-
riakdown/test.js:17:require('abstract-leveldown/abstract/put-get-del-test').all(RiakDOWN, test, testCommon, testBuffer, process.browser && Uint8Array);
riakdown/test.js:18-
leveldown/test/put-get-del-test.js:5-    , path       = require('path')
leveldown/test/put-get-del-test.js:6:    , testBuffer = fs.readFileSync(path.join(__dirname, 'data/testdata.bin'))
leveldown/test/put-get-del-test.js:7-    , abstract   = require('abstract-leveldown/abstract/put-get-del-test')
leveldown/test/put-get-del-test.js:8-
leveldown/test/put-get-del-test.js:9:abstract.all(leveldown, test, testCommon, testBuffer)
leveldown/test/put-get-del-test.js:10-
sqldown/test/test.js:5-
sqldown/test/test.js:6:var testBuffer = new Buffer('foo')
sqldown/test/test.js:7-
--
sqldown/test/test.js:14-  require('abstract-leveldown/abstract/get-test').all(leveljs, tape, testCommon);
sqldown/test/test.js:15:  require('abstract-leveldown/abstract/put-get-del-test').all(leveljs, tape, testCommon, testBuffer);
sqldown/test/test.js:16-  require('abstract-leveldown/abstract/batch-test').all(leveljs, tape, testCommon)
level.js/test/test.js:7-
level.js/test/test.js:8:var testBuffer = new Buffer('foo')
level.js/test/test.js:9-
--
level.js/test/test.js:15-require('abstract-leveldown/abstract/get-test').all(leveljs, tape, testCommon)
level.js/test/test.js:16:require('abstract-leveldown/abstract/put-get-del-test').all(leveljs, tape, testCommon, testBuffer)
level.js/test/test.js:17-require('abstract-leveldown/abstract/batch-test').all(leveljs, tape, testCommon)
memdown/test.js:4-// var AbstractIterator = require('./').AbstractIterator
memdown/test.js:5:var testBuffer = require('./testdata_b64')
memdown/test.js:6-var ltgt = require('ltgt')
--
memdown/test.js:22-
memdown/test.js:23:require('abstract-leveldown/abstract/put-get-del-test').all(MemDOWN, test, testCommon, testBuffer)
memdown/test.js:24-
leveldown-hyper/test/put-get-del-test.js:5-    , path       = require('path')
leveldown-hyper/test/put-get-del-test.js:6:    , testBuffer = fs.readFileSync(path.join(__dirname, 'data/testdata.bin'))
leveldown-hyper/test/put-get-del-test.js:7-    , abstract   = require('abstract-leveldown/abstract/put-get-del-test')
leveldown-hyper/test/put-get-del-test.js:8-
leveldown-hyper/test/put-get-del-test.js:9:abstract.all(leveldown, test, testCommon, testBuffer)
leveldown-hyper/test/put-get-del-test.js:10-
redisdown/test/test.js:5-
redisdown/test/test.js:6:var testBuffer = new Buffer('foo');
redisdown/test/test.js:7-
--
redisdown/test/test.js:17-require('abstract-leveldown/abstract/get-test').all(leveljs, tape, testCommon);
redisdown/test/test.js:18:require('abstract-leveldown/abstract/put-get-del-test').all(leveljs, tape, testCommon, testBuffer);
redisdown/test/test.js:19-require('abstract-leveldown/abstract/batch-test').all(leveljs, tape, testCommon);
medeadown/test.js:3-  , medeaDOWN  = require('./medeadown')
medeadown/test.js:4:  , testBuffer = require('fs').readFileSync(__filename)
medeadown/test.js:5-  , db
--
medeadown/test.js:18-
medeadown/test.js:19:require('abstract-leveldown/abstract/put-get-del-test').all(medeaDOWN, test, testCommon, testBuffer)
medeadown/test.js:20-
mysqldown/test.js:4-  , MysqlDOWN    = require('./mysqldown')
mysqldown/test.js:5:  , testBuffer = require('fs').readFileSync(require('path').join(__dirname, 'testdata.bin'))
mysqldown/test.js:6-
--
mysqldown/test.js:23-
mysqldown/test.js:24:require('abstract-leveldown/abstract/put-get-del-test').all(factory, test, testCommon, testBuffer)
mysqldown/test.js:25-
fruitdown/tests/test.js:9-var testCommon = require('./testCommon');
fruitdown/tests/test.js:10:var testBuffer = new Buffer('hello');
fruitdown/tests/test.js:11-
--
fruitdown/tests/test.js:17-require('abstract-leveldown/abstract/put-get-del-test').all(
fruitdown/tests/test.js:18:  lib, tape, testCommon, testBuffer);
fruitdown/tests/test.js:19-require('abstract-leveldown/abstract/close-test').close(lib, tape, testCommon);
localstorage-down/tests/test.js:7-var testCommon = require('./testCommon');
localstorage-down/tests/test.js:8:var testBuffer = new Buffer('hello');
localstorage-down/tests/test.js:9-
--
localstorage-down/tests/test.js:15-require('abstract-leveldown/abstract/put-get-del-test').all(
localstorage-down/tests/test.js:16:  localstorage, tape, testCommon, testBuffer);
localstorage-down/tests/test.js:17-require('abstract-leveldown/abstract/close-test').close(localstorage, tape, testCommon);

[ralphtheninja]

@vweevers @juliangruber Any thoughts on this?

[vweevers]

Haven't had the time yet, maybe tomorrow.

[vweevers]

Awesome research and summary @ralphtheninja!

No one seems to be doing any funky stuff so we should be good by removing testBuffer completely.

👍

However, if we want we could add a test which reads a file (e.g. the PNG or whatever) to cover a larger buffer.

As long as stores can skip that test, for example because they have limited storage space.

[ralphtheninja]

Awesome research and summary @ralphtheninja!

Thanks! This is why I implemented depgrep btw :)

[vweevers]

I saw and I starred! 😄