Skip to content

Commit

Permalink
test(all): Refactor into Jest idioms, adding integration tests (#714)
Browse files Browse the repository at this point in the history
* Unify test patterns under Jest idioms, with selective assertions that
are much more readable.
* Improve the concurrency of the tests by reducing/removing synchronous
operations that block an entire process.
* Set the stage for async/await with libraries that have an easy
migration path (execa, fs-promise, etc).

### `test/*`

* Use `jest.mock()` where it makes sense, targeted `jest.fn()` otherwise
* Convert `assert` -> `expect`

### `test/integration`

* run CLI integration tests after unit tests
* add concurrent InitCommand integration tests
* add concurrent BootstrapCommand integration tests

### `test/helpers`

* add `callsBack`
* add `initDirName`
* add `updateLernaConfig`
* add `normalizeRelativeDir`
* add `replaceLernaVersion` snapshot serializer
* refactor library usage
    - Use fs-promise instead of cpr
    - Use execa instead of child_process
    - Use fs-promise instead of graceful-fs, cpr, rimraf, and mkdirp

### `src/**`

* BootstrapCommand: move batchedPackages filtering into initialize()
* BootstrapCommand: PackageUtilities.getFilteredPackage() -> isHoistedPackage()
* Command: pass rootPath to GitUtilities.isInitialized()
* DiffCommand: pass opts.cwd to spawn() with refactored args construction
* FileSystemUtilities: .toString() on utf8 readFileSync is redundant
* FileSystemUtilities: remove unused mkdirSync() method
* FileSystemUtilities: split posix and windows guts of symlink methods for clarity
* GitUtilities: init() and isInitialized() pass opts.cwd from parameter
* GitUtilities: remove unused getTopLevelDirectory() method
* InitCommand: check repository.initVersion instead of existsSync()
* InitCommand: pass rootPath to GitUtilities.{isInitialized,init}()
* InitCommand: refactor JSON serialization with libraries
* NpmUtilities: extract dependencyIsSatisfied() method into separate file
* NpmUtilities: splitVersion() does not need to be public
* NpmUtilities: use read-pkg when reading package.json
* NpmUtilities: use write-pkg when writing package.json
* Package: add .toJSON() method, replacing .toJsonString() method
* PackageUtilities: rename getFilteredPackage() -> isHoistedPackage()
* PackageUtilities: privatize filterPackages() method
* PackageUtilities: refactor getPackage() internals
* PackageUtilities: remove getFilteredPackage() method
* PublishCommand: don't pass GitUtilities.addFile directly to a forEach
* PublishCommand: refactor JSON serialization with libraries
* Repository: use read-pkg instead of load-json-file for package.json
* utils/dependencyIsSatisfied: extract from NpmUtilities

### Dependencies Added

* `execa`
* `fs-promise`
* `read-pkg`
* `write-json-file`
* `write-pkg`

### Dependencies Removed

* `babel-register`
* `cpr`
* `mkdirp` (explicit)
* `object-assign-sorted`
* `pify` (explicit)

### Dependencies Updated

* `babel-cli`
* `babel-preset-es2015`
* `inquirer`
* `normalize-newline`
* `normalize-path` (moved to `devDependencies`)
* `path-exists`
  • Loading branch information
evocateur authored Apr 6, 2017
1 parent 6de5cc5 commit 048d0cb
Show file tree
Hide file tree
Showing 69 changed files with 4,609 additions and 3,189 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,4 @@ tmp
coverage/
.nyc_output/
.eslintcache
*.tgz
29 changes: 15 additions & 14 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,14 @@
],
"scripts": {
"build": "babel src -d lib",
"coverage": "jest --coverage",
"dev": "babel -w src -d lib",
"lint": "eslint . --ignore-path .gitignore",
"fix": "npm run lint -- --fix",
"preintegration": "npm pack --silent",
"integration": "jest --config test/config/integration.json",
"pretest": "npm run lint -- --cache",
"test": "jest",
"ci": "npm run lint && npm run coverage -- --verbose",
"ci": "npm test -- --coverage --verbose && npm run integration",
"prepublish": "npm run build"
},
"repository": {
Expand All @@ -38,43 +39,43 @@
"conventional-recommended-bump": "^1.0.0",
"cross-spawn": "^4.0.0",
"dedent": "^0.7.0",
"execa": "^0.6.3",
"find-up": "^2.1.0",
"fs-promise": "^2.0.2",
"glob": "^7.0.6",
"graceful-fs": "^4.1.11",
"inquirer": "^3.0.1",
"inquirer": "^3.0.6",
"load-json-file": "^2.0.0",
"lodash": "^4.17.4",
"meow": "^3.7.0",
"minimatch": "^3.0.0",
"mkdirp": "^0.5.1",
"normalize-path": "^2.0.1",
"object-assign-sorted": "^2.0.1",
"path-exists": "^2.1.0",
"path-exists": "^3.0.0",
"progress": "^2.0.0",
"read-cmd-shim": "^1.0.1",
"read-pkg": "^2.0.0",
"rimraf": "^2.6.1",
"semver": "^5.1.0",
"signal-exit": "^3.0.2"
"signal-exit": "^3.0.2",
"write-json-file": "^2.0.0",
"write-pkg": "^2.1.0"
},
"bin": {
"lerna": "./bin/lerna.js"
},
"devDependencies": {
"babel-cli": "^6.7.5",
"babel-cli": "^6.24.0",
"babel-eslint": "^6.0.2",
"babel-jest": "^19.0.0",
"babel-plugin-add-module-exports": "^0.2.1",
"babel-plugin-transform-decorators-legacy": "^1.3.4",
"babel-preset-es2015": "^6.6.0",
"babel-register": "^6.7.2",
"cpr": "^2.0.2",
"babel-preset-es2015": "^6.24.0",
"eslint": "^2.3.0",
"eslint-config-babel": "^1.0.1",
"eslint-plugin-babel": "^3.3.0",
"eslint-plugin-flow-vars": "^0.5.0",
"jest": "^19.0.2",
"normalize-newline": "^2.1.0",
"pify": "^2.3.0"
"normalize-newline": "^3.0.0",
"normalize-path": "^2.1.1"
},
"jest": {
"collectCoverageFrom": [
Expand Down
2 changes: 1 addition & 1 deletion src/Command.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ export default class Command {
}

runValidations() {
if (!GitUtilities.isInitialized()) {
if (!GitUtilities.isInitialized(this.repository.rootPath)) {
this.logger.warn("This is not a git repository, did you already run `git init` or `lerna init`?");
this._complete(null, 1);
return;
Expand Down
113 changes: 73 additions & 40 deletions src/FileSystemUtilities.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import fs from "graceful-fs";
import fs from "fs-promise";
import pathExists from "path-exists";
import logger from "./logger";
import mkdirp from "mkdirp";
import cmdShim from "cmd-shim";
import readCmdShim from "read-cmd-shim";
import path from "path";
Expand All @@ -14,14 +13,9 @@ function ensureEndsWithNewLine(string) {
}

export default class FileSystemUtilities {
@logger.logifySync()
static mkdirSync(filePath) {
fs.mkdirSync(filePath);
}

@logger.logifyAsync()
static mkdirp(filePath, callback) {
mkdirp(filePath, { fs }, callback);
fs.ensureDir(filePath, callback);
}

@logger.logifySync()
Expand Down Expand Up @@ -56,7 +50,7 @@ export default class FileSystemUtilities {

@logger.logifySync()
static readFileSync(filePath) {
return fs.readFileSync(filePath, "utf-8").toString().trim();
return fs.readFileSync(filePath, "utf8").trim();
}

@logger.logifySync()
Expand All @@ -80,24 +74,11 @@ export default class FileSystemUtilities {

@logger.logifyAsync()
static symlink(src, dest, type, callback) {
if (type === "exec") {
if (process.platform === "win32") {
cmdShim(src, dest, callback);
return;
}
type = "file";
}
if (process.platform !== "win32") {
src = path.relative(path.dirname(dest), src);
if (process.platform === "win32") {
createWindowsSymlink(src, dest, type, callback);
} else {
createPosixSymlink(src, dest, type, callback);
}
fs.lstat(dest, (err) => {
if (!err) {
// Something exists at `dest`. Need to remove it first.
fs.unlink(dest, () => fs.symlink(src, dest, type, callback));
} else {
fs.symlink(src, dest, type, callback);
}
});
}

@logger.logifySync()
Expand All @@ -107,20 +88,72 @@ export default class FileSystemUtilities {

@logger.logifySync()
static isSymlink(filePath) {
const lstat = fs.lstatSync(filePath);
let isSymlink = lstat && lstat.isSymbolicLink()
? path.resolve(path.dirname(filePath), fs.readlinkSync(filePath))
: false;
if (process.platform === "win32" && lstat) {
if (lstat.isFile() && !isSymlink) {
try {
return path.resolve(path.dirname(filePath), readCmdShim.sync(filePath));
} catch (e) {
return false;
}
}
isSymlink = isSymlink && path.resolve(isSymlink);
let result;

if (process.platform === "win32") {
result = resolveWindowsSymlink(filePath);
} else {
result = resolvePosixSymlink(filePath);
}

return result;
}
}

function createSymbolicLink(src, dest, type, callback) {
fs.lstat(dest, (err) => {
if (!err) {
// Something exists at `dest`. Need to remove it first.
fs.unlink(dest, () => fs.symlink(src, dest, type, callback));
} else {
fs.symlink(src, dest, type, callback);
}
return isSymlink;
});
}

function createPosixSymlink(origin, dest, type, callback) {
if (type === "exec") {
type = "file";
}
const src = path.relative(path.dirname(dest), origin);
createSymbolicLink(src, dest, type, callback);
}

function createWindowsSymlink(src, dest, type, callback) {
if (type === "exec") {
cmdShim(src, dest, callback);
} else {
createSymbolicLink(src, dest, type, callback);
}
}

function resolveSymbolicLink(filePath) {
const lstat = fs.lstatSync(filePath);
const isSymlink = lstat.isSymbolicLink()
? path.resolve(path.dirname(filePath), fs.readlinkSync(filePath))
: false;

return {
isSymlink,
lstat,
};
}

function resolvePosixSymlink(filePath) {
const {isSymlink} = resolveSymbolicLink(filePath);
return isSymlink;
}

function resolveWindowsSymlink(filePath) {
const {isSymlink, lstat} = resolveSymbolicLink(filePath);

if (lstat.isFile() && !isSymlink) {
try {
return path.resolve(path.dirname(filePath), readCmdShim.sync(filePath));
} catch (e) {
return false;
}
}

return isSymlink && path.resolve(isSymlink);
}
13 changes: 4 additions & 9 deletions src/GitUtilities.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ export default class GitUtilities {
}

@logger.logifySync()
static isInitialized() {
static isInitialized(cwd) {
try {
// we only want the return code, so ignore stdout/stderr
ChildProcessUtilities.execSync("git rev-parse", { stdio: "ignore" });
ChildProcessUtilities.execSync("git rev-parse", { cwd, stdio: "ignore" });
return true;
} catch (err) {
return false;
Expand Down Expand Up @@ -93,19 +93,14 @@ export default class GitUtilities {
return ChildProcessUtilities.execSync("git rev-parse HEAD");
}

@logger.logifySync()
static getTopLevelDirectory() {
return ChildProcessUtilities.execSync("git rev-parse --show-toplevel");
}

@logger.logifySync()
static checkoutChanges(changes) {
ChildProcessUtilities.execSync("git checkout -- " + changes);
}

@logger.logifySync()
static init() {
return ChildProcessUtilities.execSync("git init");
static init(cwd) {
return ChildProcessUtilities.execSync("git init", { cwd });
}

@logger.logifySync()
Expand Down
66 changes: 26 additions & 40 deletions src/NpmUtilities.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,26 @@
import writePkg from "write-pkg";
import ChildProcessUtilities from "./ChildProcessUtilities";
import FileSystemUtilities from "./FileSystemUtilities";
import onExit from "signal-exit";
import logger from "./logger";
import escapeArgs from "command-join";
import path from "path";
import semver from "semver";

// Take a dep like "foo@^1.0.0".
// Return a tuple like ["foo", "^1.0.0"].
// Handles scoped packages.
// Returns undefined for version if none specified.
function splitVersion(dep) {
return dep.match(/^(@?[^@]+)(?:@(.+))?/).slice(1, 3);
}

export default class NpmUtilities {
@logger.logifyAsync()
static installInDir(directory, dependencies, config, callback) {

const {registry, client} = config;
const {
registry,
client,
} = config;

// Nothing to do if we weren't given any deps.
if (!(dependencies && dependencies.length)) return callback();
Expand All @@ -27,50 +37,36 @@ export default class NpmUtilities {
if (err) return callback(err);

const cleanup = () => {

// Need to do this one synchronously because we might be doing it on exit.
FileSystemUtilities.renameSync(packageJsonBkp, packageJson);
};

// If we die we need to be sure to put things back the way we found them.
const unregister = onExit(cleanup);

// We have a few housekeeping tasks to take care of whether we succeed or fail.
const done = (err) => {
cleanup();
unregister();
callback(err);
};

// Construct a basic fake package.json with just the deps we need to install.
const tempJson = JSON.stringify({
const tempJson = {
dependencies: dependencies.reduce((deps, dep) => {
const [pkg, version] = NpmUtilities.splitVersion(dep);
const [pkg, version] = splitVersion(dep);
deps[pkg] = version || "*";
return deps;
}, {})
});
};

// Write out our temporary cooked up package.json and then install.
FileSystemUtilities.writeFile(packageJson, tempJson, (err) => {

// We have a few housekeeping tasks to take care of whether we succeed or fail.
const done = (err) => {
cleanup();
unregister();
callback(err);
};

if (err) {
return done(err);
} else {
ChildProcessUtilities.spawn(client || "npm", args, opts, done);
}
});
writePkg(packageJson, tempJson).then(() => {
ChildProcessUtilities.spawn(client || "npm", args, opts, done);
}).catch(done);
});
}

// Take a dep like "foo@^1.0.0".
// Return a tuple like ["foo", "^1.0.0"].
// Handles scoped packages.
// Returns undefined for version if none specified.
static splitVersion(dep) {
return dep.match(/^(@?[^@]+)(?:@(.+))?/).slice(1, 3);
}

@logger.logifySync()
static addDistTag(directory, packageName, version, tag, registry) {
const opts = NpmUtilities.getExecOpts(directory, registry);
Expand Down Expand Up @@ -112,16 +108,6 @@ export default class NpmUtilities {
ChildProcessUtilities.exec(`${command}`, opts, callback);
}

@logger.logifySync
static dependencyIsSatisfied(dir, dependency, needVersion) {
const packageJson = path.join(dir, dependency, "package.json");
try {
return semver.satisfies(require(packageJson).version, needVersion);
} catch (e) {
return false;
}
}

static getExecOpts(directory, registry) {
const opts = {
cwd: directory,
Expand Down
Loading

0 comments on commit 048d0cb

Please sign in to comment.