Add hapi support

[nodejs]

Add hapi support

[hueniverse]

This seems like the wrong attitude coming from people responsible for maintaining the health of the ecosystem as a whole (not to mention the fact that no one bothered to actually connect with me to address any concerns). Including hapi in the node smoke tests adds more value to node than to hapi. We test hapi extensively and we find all the issues when there are new versions of node. By excluding one of the most extensive and deepest test suite here, you are doing the node community as a whole a disservice. For many years, hapi was the only framework to surface bugs in new versions of node.

I am always open to improving the test suite for hapi and to collaborate with the node core team. Every single one of you knows this very well which is why this thread is so annoying.

[AdriVanHoudt]

@cjihrig brought this back up and I think it would be valuable to add hapi to citgm.
I think it meets all the requirements.

[DavidTPate]

@gibfahn I'm willing to volunteer for those notifications as well. I know that Eran is a very busy person these days.

[AdriVanHoudt]

@gibfahn you can add me no problem

[jasnell]

If hapi supports npm test out of the box on what is installed from npm install, then no, nothing else is required. If, it doesn't however, you'll want to add an entry to https://github.com/nodejs/citgm/blob/master/lib/lookup.json and make sure to specify the -l switch when running the hapi test

[cjihrig]

All you need is npm install and npm test.

[MylesBorins]

I am unsure if this is a regression but the current version of hapi has a single failing test. Will investigate.

[MylesBorins]

So there is a single failed test with hapi when running within citgm

Not sure what is causing this weirdness, but it is worth exploring. I ran the test suite for v11.1.2 in both citgm and bash, and all tests passed in bash.

verbose: npm-test:           | Failed tests:
verbose:                     |
verbose:                     | 70) Connection creates a server listening on a
verbose:                     | unix domain socket:
verbose:                     |
verbose:                     | listen EADDRINUSE
verbose:                     | /private/var/folders/ty/q7q6b07j5r3c7nvnpzm6hkq40…
verbose:                     | 000gn/T/422862c8-9a51-424c-b377-f22ecac239c0/hapi…
verbose:                     | /test/hapi-server.socket
verbose:                     |
verbose:                     | at Object.exports._errnoException (util.js:855:11

[rvagg]

tbh I've had a lot of trouble with Hapi's test suite, it fails inconsistently across all versions of Node.js and Hapi. I've been (a) taking failures with a grain of salt and/or (b) comparing a smoke test run with Hapi to a previous release of Node.js to see if the same failure occurs there to ensure the failure is not new.

[MylesBorins]

Closing for now... can revisit if hapi wants support

[gibfahn]

I think it would be valuable to add hapi, but I've been unable to get the tests to pass reliably on my machine.

wget https://github.com/hapijs/hapi/archive/v16.3.0.tar.gz
tar -xf v16.3.0.tar.gz
npm it

Failed tests:

  718) transmission marshal() closes file handlers when not reading file stream:

      actual expected

      200304

      Expected 200 to equal specified value

      at server.inject (/Users/gib/tmp/hapi/hapi-16.3.0/test/transmit.js:96:48

  772) transmission transmit() does not open file stream on 304:

      Timed out (3000ms) - transmission transmit() does not open file stream on 304


  780) transmission transmit() does not leak stream data when request aborts before stream is returned:

      Timed out (3000ms) - transmission transmit() does not leak stream data when request aborts before stream is returned



3 of 897 tests failed
Test duration: 14288 ms
Assertions count: 2238 (verbosity: 2.49)
No global variable leaks detected
Coverage: 99.98% (1/4225)
lib/response.js missing coverage on line(s): 633
Code coverage below threshold: 99.98 < 100
Linting results: No issues

[AdriVanHoudt]

I could reproduce, I opened an issue hapijs/hapi#3508

[AdriVanHoudt]

It seems a PR with a fix is there hapijs/hapi#3507
@gibfahn could you try that branch?

[MylesBorins]

@hueniverse to be honest this thread was closed before CITGM really had hit a stride and was primarily an experiment. The fact that hapi has fallen to the wayside is definitely an oversight

I'm reopening this so we can keep track of the status. This is definitely a priority

[gibfahn]

to be honest this thread was closed before CITGM really had hit a stride and was primarily an experiment.

Yeah this was from 2015, didn't even know CitGM was around then!

I am always open to improving the test suite for hapi and to collaborate with the node core team.

Great, we'll get it added to CitGM. As long as if there's a failure we can work with you to get it fixed (assuming it's not a Node core issue) then it should be smooth sailing.

[gibfahn]

PR: #436

[DavidTPate]

I'll keep a lookout for issues related to CiTGM with Hapi as well.

[gibfahn]

Thanks @DavidTPate.

So CitGM has a maintainers field, which is a list of people we ping if there's an issue with a module and we need some expert help.

I've included @hueniverse in #436, if anyone else is willing to be included in there let us know (maybe @AdriVanHoudt or @DavidTPate ?)