Skip to content

Commit

Permalink
fix: npx command on Windows and make sure build/test work on Windows …
Browse files Browse the repository at this point in the history
…too (#306)

* Windows CI fix (#2)

Make sure `npx gts init` runs `npm.cmd` on Windows.
Necessary changes to test/ files so they will pass on Windows

* chore: delete azure pipelines yml
  • Loading branch information
dan1wang authored and soldair committed May 1, 2019
1 parent 3341576 commit 458be26
Show file tree
Hide file tree
Showing 8 changed files with 149 additions and 40 deletions.
98 changes: 98 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
"write-file-atomic": "^2.3.0"
},
"devDependencies": {
"@types/fs-extra": "^5.0.5",
"@npm/types": "^1.0.1",
"@types/diff": "^4.0.0",
"@types/entities": "^1.1.0",
Expand All @@ -70,6 +71,7 @@
"@types/update-notifier": "^2.2.0",
"codecov": "^3.0.1",
"execa": "^1.0.0",
"fs-extra": "^7.0.1",
"inline-fixtures": "^1.0.0",
"js-green-licenses": "^0.5.0",
"mocha": "^6.0.0",
Expand Down
6 changes: 3 additions & 3 deletions src/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import * as inquirer from 'inquirer';
import * as path from 'path';

import {
getPkgManagerName,
getPkgManagerCommand,
readFilep as read,
readJsonp as readJson,
writeFileAtomicp as write,
Expand Down Expand Up @@ -76,7 +76,7 @@ export async function addScripts(
options: Options
): Promise<boolean> {
let edits = false;
const pkgManager = getPkgManagerName(options.yarn);
const pkgManager = getPkgManagerCommand(options.yarn);
const scripts: Bag<string> = {
check: `gts check`,
clean: 'gts clean',
Expand Down Expand Up @@ -282,7 +282,7 @@ export async function init(options: Options): Promise<boolean> {
// --ignore-scripts so that compilation doesn't happen because there's no
// source files yet.
cp.spawnSync(
getPkgManagerName(options.yarn),
getPkgManagerCommand(options.yarn),
['install', '--ignore-scripts'],
{ stdio: 'inherit' }
);
Expand Down
6 changes: 4 additions & 2 deletions src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,8 @@ export function isYarnUsed(existsSync = fs.existsSync): boolean {
return existsSync('yarn.lock');
}

export function getPkgManagerName(isYarnUsed?: boolean): 'yarn' | 'npm' {
return isYarnUsed ? 'yarn' : 'npm';
export function getPkgManagerCommand(isYarnUsed?: boolean): string {
return (
(isYarnUsed ? 'yarn' : 'npm') + (process.platform === 'win32' ? '.cmd' : '')
);
}
2 changes: 1 addition & 1 deletion test/mocha.opts
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
--require source-map-support/register
--timeout 30000
--timeout 40000
--throw-deprecation
3 changes: 2 additions & 1 deletion test/test-init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,8 @@ describe('init', () => {
assert.strictEqual(result, true);

const contents = await readJson('./package.json');
assert.strictEqual(contents.scripts.prepare, 'yarn run compile');
const cmd = process.platform === 'win32' ? 'yarn.cmd' : 'yarn';
assert.strictEqual(contents.scripts.prepare, cmd + ' run compile');
}
);
});
Expand Down
13 changes: 8 additions & 5 deletions test/test-kitchen.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import chalk from 'chalk';
import * as cp from 'child_process';
import * as fs from 'fs';
import * as fs from 'fs-extra';
import * as ncp from 'ncp';
import * as pify from 'pify';
import * as tmp from 'tmp';
Expand All @@ -16,6 +16,7 @@ const pkg = require('../../package.json');

const simpleExecp = pify(cp.exec);
const renamep = pify(fs.rename);
const movep = pify(fs.move);
const ncpp = pify(ncp.ncp);

// TODO: improve the typedefinitions in @types/node. Right now they specify
Expand Down Expand Up @@ -67,7 +68,8 @@ describe('🚰 kitchen sink', () => {
before(async () => {
await simpleExecp('npm pack');
const tarball = `${pkg.name}-${pkg.version}.tgz`;
await renamep(tarball, `${stagingPath}/gts.tgz`);
await renamep(tarball, 'gts.tgz');
await movep('gts.tgz', `${stagingPath}/gts.tgz`);
await ncpp('test/fixtures', `${stagingPath}/`);
});

Expand Down Expand Up @@ -137,7 +139,8 @@ describe('🚰 kitchen sink', () => {
});

it('should terminate generated json files with newline', async () => {
await simpleExecp('./node_modules/.bin/gts init -y', execOpts);
const GTS = `${stagingPath}/kitchen/node_modules/.bin/gts`;
await simpleExecp(`${GTS} init -y`, execOpts);
assert.ok(
fs
.readFileSync(`${stagingPath}/kitchen/package.json`, 'utf8')
Expand All @@ -164,11 +167,11 @@ describe('🚰 kitchen sink', () => {
it('should fix', async () => {
const preFix = fs
.readFileSync(`${stagingPath}/kitchen/src/server.ts`, 'utf8')
.split('\n');
.split(/[\n\r]+/);
await simpleExecp('npm run fix', execOpts);
const postFix = fs
.readFileSync(`${stagingPath}/kitchen/src/server.ts`, 'utf8')
.split('\n');
.split(/[\n\r]+/);
assert.strictEqual(preFix[0].trim() + ';', postFix[0]); // fix should have added a semi-colon
});

Expand Down
59 changes: 31 additions & 28 deletions test/test-util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {
ConfigFile,
getTSConfig,
isYarnUsed,
getPkgManagerName,
getPkgManagerCommand,
} from '../src/util';

/**
Expand All @@ -46,35 +46,37 @@ function makeFakeFsExistsSync(
): (path: PathLike) => boolean {
return (path: PathLike) => expected.some(item => item === path);
}

const FAKE_DIRECTORY = '/some/fake/directory';
const PATH_TO_TSCONFIG = path.resolve(FAKE_DIRECTORY, 'tsconfig.json');
const PATH_TO_CONFIG2 = path.resolve(FAKE_DIRECTORY, 'FAKE_CONFIG2');
const PATH_TO_CONFIG3 = path.resolve(FAKE_DIRECTORY, 'FAKE_CONFIG3');

describe('util', () => {
it('get should parse the correct tsconfig file', async () => {
const FAKE_DIRECTORY = '/some/fake/directory';
const FAKE_CONFIG1 = { files: ['b'] };

function fakeReadFilep(
configPath: string,
encoding: string
): Promise<string> {
assert.strictEqual(
configPath,
path.join(FAKE_DIRECTORY, 'tsconfig.json')
);
assert.strictEqual(configPath, PATH_TO_TSCONFIG);
assert.strictEqual(encoding, 'utf8');
return Promise.resolve(JSON.stringify(FAKE_CONFIG1));
}
const contents = await getTSConfig(FAKE_DIRECTORY, fakeReadFilep);

assert.deepStrictEqual(contents, FAKE_CONFIG1);
});

it('should throw an error if it finds a circular reference', () => {
const FAKE_DIRECTORY = '/some/fake/directory';
const FAKE_CONFIG1 = { files: ['b'], extends: 'FAKE_CONFIG2' };
const FAKE_CONFIG2 = { extends: 'FAKE_CONFIG3' };
const FAKE_CONFIG3 = { extends: 'tsconfig.json' };
const myMap = new Map();
myMap.set('/some/fake/directory/tsconfig.json', FAKE_CONFIG1);
myMap.set('/some/fake/directory/FAKE_CONFIG2', FAKE_CONFIG2);
myMap.set('/some/fake/directory/FAKE_CONFIG3', FAKE_CONFIG3);
myMap.set(PATH_TO_TSCONFIG, FAKE_CONFIG1);
myMap.set(PATH_TO_CONFIG2, FAKE_CONFIG2);
myMap.set(PATH_TO_CONFIG3, FAKE_CONFIG3);

return assert.rejects(
() => getTSConfig(FAKE_DIRECTORY, createFakeReadFilep(myMap)),
Expand All @@ -84,7 +86,6 @@ describe('util', () => {
});

it('should follow dependency chain caused by extends files', async () => {
const FAKE_DIRECTORY = '/some/fake/directory';
const FAKE_CONFIG1 = {
compilerOptions: { a: 'n' },
files: ['b'],
Expand All @@ -100,9 +101,9 @@ describe('util', () => {
};

const myMap = new Map();
myMap.set('/some/fake/directory/tsconfig.json', FAKE_CONFIG1);
myMap.set('/some/fake/directory/FAKE_CONFIG2', FAKE_CONFIG2);
myMap.set('/some/fake/directory/FAKE_CONFIG3', FAKE_CONFIG3);
myMap.set(PATH_TO_TSCONFIG, FAKE_CONFIG1);
myMap.set(PATH_TO_CONFIG2, FAKE_CONFIG2);
myMap.set(PATH_TO_CONFIG3, FAKE_CONFIG3);

const contents = await getTSConfig(
FAKE_DIRECTORY,
Expand All @@ -112,15 +113,14 @@ describe('util', () => {
});

it('when a file contains an extends field, the base file is loaded first then overridden by the inherited files', async () => {
const FAKE_DIRECTORY = '/some/fake/directory';
const FAKE_CONFIG1 = { files: ['b'], extends: 'FAKE_CONFIG2' };
const FAKE_CONFIG2 = { files: ['c'], extends: 'FAKE_CONFIG3' };
const FAKE_CONFIG3 = { files: ['d'] };
const combinedConfig = { compilerOptions: {}, files: ['b'] };
const myMap = new Map();
myMap.set('/some/fake/directory/tsconfig.json', FAKE_CONFIG1);
myMap.set('/some/fake/directory/FAKE_CONFIG2', FAKE_CONFIG2);
myMap.set('/some/fake/directory/FAKE_CONFIG3', FAKE_CONFIG3);
myMap.set(PATH_TO_TSCONFIG, FAKE_CONFIG1);
myMap.set(PATH_TO_CONFIG2, FAKE_CONFIG2);
myMap.set(PATH_TO_CONFIG3, FAKE_CONFIG3);

const contents = await getTSConfig(
FAKE_DIRECTORY,
Expand All @@ -130,7 +130,6 @@ describe('util', () => {
});

it('when reading a file, all filepaths should be relative to the config file currently being read', async () => {
const FAKE_DIRECTORY = '/some/fake/directory';
const FAKE_CONFIG1 = { files: ['b'], extends: './foo/FAKE_CONFIG2' };
const FAKE_CONFIG2 = { include: ['c'], extends: './bar/FAKE_CONFIG3' };
const FAKE_CONFIG3 = { exclude: ['d'] };
Expand All @@ -141,9 +140,12 @@ describe('util', () => {
include: ['c'],
};
const myMap = new Map();
myMap.set('/some/fake/directory/tsconfig.json', FAKE_CONFIG1);
myMap.set('/some/fake/directory/foo/FAKE_CONFIG2', FAKE_CONFIG2);
myMap.set('/some/fake/directory/foo/bar/FAKE_CONFIG3', FAKE_CONFIG3);
myMap.set(PATH_TO_TSCONFIG, FAKE_CONFIG1);
myMap.set(path.resolve(FAKE_DIRECTORY, './foo/FAKE_CONFIG2'), FAKE_CONFIG2);
myMap.set(
path.resolve(FAKE_DIRECTORY, './foo/bar/FAKE_CONFIG3'),
FAKE_CONFIG3
);

const contents = await getTSConfig(
FAKE_DIRECTORY,
Expand All @@ -153,7 +155,6 @@ describe('util', () => {
});

it('function throws an error when reading a file that does not exist', () => {
const FAKE_DIRECTORY = '/some/fake/directory';
const myMap = new Map();

return assert.rejects(
Expand All @@ -178,13 +179,15 @@ describe('util', () => {
assert.strictEqual(isYarnUsed(existsSync), false);
});

it('getPkgManagerName returns npm by default', () => {
assert.strictEqual(getPkgManagerName(), 'npm');
assert.strictEqual(getPkgManagerName(), getPkgManagerName(false));
const npmCmd = process.platform !== 'win32' ? 'npm' : 'npm.cmd';
const yarnCmd = process.platform !== 'win32' ? 'yarn' : 'yarn.cmd';
it('getPkgManagerCommand returns npm by default', () => {
assert.strictEqual(getPkgManagerCommand(), npmCmd);
assert.strictEqual(getPkgManagerCommand(), getPkgManagerCommand(false));
});

it('getPkgManagerName returns yarn', () => {
assert.strictEqual(getPkgManagerName(true), 'yarn');
it('getPkgManagerCommand returns yarn', () => {
assert.strictEqual(getPkgManagerCommand(true), yarnCmd);
});

// TODO: test errors in readFile, JSON.parse.
Expand Down

0 comments on commit 458be26

Please sign in to comment.