Skip to content

Commit 4c75ff7

Browse files
madhurigmadhurig
committed
PR feedback, add tests
1 parent 22e9d33 commit 4c75ff7

File tree

2 files changed

+267
-13
lines changed

2 files changed

+267
-13
lines changed

‎node/test/toolrunnertests.ts‎

Lines changed: 264 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -394,7 +394,7 @@ describe('Toolrunner Tests', function () {
394394
done(err);
395395
});
396396
})
397-
it('Exec pipe output to another tool, succeeds if both tools succeed', function(done) {
397+
it('Exec pipe output to another tool, succeeds if both tools succeed', function (done) {
398398
this.timeout(20000);
399399

400400
var _testExecOptions = <trm.IExecOptions>{
@@ -457,7 +457,7 @@ describe('Toolrunner Tests', function () {
457457
});
458458
}
459459
})
460-
it('Exec pipe output to another tool, fails if first tool fails', function(done) {
460+
it('Exec pipe output to another tool, fails if first tool fails', function (done) {
461461
this.timeout(20000);
462462

463463
var _testExecOptions = <trm.IExecOptions>{
@@ -497,7 +497,7 @@ describe('Toolrunner Tests', function () {
497497
done(err);
498498
}
499499
else {
500-
assert(err && err.message && err.message.indexOf('print-output.exe') >=0, 'error from print-output.exe is not reported');
500+
assert(err && err.message && err.message.indexOf('print-output.exe') >= 0, 'error from print-output.exe is not reported');
501501
done();
502502
}
503503
})
@@ -530,7 +530,7 @@ describe('Toolrunner Tests', function () {
530530
}
531531
else {
532532
//assert(output && output.length > 0 && output.indexOf('ps: illegal option') >= 0, `error output "ps: illegal option" is expected. actual "${output}"`);
533-
assert(err && err.message && err.message.indexOf('/bin/ps') >=0, 'error from ps is not reported');
533+
assert(err && err.message && err.message.indexOf('/bin/ps') >= 0, 'error from ps is not reported');
534534
done();
535535
}
536536
})
@@ -539,7 +539,7 @@ describe('Toolrunner Tests', function () {
539539
})
540540
}
541541
})
542-
it('Exec pipe output to another tool, fails if second tool fails', function(done) {
542+
it('Exec pipe output to another tool, fails if second tool fails', function (done) {
543543
this.timeout(20000);
544544

545545
var _testExecOptions = <trm.IExecOptions>{
@@ -586,7 +586,7 @@ describe('Toolrunner Tests', function () {
586586
}
587587
else {
588588
assert(errOut && errOut.length > 0 && errOut.indexOf('some error message') >= 0, 'error output from match-input.exe is expected');
589-
assert(err && err.message && err.message.indexOf('match-input.exe') >=0, 'error from find does not match expeced. actual: ' + err.message);
589+
assert(err && err.message && err.message.indexOf('match-input.exe') >= 0, 'error from find does not match expeced. actual: ' + err.message);
590590
done();
591591
}
592592
})
@@ -634,6 +634,263 @@ describe('Toolrunner Tests', function () {
634634
});
635635
}
636636
})
637+
it('Exec pipe output to file and another tool, succeeds if both tools succeed', function (done) {
638+
this.timeout(20000);
639+
640+
var _testExecOptions = <trm.IExecOptions>{
641+
cwd: __dirname,
642+
env: {},
643+
silent: false,
644+
failOnStdErr: false,
645+
ignoreReturnCode: false,
646+
outStream: testutil.getNullStream(),
647+
errStream: testutil.getNullStream()
648+
};
649+
650+
const testFile = path.join(testutil.getTestTemp(), 'BothToolsSucceed.log');
651+
652+
if (os.platform() === 'win32') {
653+
var matchExe = tl.tool(compileMatchExe())
654+
.arg('0') // exit code
655+
.arg('line 2'); // match value
656+
var outputExe = tl.tool(compileOutputExe())
657+
.arg('0') // exit code
658+
.arg('line 1')
659+
.arg('line 2')
660+
.arg('line 3');
661+
outputExe.pipeExecOutputToTool(matchExe, testFile);
662+
663+
var output = '';
664+
outputExe.on('stdout', (data) => {
665+
output += data.toString();
666+
});
667+
668+
outputExe.exec(_testExecOptions)
669+
.then(function (code) {
670+
assert.equal(code, 0, 'return code of exec should be 0');
671+
assert(output && output.length > 0 && output.indexOf('line 2') >= 0, 'should have emitted stdout ' + output);
672+
assert(fs.existsSync(testFile), 'Log of first tool output is created when both tools succeed');
673+
assert(fs.readFileSync(testFile).indexOf('line 2') >= 0, 'Log file of first toool should have stdout from first tool')
674+
done();
675+
})
676+
.fail(function (err) {
677+
done(err);
678+
});
679+
}
680+
else {
681+
var grep = tl.tool(tl.which('grep', true));
682+
grep.arg('node');
683+
684+
var ps = tl.tool(tl.which('ps', true));
685+
ps.arg('ax');
686+
ps.pipeExecOutputToTool(grep, testFile);
687+
688+
var output = '';
689+
ps.on('stdout', (data) => {
690+
output += data.toString();
691+
});
692+
693+
ps.exec(_testExecOptions)
694+
.then(function (code) {
695+
assert.equal(code, 0, 'return code of exec should be 0');
696+
assert(output && output.length > 0 && output.indexOf('node') >= 0, 'should have emitted stdout ' + output);
697+
assert(fs.existsSync(testFile), 'Log of first tool output is created when both tools succeed');
698+
assert(fs.readFileSync(testFile).indexOf('PID') >= 0, 'Log of first tool should have stdout from first tool');
699+
done();
700+
})
701+
.fail(function (err) {
702+
done(err);
703+
});
704+
}
705+
})
706+
it('Exec pipe output to another tool, fails if first tool fails', function (done) {
707+
this.timeout(20000);
708+
709+
var _testExecOptions = <trm.IExecOptions>{
710+
cwd: __dirname,
711+
env: {},
712+
silent: false,
713+
failOnStdErr: false,
714+
ignoreReturnCode: false,
715+
outStream: testutil.getNullStream(),
716+
errStream: testutil.getNullStream()
717+
};
718+
719+
const testFile = path.join(testutil.getTestTemp(), 'FirstToolFails.log');
720+
721+
if (os.platform() === 'win32') {
722+
var matchExe = tl.tool(compileMatchExe())
723+
.arg('0') // exit code
724+
.arg('line 2'); // match value
725+
var outputExe = tl.tool(compileOutputExe())
726+
.arg('1') // exit code
727+
.arg('line 1')
728+
.arg('line 2')
729+
.arg('line 3');
730+
outputExe.pipeExecOutputToTool(matchExe, testFile);
731+
732+
var output = '';
733+
outputExe.on('stdout', (data) => {
734+
output += data.toString();
735+
});
736+
737+
var succeeded = false;
738+
outputExe.exec(_testExecOptions)
739+
.then(function () {
740+
succeeded = true;
741+
assert.fail('print-output.exe | findstr "line 2" was a bad command and it did not fail');
742+
})
743+
.fail(function (err) {
744+
if (succeeded) {
745+
done(err);
746+
}
747+
else {
748+
assert(err && err.message && err.message.indexOf('print-output.exe') >= 0, 'error from print-output.exe is not reported');
749+
assert(fs.existsSync(testFile), 'Log of first tool output is created when first tool fails');
750+
assert(fs.readFileSync(testFile).indexOf('line 3') >= 0, 'Error from first tool should be written to log file');
751+
done();
752+
}
753+
})
754+
.fail(function (err) {
755+
done(err);
756+
});
757+
}
758+
else {
759+
var grep = tl.tool(tl.which('grep', true));
760+
grep.arg('ssh');
761+
762+
var ps = tl.tool(tl.which('ps', true));
763+
ps.arg('bad');
764+
ps.pipeExecOutputToTool(grep, testFile);
765+
766+
var output = '';
767+
ps.on('stdout', (data) => {
768+
output += data.toString();
769+
});
770+
771+
var succeeded = false;
772+
ps.exec(_testExecOptions)
773+
.then(function () {
774+
succeeded = true;
775+
assert.fail('ps bad | grep ssh was a bad command and it did not fail');
776+
})
777+
.fail(function (err) {
778+
if (succeeded) {
779+
done(err);
780+
}
781+
else {
782+
assert(err && err.message && err.message.indexOf('/bin/ps') >= 0, 'error from ps is not reported');
783+
assert(fs.existsSync(testFile), 'Log of first tool output is created when first tool fails');
784+
assert(fs.readFileSync(testFile).indexOf('illegal option') >= 0, 'error from first tool should be written to log file');
785+
done();
786+
}
787+
})
788+
.fail(function (err) {
789+
done(err);
790+
})
791+
}
792+
})
793+
it('Exec pipe output to another tool, fails if second tool fails', function (done) {
794+
this.timeout(20000);
795+
796+
var _testExecOptions = <trm.IExecOptions>{
797+
cwd: __dirname,
798+
env: {},
799+
silent: false,
800+
failOnStdErr: false,
801+
ignoreReturnCode: false,
802+
outStream: testutil.getNullStream(),
803+
errStream: testutil.getNullStream()
804+
};
805+
806+
const testFile = path.join(testutil.getTestTemp(), 'SecondToolFails.log');
807+
808+
if (os.platform() === 'win32') {
809+
var matchExe = tl.tool(compileMatchExe())
810+
.arg('1') // exit code
811+
.arg('line 2') // match value
812+
.arg('some error message'); // error
813+
var outputExe = tl.tool(compileOutputExe())
814+
.arg('0') // exit code
815+
.arg('line 1')
816+
.arg('line 2')
817+
.arg('line 3');
818+
outputExe.pipeExecOutputToTool(matchExe, testFile);
819+
820+
var output = '';
821+
outputExe.on('stdout', (data) => {
822+
output += data.toString();
823+
});
824+
825+
var errOut = '';
826+
outputExe.on('stderr', (data) => {
827+
errOut += data.toString();
828+
});
829+
830+
var succeeded = false;
831+
outputExe.exec(_testExecOptions)
832+
.then(function (code) {
833+
succeeded = true;
834+
assert.fail('print-output.exe 0 "line 1" "line 2" "line 3" | match-input.exe 1 "line 2" "some error message" was a bad command and it did not fail');
835+
})
836+
.fail(function (err) {
837+
if (succeeded) {
838+
done(err);
839+
}
840+
else {
841+
assert(errOut && errOut.length > 0 && errOut.indexOf('some error message') >= 0, 'error output from match-input.exe is expected');
842+
assert(err && err.message && err.message.indexOf('match-input.exe') >= 0, 'error from find does not match expeced. actual: ' + err.message);
843+
assert(fs.existsSync(testFile), 'Log of first tool output is created when second tool fails');
844+
assert(fs.readFileSync(testFile).indexOf('some error message') < 0, 'error from second tool should not be in the log for first tool');
845+
done();
846+
}
847+
})
848+
.fail(function (err) {
849+
done(err);
850+
});
851+
}
852+
else {
853+
var grep = tl.tool(tl.which('grep', true));
854+
grep.arg('--?');
855+
856+
var ps = tl.tool(tl.which('ps', true));
857+
ps.arg('ax');
858+
ps.pipeExecOutputToTool(grep, testFile);
859+
860+
var output = '';
861+
ps.on('stdout', (data) => {
862+
output += data.toString();
863+
});
864+
865+
var errOut = '';
866+
ps.on('stderr', (data) => {
867+
errOut += data.toString();
868+
})
869+
870+
var succeeded = false;
871+
ps.exec(_testExecOptions)
872+
.then(function (code) {
873+
succeeded = true;
874+
assert.fail('ps ax | grep --? was a bad command and it did not fail');
875+
})
876+
.fail(function (err) {
877+
if (succeeded) {
878+
done(err);
879+
}
880+
else {
881+
assert(errOut && errOut.length > 0 && errOut.indexOf('grep: unrecognized option') >= 0, 'error output from ps command is expected');
882+
// grep is /bin/grep on Linux and /usr/bin/grep on OSX
883+
assert(err && err.message && err.message.match(/\/[usr\/]?bin\/grep/), 'error from grep is not reported. actual: ' + err.message);
884+
assert(fs.existsSync(testFile), 'Log of first tool output is created when second tool fails');
885+
assert(fs.readFileSync(testFile).indexOf('unrecognized option') < 0, 'error from second tool should not be in the first tool log file');
886+
done();
887+
}
888+
})
889+
.fail(function (err) {
890+
done(err);
891+
});
892+
}
893+
})
637894
it('handles single args', function (done) {
638895
this.timeout(10000);
639896

@@ -653,7 +910,7 @@ describe('Toolrunner Tests', function () {
653910
assert.equal((node as any).args.length, 5, 'should have 5 args');
654911
assert.equal((node as any).args.toString(), 'one,two,three,four,five', 'should be one,two,three,four,five');
655912
done();
656-
})
913+
})
657914
it('handles padded spaces', function (done) {
658915
this.timeout(10000);
659916

‎node/toolrunner.ts‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -589,15 +589,12 @@ export class ToolRunner extends events.EventEmitter {
589589
this.pipeOutputToTool._getSpawnOptions(options));
590590

591591
let fileStream: fs.WriteStream = this.pipeOutputToFile ? fs.createWriteStream(this.pipeOutputToFile) : null;
592-
if (fileStream) {
593-
fileStream.write(this._getCommandString(options) + os.EOL);
594-
}
595-
592+
596593
//pipe stdout of first tool to stdin of second tool
597594
cpFirst.stdout.on('data', (data: Buffer) => {
598595
try {
599596
if (fileStream) {
600-
fileStream.write(data.toString());
597+
fileStream.write(data);
601598
}
602599
cp.stdin.write(data);
603600
} catch (err) {
@@ -607,7 +604,7 @@ export class ToolRunner extends events.EventEmitter {
607604
});
608605
cpFirst.stderr.on('data', (data: Buffer) => {
609606
if (fileStream) {
610-
fileStream.write(data.toString());
607+
fileStream.write(data);
611608
}
612609
successFirst = !options.failOnStdErr;
613610
if (!options.silent) {

0 commit comments

Comments
 (0)