tests failing on Node 11 on travis

[mcollina]

The tests are failing on Node 11 on travis.

I cannot understand why, so I'm disabling them.

It would be fantastic if someone could pick this up.

[MichielDeMey]

Does the issue on occur on Travis?
It seems to work fine when running the test-suite locally using Node 11.

[mcollina]

Unfortunately they were failing on Travis. Would you like to send a PR to enable Node 11 and check if they pass?

[MichielDeMey]

Will do 👍

[MichielDeMey]

Debugging this further, it only seems to fail when native is set to true.
(cfr. https://github.com/fastify/fastify-postgres/blob/master/test.js#L102)

Setting this to false passes the tests. This leads me to believe it's an issue with the native module, but the logs show that the compilation passes.

[MichielDeMey]

Looks like the issue is related to the actual node-pg-native library.
Running the testsuite on Node 11 on Travis yields a Segmentation fault.

See: https://travis-ci.org/brianc/node-pg-native/jobs/492244123

[mcollina]

Is there an issue open for this on node-pg-native?

Can we:

a. document this fact in the README
b. disable the native tests in Node 11

[MichielDeMey]

I've logged an issue in the repo, tracking here: brianc/node-pg-native#78.
For now we'll have to document and disable Node 11 support until the issue has been fixed.

Although it might work on a local machine and this issue could be Travis specific, there's currently no way to guarantee compatibility due to failing tests.

[darkgl0w]

Hello,

Here is a little update on this issue:
So far I have narrowed down this issue to the node-libpq package that doesn't support Node.js 11 and 12.

I opened this PR brianc/node-libpq#67 to address the necessary changes in order to make node-libpq compile successfully on Node 11 and 12 (I received invaluable help to get there with something clean seen that my cpp has proven to be beyond rusty 🤣 thanks again @Flarna ^^).

Now I am proceeding step by step to identify the issue causing the segfault during test in the CI Node 12 environment (this is where I got for now: https://travis-ci.org/brianc/node-libpq/jobs/531071467).

So far by carefully updating the dev dependencies I made some progress but it is a tedious task as I must dig in packages changelogs to avoid bumping a dep that will break support on previous Node versions (node-libpq supports up to Node v4 😿 ).

I will keep this updated as I make some progress if someone wants to give it a go.

NB: Currently the package compile successfully and pass all tests on my Node 12 dev environment (copy pasted the results here: https://pastebin.com/HWWcQD2v).

[mcollina]

Awesome work! Keep us posted!

[jsumners]

Why do we even depend on pg-native?

[mcollina]

To test everything is working fine with it I guess:

fastify-postgres/index.js

Lines 60 to 67 in 2313d6e

	if (options.native) {
	delete options.native
	if (!pg.native) {
	console.warn('pg-native not installed, can\'t use native option - fallback to pg module')
	} else {
	pg = pg.native
	}
	}

.

[darkgl0w]

Hello,

Some news on this issue:

So far it was a conflict between Node.js >= 10.16.0 shipping OpenSSL 1.1.1b and environments shipping with OpenSSL 1.0.x (Travis aging environment is one of those environments that ships with 1.0.x version OpenSSL).

This is a related issue: #49

With some contributors we investigated the issue and we were finally able to issue a fully working PR on node-libpq repo and now we just need to wait for a fast review and hopefully a merge.

[mcollina]

I've been following your work, it's fantastic. Thank you for picking this up @darkgl0w!

[mcollina]

(We could potentially switch this to Azure pipelines, if this would simplify porting it to Node 12)

[darkgl0w]

I have little to no knowledge on Azure Pipelines :/
but from what I saw from googling it, defining container based jobs should allow us to simplify the testing process for fastify-postgres (IMO we will need at least a container matrix strategy running ubuntu:18.04 and updated with OpenSSL 1.1.1 and libpq linked with it).

The solution we came up with to bypass Travis env limitations on node-libpq repo PR is kind of heavy (on Node.js 10 and 12 the CI takes around 3 minutes and a half to complete a job) and if we stay on Travis here we will need to setup the CI in a same way to satisfy OpenSSL dependencies in order to complete the testing process when the PR will land in a new release of node-libpq.

[mcollina]

If I enable Azure pipelines, would you like to tinker with a config?

[darkgl0w]

I have made some configuration tests with environments based on ubuntu:19.04 containers inside a forked repo with Azure pipelines activated and that feels even more hacky and heavy than the Travis solution 😆

The only thing I didn't test currently is building a custom docker image out of ubuntu:19.04 or debian:buster with everything that's needed to run the tests inside, I will try to work on this this week-end if I have the time but even if it's a success, building our own docker image implies another maintenance duty. 😞

For now I setup a branch with a fully working Travis conf and if that's ok with you I can send a PR in order to solve this kind of issues #49 (you can see the CI results and running process here) and when the node-libpq will land we will already be prepared.

[mcollina]

I'm ok with the custom Travis config. Thanks!