From 6530ce27fe42a6da22d2f308396f7e530c74a419 Mon Sep 17 00:00:00 2001 From: Gurjeet Singh Date: Sun, 15 Jun 2014 17:33:23 -0400 Subject: [PATCH 1/3] Miscellaneous improvements in unit tests. End statements with semicolons, to be consistent with the surrounding code. Added a new unit test to ensure environment variables are honored when parsing a connection string. Added a TODO to cleanup a test that emits messages using console.log(). Correct a query's syntax. Looks like a good thing to do even though the syntax doesn't matter in mocked out tests. Removed a test that tests for SELECT tags; AFAIK, SELECT commands don't emit a tag. --- test/unit/client/cleartext-password-tests.js | 5 + test/unit/client/configuration-tests.js | 99 +++++++++++++++----- test/unit/client/connection-string-tests.js | 18 ++-- test/unit/client/early-disconnect-tests.js | 5 +- test/unit/client/md5-password-tests.js | 5 +- test/unit/client/notification-tests.js | 1 + test/unit/client/prepared-statement-tests.js | 2 +- test/unit/client/result-metadata-tests.js | 1 - 8 files changed, 102 insertions(+), 34 deletions(-) diff --git a/test/unit/client/cleartext-password-tests.js b/test/unit/client/cleartext-password-tests.js index 13a9c9bd..e880908b 100644 --- a/test/unit/client/cleartext-password-tests.js +++ b/test/unit/client/cleartext-password-tests.js @@ -1,5 +1,10 @@ require(__dirname+'/test-helper'); +/* + * TODO: Add _some_ comments to explain what it is we're testing, and how the + * code-being-tested works behind the scenes. + */ + test('cleartext password authentication', function(){ var client = createClient(); diff --git a/test/unit/client/configuration-tests.js b/test/unit/client/configuration-tests.js index 4e3724ca..d42ee0ee 100644 --- a/test/unit/client/configuration-tests.js +++ b/test/unit/client/configuration-tests.js @@ -36,33 +36,88 @@ test('initializing from a config string', function() { test('uses the correct values from the config string', function() { var client = new Client("postgres://brian:pass@host1:333/databasename") - assert.equal(client.user, 'brian') - assert.equal(client.password, "pass") - assert.equal(client.host, "host1") - assert.equal(client.port, 333) - assert.equal(client.database, "databasename") - }) + assert.equal(client.user, 'brian'); + assert.equal(client.password, "pass"); + assert.equal(client.host, "host1"); + assert.equal(client.port, 333); + assert.equal(client.database, "databasename"); + }); test('uses the correct values from the config string with space in password', function() { var client = new Client("postgres://brian:pass word@host1:333/databasename") - assert.equal(client.user, 'brian') - assert.equal(client.password, "pass word") - assert.equal(client.host, "host1") - assert.equal(client.port, 333) - assert.equal(client.database, "databasename") - }) + assert.equal(client.user, 'brian'); + assert.equal(client.password, "pass word"); + assert.equal(client.host, "host1"); + assert.equal(client.port, 333); + assert.equal(client.database, "databasename"); + }); test('when not including all values the defaults are used', function() { - var client = new Client("postgres://host1") - assert.equal(client.user, process.env['PGUSER'] || process.env.USER) - assert.equal(client.password, process.env['PGPASSWORD'] || null) - assert.equal(client.host, "host1") - assert.equal(client.port, process.env['PGPORT'] || 5432) - assert.equal(client.database, process.env['PGDATABASE'] || process.env.USER) - }) + var client = new Client("postgres://host1"); + assert.equal(client.user, process.env['PGUSER'] || process.env.USER); + assert.equal(client.password, process.env['PGPASSWORD'] || null); + assert.equal(client.host, "host1"); + assert.equal(client.port, process.env['PGPORT'] || 5432); + assert.equal(client.database, process.env['PGDATABASE'] || process.env.USER); + }); + test('when not including all values the environment variables are used', function() { + var envUserDefined = process.env['PGUSER'] !== undefined; + var envPasswordDefined = process.env['PGPASSWORD'] !== undefined; + var envDBDefined = process.env['PGDATABASE'] !== undefined; + var envHostDefined = process.env['PGHOST'] !== undefined; + var envPortDefined = process.env['PGPORT'] !== undefined; -}) + var savedEnvUser = process.env['PGUSER']; + var savedEnvPassword = process.env['PGPASSWORD']; + var savedEnvDB = process.env['PGDATABASE']; + var savedEnvHost = process.env['PGHOST']; + var savedEnvPort = process.env['PGPORT']; + + process.env['PGUSER'] = 'utUser1'; + process.env['PGPASSWORD'] = 'utPass1'; + process.env['PGDATABASE'] = 'utDB1'; + process.env['PGHOST'] = 'utHost1'; + process.env['PGPORT'] = 5464; + + var client = new Client("postgres://host1"); + assert.equal(client.user, process.env['PGUSER']); + assert.equal(client.password, process.env['PGPASSWORD']); + assert.equal(client.host, "host1"); + assert.equal(client.port, process.env['PGPORT']); + assert.equal(client.database, process.env['PGDATABASE']); + + if (envUserDefined) { + process.env['PGUSER'] = savedEnvUser; + } else { + delete process.env['PGUSER']; + } + + if (envPasswordDefined) { + process.env['PGPASSWORD'] = savedEnvPassword; + } else { + delete process.env['PGPASSWORD']; + } + + if (envDBDefined) { + process.env['PGDATABASE'] = savedEnvDB; + } else { + delete process.env['PGDATABASE']; + } + + if (envHostDefined) { + process.env['PGHOST'] = savedEnvHost; + } else { + delete process.env['PGHOST']; + } + + if (envPortDefined) { + process.env['PGPORT'] = savedEnvPort; + } else { + delete process.env['PGPORT']; + } + }); +}); test('calls connect correctly on connection', function() { var client = new Client("/tmp"); @@ -74,6 +129,6 @@ test('calls connect correctly on connection', function() { }; client.connect(); assert.equal(usedPort, "/tmp/.s.PGSQL." + pgport); - assert.strictEqual(usedHost, undefined) -}) + assert.strictEqual(usedHost, undefined); +}); diff --git a/test/unit/client/connection-string-tests.js b/test/unit/client/connection-string-tests.js index 5c08a0fd..9316daa9 100644 --- a/test/unit/client/connection-string-tests.js +++ b/test/unit/client/connection-string-tests.js @@ -1,21 +1,27 @@ require(__dirname + '/test-helper'); +/* + * Perhaps duplicate of test named 'initializing from a config string' in + * configuration-tests.js + */ + test("using connection string in client constructor", function() { var client = new Client("postgres://brian:pw@boom:381/lala"); + test("parses user", function() { assert.equal(client.user,'brian'); - }) + }); test("parses password", function() { assert.equal(client.password, 'pw'); - }) + }); test("parses host", function() { assert.equal(client.host, 'boom'); - }) + }); test('parses port', function() { assert.equal(client.port, 381) - }) + }); test('parses database', function() { assert.equal(client.database, 'lala') - }) -}) + }); +}); diff --git a/test/unit/client/early-disconnect-tests.js b/test/unit/client/early-disconnect-tests.js index edf57483..c67a783a 100644 --- a/test/unit/client/early-disconnect-tests.js +++ b/test/unit/client/early-disconnect-tests.js @@ -1,7 +1,8 @@ var helper = require(__dirname + '/test-helper'); var net = require('net'); var pg = require('../../..//lib/index.js'); - + +/* console.log() messages show up in `make test` output. TODO: fix it. */ var server = net.createServer(function(c) { console.log('server connected'); c.destroy(); @@ -18,5 +19,5 @@ server.listen(7777, function() { else console.log('client connected'); assert(err); })); - + }); diff --git a/test/unit/client/md5-password-tests.js b/test/unit/client/md5-password-tests.js index 2cd929bb..bd5ca46f 100644 --- a/test/unit/client/md5-password-tests.js +++ b/test/unit/client/md5-password-tests.js @@ -1,4 +1,5 @@ -require(__dirname + '/test-helper') +require(__dirname + '/test-helper'); + test('md5 authentication', function() { var client = createClient(); client.password = "!"; @@ -13,7 +14,7 @@ test('md5 authentication', function() { var password = "md5" + encrypted //how do we want to test this? assert.equalBuffers(client.connection.stream.packets[0], new BufferList() - .addCString(password).join(true,'p')) + .addCString(password).join(true,'p')); }); }); diff --git a/test/unit/client/notification-tests.js b/test/unit/client/notification-tests.js index 24d74604..e6b0dff1 100644 --- a/test/unit/client/notification-tests.js +++ b/test/unit/client/notification-tests.js @@ -1,4 +1,5 @@ var helper = require(__dirname + "/test-helper"); + test('passes connection notification', function() { var client = helper.client(); assert.emits(client, 'notice', function(msg) { diff --git a/test/unit/client/prepared-statement-tests.js b/test/unit/client/prepared-statement-tests.js index 15bb7441..c460b991 100644 --- a/test/unit/client/prepared-statement-tests.js +++ b/test/unit/client/prepared-statement-tests.js @@ -50,7 +50,7 @@ test('bound command', function() { assert.ok(client.connection.emit('readyForQuery')); var query = client.query({ - text: 'select * where name = $1', + text: 'select * form X where name = $1', values: ['hi'] }); diff --git a/test/unit/client/result-metadata-tests.js b/test/unit/client/result-metadata-tests.js index 435af25b..8fe892ed 100644 --- a/test/unit/client/result-metadata-tests.js +++ b/test/unit/client/result-metadata-tests.js @@ -36,4 +36,3 @@ testForTag("INSERT 0 3", check(0, 3, "INSERT")); testForTag("INSERT 841 1", check(841, 1, "INSERT")); testForTag("DELETE 10", check(null, 10, "DELETE")); testForTag("UPDATE 11", check(null, 11, "UPDATE")); -testForTag("SELECT 20", check(null, 20, "SELECT")); From 23b29c9846c2f6301474e897c5158372bc1630e8 Mon Sep 17 00:00:00 2001 From: Gurjeet Singh Date: Tue, 17 Jun 2014 09:15:14 -0400 Subject: [PATCH 2/3] Update a test to pass. --- test/unit/client/prepared-statement-tests.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/unit/client/prepared-statement-tests.js b/test/unit/client/prepared-statement-tests.js index c460b991..6e26c796 100644 --- a/test/unit/client/prepared-statement-tests.js +++ b/test/unit/client/prepared-statement-tests.js @@ -50,14 +50,14 @@ test('bound command', function() { assert.ok(client.connection.emit('readyForQuery')); var query = client.query({ - text: 'select * form X where name = $1', + text: 'select * from X where name = $1', values: ['hi'] }); assert.emits(query,'end', function() { test('parse argument', function() { assert.equal(parseArg.name, null); - assert.equal(parseArg.text, 'select * where name = $1'); + assert.equal(parseArg.text, 'select * from X where name = $1'); assert.equal(parseArg.types, null); }); From ef40b6f3e9dd7847b34523da4e823474ebc4e601 Mon Sep 17 00:00:00 2001 From: Gurjeet Singh Date: Sun, 22 Jun 2014 08:19:26 -0400 Subject: [PATCH 3/3] Re-add the test for SELECT tag; I was wrong in my assumption. Postgres generally does not emit a SELECT tag after a SELECT query, but it does emit that tag after a CREATE TABLE x AS SELECT query. Example: postgres=# create table t as select 1; SELECT 1 --- test/unit/client/result-metadata-tests.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/unit/client/result-metadata-tests.js b/test/unit/client/result-metadata-tests.js index 8fe892ed..435af25b 100644 --- a/test/unit/client/result-metadata-tests.js +++ b/test/unit/client/result-metadata-tests.js @@ -36,3 +36,4 @@ testForTag("INSERT 0 3", check(0, 3, "INSERT")); testForTag("INSERT 841 1", check(841, 1, "INSERT")); testForTag("DELETE 10", check(null, 10, "DELETE")); testForTag("UPDATE 11", check(null, 11, "UPDATE")); +testForTag("SELECT 20", check(null, 20, "SELECT"));