Resolve #22: --unanimous will exit(0) and output nothing if all hosts agree.
#35
Conversation
…l hosts agree. Otherwise, exit(1) and show output as usual. Resolves bterlson#22
bin/eshost.js
Outdated
| @@ -45,6 +45,9 @@ const yargv = yargs | |||
| .describe('showSource', 'show input source') | |||
| .boolean('showSource') | |||
| .alias('showSource', 'i') | |||
| .describe('quorum', 'If all engines agree, exit(0) with no output, otherwise print and exit(1); implies --coalesce') | |||
| .boolean('quorum') | |||
| .alias('quorum', 'q') | |||
There was a problem hiding this comment.
I like this, but not sure about the naming. Maybe "unanimous"?
There was a problem hiding this comment.
Yes, good point. That's more accurate to the intent anyway. Short form -u is also available.
There was a problem hiding this comment.
Short form -u is also available.
Maybe it was meant to be!
dilijev
changed the title
--quorum will exit(0) and output nothing if all hosts agree.--unanimous will exit(0) and output nothing if all hosts agree.
There was a problem hiding this comment.
As I was writing tests for this branch, I discovered the issues that I posted here. Instead of burdening @dilijev with the workload, I opted to resolve the issues myself, along with adding the necessary tests. I've moved our combined commits into a new PR.
| @@ -34,4 +36,16 @@ module.exports = class Reporter { | |||
| ]); | |||
| } | |||
| } | |||
|
|
|||
| static addTo(hostName, result) { | |||
| table.push([ | |||
There was a problem hiding this comment.
table is undefined here. This can be fixed by changing the signature to match Reporter.coalesceInto(this.results, host, resultString, this.resultsMap, ", ");, ie. Reporter.addTo(this.results, host, resultString); and then addTo becomes:
static addTo(table, host, result) {
table.push([
host.name, result
]);
}| @@ -169,11 +178,10 @@ if (hosts.length === 0) { | |||
|
|
|||
| let reporterOptions = { | |||
| showSource: argv.showSource, | |||
| coalesce: argv.coalesce | |||
| coalesce: argv.coalesce, | |||
| showSource: argv.showSource, | |||
| @@ -72,6 +75,7 @@ const yargv = yargs | |||
| .example('eshost -h ch-*,node test.js') | |||
| .example('eshost -h ch-1.?.? test.js') | |||
| .example('eshost --tags latest test.js') | |||
| .example('eshost --unanimous test.js') | |||
| this.results.forEach(row => { | ||
| printHostResult(row[0], row[1]); | ||
| }) | ||
| if (this.options.unanimous && this.results.length == 1) { |
There was a problem hiding this comment.
I don't understand this logic, as the following results are unanimous, but would fail this check.
Given:
eshost -u -e "1+1"
this.results might look like:
[ [ 'd8', '2' ], [ 'js', '2' ], [ 'ch', '2' ] ]
But that would fail this check and thus fail
If all engines under test produce identical output (logic already exists to test this in the collate option), print no output and set exit code to 0.
|
Combined here: #37 |
--unanimous tests (contains all commits from, and supersedes, #35)
Otherwise, exit(1) and show output as usual.
Resolves #22