Merge pull request #608 from gurjeet/misc_unit_test_improvements

Miscellaneous improvements in unit tests.

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();

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);
+});
 

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')
-  })
-})
+  });
+});
 

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);
   }));
-   
+
 });

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'));
     });
   });
 

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) {

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 * 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);
       });