CLI Daemon tests

[richardschneider]

Three tests (Addresses.Swarm, SIGINT and SIGTERM) make the assumption that the daemon can be spawned and the process output can be read in real time. This is not true because node's childProcess buffers the output.

What really happens, is the command timeout is detected by execa and the process is then killed and then the test gets the process output, which then tries to the kill process. Also the 1st killing of the process raises a Promise rejection that is unhandled and Node tells us

(node:12988) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

The SIGINT and SIGTERM tests really belong to js-ipfsd-ctl, see issue #1192

Is the Addresses.Swarm test really needed? If so, shouldn't it be a core test?

[dryajov]

I believe the issues is actually this - https://stackoverflow.com/questions/23429499/stdout-buffer-issue-using-node-child-process, I've ran across it myself, the solution is to increase the buffer to something more reasonable - {maxBuffer: 1024 * 500}.

I'm not sure I agree with this -

The SIGINT and SIGTERM tests really belong to js-ipfsd-ctl, see issue #1192

We want to run this tests as part of each CI run so we make sure it's not broken.

Is the Addresses.Swarm test really needed?

This was due to the fact it was only an issue with the daemon IIRC. But if we can run them as part of core, I think we should.

[dryajov]

Here are the actually docs on maxBuffer - https://nodejs.org/api/child_process.html#child_process_child_process_exec_command_options_callback

[richardschneider]

@dryajov sorry it's not a buffer size issue. The issue is that nothing is received until the daemon process stops.

[dryajov]

@richardschneider we're using https://www.npmjs.com/package/execa to spawn the process, which uses spawn underneath, which returns stdout/stderr as streams, without waiting for the process to finish, the sameway child_process.spawn does.

[dryajov]

This is not true because node's childProcess buffers the output.

I think the confusion with this is because execa gets mapped to exec -

js-ipfs/test/utils/ipfs-exec.js

Line 31 in ac95601

const exec = (args) => execa(`${process.cwd()}/src/cli/bin.js`, args, config)

[daviddias]

I believe this is being handled at #1103, do you confirm?

[dryajov]

Three tests (Addresses.Swarm, SIGINT and SIGTERM) make the assumption that the daemon can be spawned and the process output can be read in real time. This is not true because node's childProcessbuffers the output.

From my side of things, I was looking specifically into this statement, which I didn't think was accurate - confirmed by my last two comments. As for the failures related to this, I'm not sure where they're being handled.

[daviddias]

All good now with latest ipfsd-ctl changes. Thank you both! ❤️