Cloning the library throws error when pg-native is not installed

[janmeier]

Related to sequelize/sequelize#5438 and sequelize/sequelize#5476.

var pg = require('pg')
_.clone(pg);

Throws Error: Cannot find module 'pg-native'

Wanting to clone the library directly doesn't make much sense, but in the issues linked above, it happens because a sequelize model, which has a reference to pg is being deeply cloned.

This should be fixable by switching to defineProperty and marking the native property as non-enumerable.

I can write up a PR later today if you'd like - I just need a hint as to where to put the test

[brianc]

Hey @janmeier - I would definitely appreciate a PR for this issue! Sounds like a pretty annoying bug and also not a super difficult fix!

As far as where to put the tests - if a test actually needs a postgres instance running I put it under test/integration otherwise under test/unit - either is fine really. And for making a test...node-postgres was written initially before any of the now defacto test frameworks existed so I "wrote my own" in a sense. Every single file under the test/ directory is recursively required & is expected to run to completion & cleanly exit as fast as possible. So you can just dump any old whatever.js file under the test directory & include a test using the standard var assert = require('assert') module from node. As long as the js file exits cleanly (0 exit code) the test is considered passing!

Does that make sense?

basically:

$: touch test/unit/clone-pg-object-tests.js
$: vim test/unit/clone-pg-object-tests.js
# ... some code editing ensues
$ node test/unit/clone-pg-object-tests.js  <--- that should exit with a 0 status 

If you want to run all the unit tests (including the new one you added) you can type make test-unit from the top level dir. If you want to run all the tests you can type make test-integration but you need a running instance of postgres.

Hope that helps!

[janmeier]

Thanks for the thorough contribution guide! ;)

[jibwa]

+1

[PatrickKelly]

+1

[TinOo512]

@janmeier any plan of opening a PR here with your fork ? I have the same issue and your fork seems to resolve it :)

[janmeier]

@TinOo512 #950 has been merged instead

[TinOo512]

@janmeier you are right, I thought the opposite because of the remaining console.log(err.message) which print Cannot find module 'pg-native' sometimes, ... Don't really know which module try to access the native driver for now, ...