Skip to content

External runner fixes #24115

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 15, 2018
Merged

External runner fixes #24115

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
be08c1c
Add missing @types/node dep to so many projects, rename parent node_m…
weswigham May 14, 2018
693e2d0
Accept new baselines
weswigham May 14, 2018
904560a
Satisfy linter
weswigham May 14, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 53 additions & 2 deletions src/harness/externalCompileRunner.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
/// <reference path="runnerbase.ts" />
const fs = require("fs");
const path = require("path");
const del = require("del");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why change this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So it's required before I renamed node_modules and make it unreachable.


interface ExecResult {
stdout: Buffer;
Expand All @@ -27,9 +28,32 @@ abstract class ExternalCompileRunnerBase extends RunnerBase {
const testList = this.tests && this.tests.length ? this.tests : this.enumerateTestFiles();

describe(`${this.kind()} code samples`, () => {
const cwd = path.join(Harness.IO.getWorkspaceRoot(), this.testDir);
const placeholderName = ".node_modules";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not just add a types: [..] list?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They have one already. That only says which things get auto-included without even doing an import, but the issue here is when i say const x = require("chalk") and we walk up the tree and find ours when we can't find it locally. 🌞

const moduleDirName = "node_modules";
before(() => {
ts.forEachAncestorDirectory(cwd, dir => {
try {
fs.renameSync(path.join(dir, moduleDirName), path.join(dir, placeholderName));
}
catch {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why catch here, at least with nothing in the body?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't care if the rename fails (since it means the folder didn't exist or we didn't have permissions to rename - in either case we're just going to continue)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, as long as the failure is obvious (and linkable back to this line of code) some other way. I think in this case it will be obvious in the baselines.

// empty
}
});
});
for (const test of testList) {
this.runTest(typeof test === "string" ? test : test.file);
}
after(() => {
ts.forEachAncestorDirectory(cwd, dir => {
try {
fs.renameSync(path.join(dir, placeholderName), path.join(dir, moduleDirName));
}
catch {
// empty
}
});
});
});
}
private runTest(directoryName: string) {
Expand All @@ -42,6 +66,7 @@ abstract class ExternalCompileRunnerBase extends RunnerBase {

it("should build successfully", () => {
let cwd = path.join(Harness.IO.getWorkspaceRoot(), cls.testDir, directoryName);
const originalCwd = cwd;
const stdio = isWorker ? "pipe" : "inherit";
let types: string[];
if (fs.existsSync(path.join(cwd, "test.json"))) {
Expand All @@ -64,14 +89,17 @@ abstract class ExternalCompileRunnerBase extends RunnerBase {
fs.unlinkSync(path.join(cwd, "package-lock.json"));
}
if (fs.existsSync(path.join(cwd, "node_modules"))) {
require("del").sync(path.join(cwd, "node_modules"), { force: true });
del.sync(path.join(cwd, "node_modules"), { force: true });
}
const install = cp.spawnSync(`npm`, ["i", "--ignore-scripts"], { cwd, timeout: timeout / 2, shell: true, stdio }); // NPM shouldn't take the entire timeout - if it takes a long time, it should be terminated and we should log the failure
if (install.status !== 0) throw new Error(`NPM Install for ${directoryName} failed: ${install.stderr.toString()}`);
}
const args = [path.join(Harness.IO.getWorkspaceRoot(), "built/local/tsc.js")];
if (types) {
args.push("--types", types.join(","));
// Also actually install those types (for, eg, the js projects which need node)
const install = cp.spawnSync(`npm`, ["i", ...types.map(t => `@types/${t}`), "--no-save", "--ignore-scripts"], { cwd: originalCwd, timeout: timeout / 2, shell: true, stdio }); // NPM shouldn't take the entire timeout - if it takes a long time, it should be terminated and we should log the failure
if (install.status !== 0) throw new Error(`NPM Install types for ${directoryName} failed: ${install.stderr.toString()}`);
}
args.push("--noEmit");
Harness.Baseline.runBaseline(`${cls.kind()}/${directoryName}.log`, () => {
Expand All @@ -91,7 +119,7 @@ class UserCodeRunner extends ExternalCompileRunnerBase {
// tslint:disable-next-line:no-null-keyword
return result.status === 0 && !result.stdout.length && !result.stderr.length ? null : `Exit Code: ${result.status}
Standard output:
${stripAbsoluteImportPaths(result.stdout.toString().replace(/\r\n/g, "\n"))}
${sortErrors(stripAbsoluteImportPaths(result.stdout.toString().replace(/\r\n/g, "\n")))}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this change address the path changes (eg from E:/... to /opt/...)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They were already fixed by the stripAbsoluteImportPaths function - the only one that remained was one from outside the test directly, which shouldn't have been in the test in the first place (and is now gone).



Standard error:
Expand All @@ -109,6 +137,29 @@ function stripAbsoluteImportPaths(result: string) {
.replace(/Module '".*?\/tests\/cases\/user\//g, `Module '"/`);
}

function sortErrors(result: string) {
return ts.flatten(splitBy(result.split("\n"), s => /^\S+/.test(s)).sort(compareErrorStrings)).join("\n");
}

const errorRegexp = /^(.+\.[tj]sx?)\((\d+),(\d+)\): error TS/;
function compareErrorStrings(a: string[], b: string[]) {
ts.Debug.assertGreaterThanOrEqual(a.length, 1);
ts.Debug.assertGreaterThanOrEqual(b.length, 1);
const matchA = a[0].match(errorRegexp);
if (!matchA) {
return -1;
}
const matchB = b[0].match(errorRegexp);
if (!matchB) {
return 1;
}
const [, errorFileA, lineNumberStringA, columnNumberStringA] = matchA;
const [, errorFileB, lineNumberStringB, columnNumberStringB] = matchB;
return ts.comparePathsCaseSensitive(errorFileA, errorFileB) ||
ts.compareValues(parseInt(lineNumberStringA), parseInt(lineNumberStringB)) ||
ts.compareValues(parseInt(columnNumberStringA), parseInt(columnNumberStringB));
}

class DefinitelyTypedRunner extends ExternalCompileRunnerBase {
readonly testDir = "../DefinitelyTyped/types/";
workingDirectory = this.testDir;
Expand Down
12 changes: 6 additions & 6 deletions tests/baselines/reference/user/adonis-framework.log
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,12 @@ node_modules/adonis-framework/src/Session/Drivers/File/index.js(79,15): error TS
node_modules/adonis-framework/src/Session/Drivers/Redis/index.js(23,14): error TS1056: Accessors are only available when targeting ECMAScript 5 and higher.
node_modules/adonis-framework/src/Session/Drivers/Redis/index.js(59,15): error TS2322: Type 'IterableIterator<any>' is not assignable to type 'boolean'.
node_modules/adonis-framework/src/Session/Drivers/Redis/index.js(72,15): error TS2322: Type 'IterableIterator<any>' is not assignable to type 'boolean'.
node_modules/adonis-framework/src/Session/SessionManager.js(13,21): error TS2307: Cannot find module 'adonis-fold'.
node_modules/adonis-framework/src/Session/SessionManager.js(69,22): error TS2339: Property 'drivers' does not exist on type 'Function'.
node_modules/adonis-framework/src/Session/SessionManager.js(69,49): error TS2339: Property 'drivers' does not exist on type 'Function'.
node_modules/adonis-framework/src/Session/SessionManager.js(71,76): error TS2339: Property 'drivers' does not exist on type 'Function'.
node_modules/adonis-framework/src/Session/Store.js(28,13): error TS2304: Cannot find name 'Mixed'.
node_modules/adonis-framework/src/Session/Store.js(80,13): error TS2304: Cannot find name 'Mixed'.
node_modules/adonis-framework/src/Session/index.js(10,14): error TS2304: Cannot find name 'SessionDriver'.
node_modules/adonis-framework/src/Session/index.js(11,2): error TS1003: Identifier expected.
node_modules/adonis-framework/src/Session/index.js(11,11): error TS2304: Cannot find name 'Class'.
Expand Down Expand Up @@ -173,12 +179,6 @@ node_modules/adonis-framework/src/Session/index.js(249,15): error TS2304: Cannot
node_modules/adonis-framework/src/Session/index.js(267,15): error TS2304: Cannot find name 'Mixed'.
node_modules/adonis-framework/src/Session/index.js(287,15): error TS2322: Type 'IterableIterator<any>' is not assignable to type 'boolean'.
node_modules/adonis-framework/src/Session/index.js(293,12): error TS2532: Object is possibly 'undefined'.
node_modules/adonis-framework/src/Session/SessionManager.js(13,21): error TS2307: Cannot find module 'adonis-fold'.
node_modules/adonis-framework/src/Session/SessionManager.js(69,22): error TS2339: Property 'drivers' does not exist on type 'Function'.
node_modules/adonis-framework/src/Session/SessionManager.js(69,49): error TS2339: Property 'drivers' does not exist on type 'Function'.
node_modules/adonis-framework/src/Session/SessionManager.js(71,76): error TS2339: Property 'drivers' does not exist on type 'Function'.
node_modules/adonis-framework/src/Session/Store.js(28,13): error TS2304: Cannot find name 'Mixed'.
node_modules/adonis-framework/src/Session/Store.js(80,13): error TS2304: Cannot find name 'Mixed'.
node_modules/adonis-framework/src/View/Form/index.js(75,11): error TS2532: Object is possibly 'undefined'.
node_modules/adonis-framework/src/View/Form/index.js(115,15): error TS2304: Cannot find name 'Mixed'.
node_modules/adonis-framework/src/View/Form/index.js(147,63): error TS2345: Argument of type 'string | any[]' is not assignable to parameter of type 'any[]'.
Expand Down
10 changes: 5 additions & 5 deletions tests/baselines/reference/user/async.log
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ node_modules/async/auto.js(159,18): error TS2695: Left side of comma operator is
node_modules/async/auto.js(159,50): error TS2695: Left side of comma operator is unused and has no side effects.
node_modules/async/autoInject.js(44,17): error TS2695: Left side of comma operator is unused and has no side effects.
node_modules/async/autoInject.js(134,6): error TS2695: Left side of comma operator is unused and has no side effects.
node_modules/async/autoInject.js(136,25): error TS2532: Object is possibly 'undefined'.
node_modules/async/autoInject.js(136,25): error TS2722: Cannot invoke an object which is possibly 'undefined'.
node_modules/async/autoInject.js(136,25): error TS2532: Object is possibly 'undefined'.
node_modules/async/autoInject.js(136,26): error TS2695: Left side of comma operator is unused and has no side effects.
node_modules/async/autoInject.js(139,14): error TS2695: Left side of comma operator is unused and has no side effects.
node_modules/async/autoInject.js(160,28): error TS2695: Left side of comma operator is unused and has no side effects.
Expand Down Expand Up @@ -145,9 +145,9 @@ node_modules/async/dist/async.js(2963,25): error TS2722: Cannot invoke an object
node_modules/async/dist/async.js(2970,25): error TS2722: Cannot invoke an object which is possibly 'undefined'.
node_modules/async/dist/async.js(2971,28): error TS2722: Cannot invoke an object which is possibly 'undefined'.
node_modules/async/dist/async.js(3005,25): error TS2722: Cannot invoke an object which is possibly 'undefined'.
node_modules/async/dist/async.js(3008,9): error TS2532: Object is possibly 'undefined'.
node_modules/async/dist/async.js(3008,9): error TS2684: The 'this' context of type 'Function | undefined' is not assignable to method's 'this' of type 'Function'.
Type 'undefined' is not assignable to type 'Function'.
node_modules/async/dist/async.js(3008,9): error TS2532: Object is possibly 'undefined'.
node_modules/async/dist/async.js(3081,25): error TS2722: Cannot invoke an object which is possibly 'undefined'.
node_modules/async/dist/async.js(3086,25): error TS2722: Cannot invoke an object which is possibly 'undefined'.
node_modules/async/dist/async.js(3087,28): error TS2722: Cannot invoke an object which is possibly 'undefined'.
Expand Down Expand Up @@ -175,18 +175,18 @@ node_modules/async/dist/async.js(4153,14): error TS2339: Property 'unshift' does
node_modules/async/dist/async.js(4367,5): error TS2322: Type 'any[] | {}' is not assignable to type 'any[]'.
Type '{}' is not assignable to type 'any[]'.
Property 'flatMap' is missing in type '{}'.
node_modules/async/dist/async.js(4603,17): error TS2532: Object is possibly 'undefined'.
node_modules/async/dist/async.js(4603,17): error TS2684: The 'this' context of type 'Function | undefined' is not assignable to method's 'this' of type 'Function'.
Type 'undefined' is not assignable to type 'Function'.
node_modules/async/dist/async.js(4603,17): error TS2532: Object is possibly 'undefined'.
node_modules/async/dist/async.js(4917,19): error TS2339: Property 'code' does not exist on type 'Error'.
node_modules/async/dist/async.js(4919,23): error TS2339: Property 'info' does not exist on type 'Error'.
node_modules/async/dist/async.js(5090,9): error TS2722: Cannot invoke an object which is possibly 'undefined'.
node_modules/async/dist/async.js(5146,9): error TS2722: Cannot invoke an object which is possibly 'undefined'.
node_modules/async/dist/async.js(5165,20): error TS2339: Property 'unmemoized' does not exist on type 'Function'.
node_modules/async/dist/async.js(5208,25): error TS2722: Cannot invoke an object which is possibly 'undefined'.
node_modules/async/dist/async.js(5211,9): error TS2532: Object is possibly 'undefined'.
node_modules/async/dist/async.js(5211,9): error TS2684: The 'this' context of type 'Function | undefined' is not assignable to method's 'this' of type 'Function'.
Type 'undefined' is not assignable to type 'Function'.
node_modules/async/dist/async.js(5211,9): error TS2532: Object is possibly 'undefined'.
node_modules/async/dist/async.js(5315,20): error TS2532: Object is possibly 'undefined'.
node_modules/async/dist/async.js(5315,20): error TS2684: The 'this' context of type 'Function | undefined' is not assignable to method's 'this' of type 'Function'.
Type 'undefined' is not assignable to type 'Function'.
Expand Down Expand Up @@ -240,8 +240,8 @@ node_modules/async/eachSeries.js(28,12): error TS2304: Cannot find name 'AsyncFu
node_modules/async/eachSeries.js(36,20): error TS2695: Left side of comma operator is unused and has no side effects.
node_modules/async/ensureAsync.js(34,12): error TS2304: Cannot find name 'AsyncFunction'.
node_modules/async/ensureAsync.js(36,14): error TS2304: Cannot find name 'AsyncFunction'.
node_modules/async/ensureAsync.js(56,9): error TS2532: Object is possibly 'undefined'.
node_modules/async/ensureAsync.js(56,9): error TS2722: Cannot invoke an object which is possibly 'undefined'.
node_modules/async/ensureAsync.js(56,9): error TS2532: Object is possibly 'undefined'.
node_modules/async/ensureAsync.js(56,10): error TS2695: Left side of comma operator is unused and has no side effects.
node_modules/async/ensureAsync.js(57,13): error TS2695: Left side of comma operator is unused and has no side effects.
node_modules/async/ensureAsync.js(62,18): error TS2695: Left side of comma operator is unused and has no side effects.
Expand Down
14 changes: 7 additions & 7 deletions tests/baselines/reference/user/bcryptjs.log
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,6 @@ Standard output:
node_modules/bcryptjs/scripts/build.js(1,26): error TS2307: Cannot find module 'metascript'.
node_modules/bcryptjs/scripts/build.js(32,1): error TS2322: Type '{ VERSION: any; }' is not assignable to type '{ [x: string]: any; VERSION: any; ISAAC: boolean; }'.
Property 'ISAAC' is missing in type '{ VERSION: any; }'.
node_modules/bcryptjs/src/bcrypt.js(25,13): error TS2322: Type 'Buffer' is not assignable to type 'number[]'.
Property 'flatMap' is missing in type 'Buffer'.
node_modules/bcryptjs/src/bcrypt.js(94,14): error TS2366: Function lacks ending return statement and return type does not include 'undefined'.
node_modules/bcryptjs/src/bcrypt.js(150,5): error TS2322: Type 'string | undefined' is not assignable to type 'string'.
Type 'undefined' is not assignable to type 'string'.
node_modules/bcryptjs/src/bcrypt.js(160,14): error TS2366: Function lacks ending return statement and return type does not include 'undefined'.
node_modules/bcryptjs/src/bcrypt.js(238,14): error TS2366: Function lacks ending return statement and return type does not include 'undefined'.
node_modules/bcryptjs/src/bcrypt/impl.js(516,22): error TS2345: Argument of type 'Int32Array | number[]' is not assignable to parameter of type 'number[]'.
Type 'Int32Array' is not assignable to type 'number[]'.
Property 'flatMap' is missing in type 'Int32Array'.
Expand All @@ -27,6 +20,13 @@ node_modules/bcryptjs/src/bcrypt/prng/accum.js(65,74): error TS2339: Property 'd
node_modules/bcryptjs/src/bcrypt/prng/accum.js(66,22): error TS2339: Property 'detachEvent' does not exist on type 'Document'.
node_modules/bcryptjs/src/bcrypt/prng/accum.js(67,22): error TS2339: Property 'detachEvent' does not exist on type 'Document'.
node_modules/bcryptjs/src/bcrypt/util.js(20,5): error TS2304: Cannot find name 'utfx'.
node_modules/bcryptjs/src/bcrypt.js(25,13): error TS2322: Type 'Buffer' is not assignable to type 'number[]'.
Property 'flatMap' is missing in type 'Buffer'.
node_modules/bcryptjs/src/bcrypt.js(94,14): error TS2366: Function lacks ending return statement and return type does not include 'undefined'.
node_modules/bcryptjs/src/bcrypt.js(150,5): error TS2322: Type 'string | undefined' is not assignable to type 'string'.
Type 'undefined' is not assignable to type 'string'.
node_modules/bcryptjs/src/bcrypt.js(160,14): error TS2366: Function lacks ending return statement and return type does not include 'undefined'.
node_modules/bcryptjs/src/bcrypt.js(238,14): error TS2366: Function lacks ending return statement and return type does not include 'undefined'.
node_modules/bcryptjs/src/wrap.js(37,26): error TS2304: Cannot find name 'define'.
node_modules/bcryptjs/src/wrap.js(37,51): error TS2304: Cannot find name 'define'.
node_modules/bcryptjs/src/wrap.js(38,9): error TS2304: Cannot find name 'define'.
Expand Down
Loading