From 4cb8968fc725cf8f29acdad4123ecfb70bb3640b Mon Sep 17 00:00:00 2001 From: Tim Branyen Date: Fri, 21 Nov 2014 19:56:39 -0500 Subject: [PATCH 1/7] Added in --expose-gc flag Also added in --build-only stub, will fill in later --- .travis.yml | 2 +- package.json | 2 +- test/tests/revwalk.js | 1 + 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index c7de3a62f..3c5d107ef 100644 --- a/.travis.yml +++ b/.travis.yml @@ -31,7 +31,7 @@ branches: os: - linux - osx -script: npm --expose-gc test +script: npm test notifications: slack: secure: KglNSqZiid9YudCwkPFDh+sZfW5BwFlM70y67E4peHwwlbbV1sSBPHcs74ZHP/lqgEZ4hMv4N2NI58oYFD5/1a+tKIQP1TkdIMuq4j2LXheuirA2HDcydOVrsC8kRx5XFGKdVRg/uyX2dlRHcOWFhxrS6yc6IxtxYWlRTD2SmEc= diff --git a/package.json b/package.json index 34abef0d8..e18991605 100644 --- a/package.json +++ b/package.json @@ -89,7 +89,7 @@ }, "scripts": { "lint": "jshint lib test/tests", - "cov": "istanbul cover node_modules/mocha/bin/_mocha -- test/runner test/tests --report=lcov", + "cov": "node --expose-gc ./node_modules/istanbul/lib/cli.js cover _mocha -- test/runner test/tests --report=lcov", "mocha": "mocha test/runner test/tests", "test": "npm run lint && npm run cov", "missing-tests": "node generate/missing-tests", diff --git a/test/tests/revwalk.js b/test/tests/revwalk.js index 21e10a172..1b7ca79c3 100644 --- a/test/tests/revwalk.js +++ b/test/tests/revwalk.js @@ -87,6 +87,7 @@ describe("Revwalk", function() { testGC("doesnt segfault when accessing .author() twice", function(done) { this.timeout(10000); + return Repository.open(reposPath).then(function(repository) { var walker = repository.createRevWalk(); return repository.getMasterCommit().then(function(firstCommitOnMaster) { From 83407702be9d97834e1d4c3f4e9814716bed4306 Mon Sep 17 00:00:00 2001 From: Tim Branyen Date: Fri, 21 Nov 2014 20:27:56 -0500 Subject: [PATCH 2/7] Crank up the timeout --- install.js | 8 ++++++++ test/runner.js | 2 +- test/tests/revwalk.js | 2 ++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/install.js b/install.js index 6bd410b16..33dd5d4fb 100644 --- a/install.js +++ b/install.js @@ -10,6 +10,14 @@ var which = require("which"); var rimraf = require("rimraf"); var NODE_VERSION = Number(process.version.match(/^v(\d+\.\d+)/)[1]); +// If the build only flag is set +var buildOnly = process.argv.indexOf("--build-only") > -1; + +if (buildOnly) { + console.log("buildOnly"); + return; +} + // This will take in an object and find any matching keys in the environment // to use as overrides. // diff --git a/test/runner.js b/test/runner.js index 273546e95..8b2d0052e 100644 --- a/test/runner.js +++ b/test/runner.js @@ -7,7 +7,7 @@ var exec = promisify(function(command, opts, callback) { }); before(function(done) { - this.timeout(150000); + this.timeout(350000); var url = "https://github.com/nodegit/test"; var done = done.bind(null, null); diff --git a/test/tests/revwalk.js b/test/tests/revwalk.js index 1b7ca79c3..737431727 100644 --- a/test/tests/revwalk.js +++ b/test/tests/revwalk.js @@ -106,6 +106,8 @@ describe("Revwalk", function() { } }); }); + }).then(function() { + global.gc(); }); }); From 551fd42a7fc193487f74e0ed14d7397add96cb36 Mon Sep 17 00:00:00 2001 From: Tim Branyen Date: Fri, 21 Nov 2014 23:24:43 -0500 Subject: [PATCH 3/7] Modified revwalk test to only call done once --- test/tests/revwalk.js | 38 ++++++++++++++++++-------------------- 1 file changed, 18 insertions(+), 20 deletions(-) diff --git a/test/tests/revwalk.js b/test/tests/revwalk.js index 737431727..c8d0305a4 100644 --- a/test/tests/revwalk.js +++ b/test/tests/revwalk.js @@ -8,6 +8,9 @@ describe("Revwalk", function() { var Revwalk = require("../../lib/revwalk"); var Oid = require("../../lib/oid"); + // Set a reasonable timeout here now that our repository has grown. + this.timeout(150000); + before(function(done) { var test = this; return Repository.open(reposPath).then(function(repository) { @@ -81,34 +84,29 @@ describe("Revwalk", function() { }); }); - // This test requires forcing garbage collection, so mocha needs to be run via - // node rather than npm, with a la `node --expose-gc [pathtohmoca] [testglob]` - var testGC = (global.gc ? it : it.skip); + // This test requires forcing garbage collection, so mocha needs to be run + // via node rather than npm, with a la `node --expose-gc [pathtohmoca] + // [testglob]` + var testGC = global.gc ? it : it.skip; testGC("doesnt segfault when accessing .author() twice", function(done) { - this.timeout(10000); - - return Repository.open(reposPath).then(function(repository) { + Repository.open(reposPath).then(function(repository) { var walker = repository.createRevWalk(); - return repository.getMasterCommit().then(function(firstCommitOnMaster) { - var did = false; - walker.walk(firstCommitOnMaster, function(error, commit) { + + repository.getMasterCommit().then(function(firstCommitOnMaster) { + walker.walk(firstCommitOnMaster, function(err, commit) { + if (!err && !commit) { + return done(); + } + for (var i = 0; i < 1000; i++) { - if (true) { - commit.author().name(); - commit.author().email(); - } + commit.author().name(); + commit.author().email(); + global.gc(); } - if (!did) { - done(); - did = true; - } }); }); - }).then(function() { - global.gc(); }); }); - }); From 0a5632c529ce1f3f0c6988db7e9d9337a1377982 Mon Sep 17 00:00:00 2001 From: Maximiliano Korp Date: Fri, 21 Nov 2014 22:59:08 -0700 Subject: [PATCH 4/7] stop thrashing cpu with GCs --- test/tests/revwalk.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/test/tests/revwalk.js b/test/tests/revwalk.js index c8d0305a4..9f42536d6 100644 --- a/test/tests/revwalk.js +++ b/test/tests/revwalk.js @@ -9,7 +9,7 @@ describe("Revwalk", function() { var Oid = require("../../lib/oid"); // Set a reasonable timeout here now that our repository has grown. - this.timeout(150000); + this.timeout(60000); before(function(done) { var test = this; @@ -99,11 +99,13 @@ describe("Revwalk", function() { return done(); } - for (var i = 0; i < 1000; i++) { + for (var i = 0; i < 500; i++) { commit.author().name(); commit.author().email(); - global.gc(); + if ( i % 250 === 0) { + global.gc(); + } } }); }); From 9432ae05ac6cb33d22dbeb2c91cff1c499c3bd5c Mon Sep 17 00:00:00 2001 From: Tim Branyen Date: Mon, 24 Nov 2014 14:01:23 -0500 Subject: [PATCH 5/7] Adds in a BUILD_ONLY environment variable By default the module will attempt to install from a prebuilt binary from S3. If this fails it will attempt a native compile. For our continuous integration and local development we'll always run with `BUILD_ONLY=true`. --- .travis.yml | 2 +- appveyor.yml | 1 + generate/index.js | 4 ++- generate/setup.js | 12 ++++++--- generate/utils.js | 4 ++- install.js | 63 ++++++++++++++++++++++------------------------- package.json | 2 +- 7 files changed, 47 insertions(+), 41 deletions(-) diff --git a/.travis.yml b/.travis.yml index 3c5d107ef..8a78fce67 100644 --- a/.travis.yml +++ b/.travis.yml @@ -14,7 +14,7 @@ before_install: export CXX='g++-4.8'; fi - "export JOBS=4" - - npm install + - BUILD_ONLY=true npm install # This is a random private key used purely for testing. before_script: - echo -e "Host *\n\tStrictHostKeyChecking no\n" >> ~/.ssh/config diff --git a/appveyor.yml b/appveyor.yml index 2612c41a8..47e35efe3 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -27,6 +27,7 @@ install: - cmd: SET PATH=C:\Program Files (x86)\MSBuild\12.0\bin\;%PATH% - cmd: SET PATH=c:\python27;%PATH% - cmd: SET JOBS=4 + - cmd: SET BUILD_ONLY=true - cmd: SET GIT_SSH=c:\projects\nodegit\vendor\plink.exe - ps: Start-Process c:\projects\nodegit\vendor\pageant.exe c:\projects\nodegit\vendor\private.ppk - cmd: npm install -g node-gyp diff --git a/generate/index.js b/generate/index.js index ea479600f..23e36792e 100644 --- a/generate/index.js +++ b/generate/index.js @@ -100,7 +100,9 @@ fse.remove(path.resolve(__dirname, "../src")).then(function() { } } catch (e) { - console.log(e); + if (process.env.BUILD_ONLY) { + console.log(e); + } } }); diff --git a/generate/setup.js b/generate/setup.js index c92d44b13..98cb3617f 100644 --- a/generate/setup.js +++ b/generate/setup.js @@ -58,12 +58,16 @@ libgit2.types.forEach(function(current) { var previous = ""; enums = _(enums).sortBy("name").reduce(function(enumMemo, enumerable) { if (previous == enumerable.typeName) { - console.log('WARNING: duplicate definition for enum ' + enumerable.typeName + - ". skipped."); + if (process.env.BUILD_ONLY) { + console.warn('Duplicate definition for enum ' + enumerable.typeName + + ". skipped."); + } } else if (!enumerable.fields) { - console.log('WARNING: incomplete definition for enum ' + enumerable.typeName + - ". skipped."); + if (process.env.BUILD_ONLY) { + console.warn('Incomplete definition for enum ' + enumerable.typeName + + ". skipped."); + } } else { enumMemo[enumerable.typeName] = { diff --git a/generate/utils.js b/generate/utils.js index ff75d67b3..24f612d8b 100644 --- a/generate/utils.js +++ b/generate/utils.js @@ -136,7 +136,9 @@ var Utils = { _.merge(field, callbackDefs[field.type]); } else { - console.log("WARNING: Couldn't find callback definition for " + field.type); + if (process.env.BUILD_ONLY) { + console.warn("Couldn't find callback definition for " + field.type); + } } }, diff --git a/install.js b/install.js index 33dd5d4fb..bc1d876fb 100644 --- a/install.js +++ b/install.js @@ -10,13 +10,8 @@ var which = require("which"); var rimraf = require("rimraf"); var NODE_VERSION = Number(process.version.match(/^v(\d+\.\d+)/)[1]); -// If the build only flag is set -var buildOnly = process.argv.indexOf("--build-only") > -1; - -if (buildOnly) { - console.log("buildOnly"); - return; -} +// If the build only flag is set. +var buildOnly = process.env.BUILD_ONLY; // This will take in an object and find any matching keys in the environment // to use as overrides. @@ -71,13 +66,34 @@ if (NODE_VERSION === 0.1) { pkg.http_parser = pkg.http_parser["0.10"]; } -// Ensure all dependencies are available. -var dependencies = Q.allSettled([ - // This will prioritize `python2` over `python`, because we always want to - // work with Python 2.* if it"s available. - Q.nfcall(which, "python2"), - Q.nfcall(which, "python") -]) +// Attempt to fallback on a prebuilt binary. +function fetchPrebuilt() { + if (!buildOnly) { + console.info("[nodegit] Fetching binary from S3."); + + // Using the node-pre-gyp module, attempt to fetch a compatible build. + return Q.nfcall(exec, "node-pre-gyp install"); + } + + throw new Error("Build only"); +} + +// Attempt to fetch prebuilt binary. +Q.ninvoke(fs, "mkdir", paths.release).then(fetchPrebuilt, fetchPrebuilt) + +.fail(function() { + if (!buildOnly) { + console.info("[nodegit] Failed to install prebuilt, attempting compile."); + } + + // Ensure all dependencies are available. + return Q.allSettled([ + // This will prioritize `python2` over `python`, because we always want to + // work with Python 2.* if it"s available. + Q.nfcall(which, "python2"), + Q.nfcall(which, "python") + ]) +}) // Determine if all the dependency requirements are met. .then(function(results) { @@ -226,25 +242,6 @@ var dependencies = Q.allSettled([ }); }) -// Attempt to fallback on a prebuilt binary. -.fail(function(message) { - console.info("[nodegit] Failed to build nodegit."); - console.info("[nodegit] Attempting to fallback on a prebuilt binary."); - - console.log(message.stack); - - function fetchPrebuilt() { - console.info("[nodegit] Fetching binary from S3."); - - // Using the node-pre-gyp module, attempt to fetch a compatible build. - return Q.nfcall(exec, "node-pre-gyp install"); - } - - // Attempt to fetch prebuilt binary. - return Q.ninvoke(fs, "mkdir", paths.release) - .then(fetchPrebuilt, fetchPrebuilt); -}) - // Display a warning message about failing to build native node module. .fail(function(message) { console.info("[nodegit] Failed to build and install nodegit."); diff --git a/package.json b/package.json index e18991605..e725c95e8 100644 --- a/package.json +++ b/package.json @@ -96,6 +96,6 @@ "publish": "node-pre-gyp package && node-pre-gyp publish", "generate": "node generate/setup && node generate", "install": "npm run generate && node install", - "rebuild": "npm run generate && node-gyp configure build" + "rebuild": "BUILD_ONLY=true npm run generate && node-gyp configure build" } } From 5b029725d3381b33f4df21415a13659ef49c3b6f Mon Sep 17 00:00:00 2001 From: Tim Branyen Date: Mon, 24 Nov 2014 15:23:00 -0500 Subject: [PATCH 6/7] More advanced test script Can determine environment and which flags should be set. Will fix expose-gc bug in Windows. Test coverage reports have been moved to the test folder and the gitignore is updated accordingly. --- .gitignore | 2 +- package.json | 2 +- test/index.js | 8 ++++++++ 3 files changed, 10 insertions(+), 2 deletions(-) create mode 100644 test/index.js diff --git a/.gitignore b/.gitignore index d3c4d70cf..90de04af7 100644 --- a/.gitignore +++ b/.gitignore @@ -3,7 +3,7 @@ /vendor/libssh2/ /vendor/http_parser/ /build/ -/coverage/ +/test/coverage/ /test/repos/ /src/ /include/ diff --git a/package.json b/package.json index e725c95e8..c92d421f2 100644 --- a/package.json +++ b/package.json @@ -89,7 +89,7 @@ }, "scripts": { "lint": "jshint lib test/tests", - "cov": "node --expose-gc ./node_modules/istanbul/lib/cli.js cover _mocha -- test/runner test/tests --report=lcov", + "cov": "node test", "mocha": "mocha test/runner test/tests", "test": "npm run lint && npm run cov", "missing-tests": "node generate/missing-tests", diff --git a/test/index.js b/test/index.js new file mode 100644 index 000000000..020693807 --- /dev/null +++ b/test/index.js @@ -0,0 +1,8 @@ +var args = "cover _mocha -- runner tests --report=lcov".split(" "); + +require("child_process").fork("../node_modules/istanbul/lib/cli.js", args, { + cwd: __dirname, + execArgv: [ + process.platform !== "win32" ? "--expose-gc" : "" + ] +}); From c4aa95892a8cbe657d3bc4f3d137715ce4d8990e Mon Sep 17 00:00:00 2001 From: John Haley Date: Mon, 24 Nov 2014 16:31:06 -0700 Subject: [PATCH 7/7] Fixed tests running on windows --- .gitignore | 1 + test/index.js | 15 ++++++++++----- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/.gitignore b/.gitignore index 90de04af7..5188a2e6a 100644 --- a/.gitignore +++ b/.gitignore @@ -5,6 +5,7 @@ /build/ /test/coverage/ /test/repos/ +/test/test/repos/ /src/ /include/ /lib/enums.js diff --git a/test/index.js b/test/index.js index 020693807..288e7a720 100644 --- a/test/index.js +++ b/test/index.js @@ -1,8 +1,13 @@ -var args = "cover _mocha -- runner tests --report=lcov".split(" "); +var args = [ + "cover", + process.platform != "win32" ? "_mocha" : "../node_modules/mocha/bin/_mocha", + "--", + "runner", + "tests", + "--report=lcov", + "--expose-gc" + ]; require("child_process").fork("../node_modules/istanbul/lib/cli.js", args, { - cwd: __dirname, - execArgv: [ - process.platform !== "win32" ? "--expose-gc" : "" - ] + cwd: __dirname });