Skip to content

Commit

Permalink
test: check for openssl cli and provide path if it exists
Browse files Browse the repository at this point in the history
the previous version checked if io.js was compiled with openssl support which
isn't really relevant since we're starting a http server. we on the other hand
need an openssl-cli which may or may not exist.

PR-URL: #1049
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
  • Loading branch information
jbergstroem authored and Shigeki Ohtsu committed Mar 5, 2015
1 parent 71776f9 commit c7ad320
Showing 1 changed file with 5 additions and 5 deletions.
10 changes: 5 additions & 5 deletions test/parallel/test-http-curl-chunk-problem.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
if (!process.versions.openssl) {
console.error('Skipping because node compiled without OpenSSL.');
var common = require('../common');
var assert = require('assert');
if (!common.opensslCli) {
console.error('Skipping because node compiled without OpenSSL CLI.');
process.exit(0);
}

// http://groups.google.com/group/nodejs/browse_thread/thread/f66cd3c960406919
var common = require('../common');
var assert = require('assert');
var http = require('http');
var cp = require('child_process');
var fs = require('fs');
Expand All @@ -16,7 +16,7 @@ var count = 0;
function maybeMakeRequest() {
if (++count < 2) return;
console.log('making curl request');
var cmd = 'curl http://127.0.0.1:' + common.PORT + '/ | openssl sha1';
var cmd = 'curl http://127.0.0.1:' + common.PORT + '/ | ' + common.opensslCli + ' sha1';

This comment has been minimized.

Copy link
@piscisaureus

piscisaureus Mar 5, 2015

Contributor

@jbergstroem @shigeki This won't work if there are spaces in the path to openssl-cli (which often happens on windows).

This comment has been minimized.

Copy link
@shigeki

shigeki Mar 5, 2015

Contributor

@piscisaureus Indeed. Also this test was failed on my Win because curl is not installed. I think it is better for us to separate two spawnSync. Thoughts?

This comment has been minimized.

Copy link
@piscisaureus

piscisaureus Mar 5, 2015

Contributor

We're creating a pipe, which is not easy to do with spawnSync. execSync might work though.
However the easiest fix would be to put quotation marks (") around the openssl path, at least on windows.

This comment has been minimized.

Copy link
@jbergstroem

jbergstroem Mar 5, 2015

Author Member

Ouch. Sorry - no access to Windows. I'll improve as I get access to the Windows buildbots through iojs/build. If someone else could test/verify I'd appreciate it greatly.

This comment has been minimized.

Copy link
@shigeki

shigeki Mar 5, 2015

Contributor

@jbergstroem Don't worry, I will fix this after installing curl. Just moment.

This comment has been minimized.

Copy link
@jbergstroem

jbergstroem Mar 5, 2015

Author Member

Thanks!

cp.exec(cmd, function(err, stdout, stderr) {
if (err) throw err;
var hex = stdout.match(/([A-Fa-f0-9]{40})/)[0];
Expand Down

0 comments on commit c7ad320

Please sign in to comment.