Skip to content

Commit

Permalink
Improve watcher (#475)
Browse files Browse the repository at this point in the history
This makes the watcher faster and more stable! As a bonus, this made the code much more organized.

- Concentrate all `process.exit` in elm-test.js (except one in Install.js where I found no clean solution). Besides making the code easier to follow, it allows the watcher to continue running (and recover) even if there are errors.

- Don’t crash/stop running tests if files are removed.

- Pick up new Elm files added.

- Fixes #225

  We already had code for waiting for the previous run to finish before starting the next run, but it queued up every single file change/add/remove event and ran that many runs one after the other. Now, for all watch events during a run we run _once_ after the current run for all of them. We also sleep 100 ms at the start of each run to catch more events happening close by (such as when adding/removing a whole folder of tests). (Note: We used to sleep at least 500 ms via the [awaitWriteFinish](https://github.com/paulmillr/chokidar#performance) chokidar option.)

  I’ve also tested manually that running elm-format on save doesn’t cause double test runs: #194

- Fixes #355

  We now find the closest elm.json upwards in the folder hierarchy.

- Resolves #453 (comment)

  The watcher now listens for changes to elm.json and properly recalculates things. I thought that I would have to recalculate some things only when elm.json changes for performance, but after making the globbing faster there was no need so we simply rebuild everything on each run.

- Closes #463 and closes #464

  I can’t find any more places to use async/await.

- elm.json is now decoded properly in one place rather than here and there and just being `any`. We now have 0 explicit `any` type annotations!

- elm-test.js now only contains CLI logic and subcommand dispatching. The code for running tests have been moved to RunTests.js and to FindTests.js (which used to be called Runner.js).

- Improved .flowconfig that is stricter. Closes #478

- And lots of little code improvements along the way! I only touched things that I needed to change anyway in order to not make the diff balloon too much.

I’ve done some manual testing on Windows, Mac and Linux to make sure things work like they should.
  • Loading branch information
lydell authored Dec 17, 2020
1 parent dafa12e commit 45c5245
Show file tree
Hide file tree
Showing 50 changed files with 1,738 additions and 881 deletions.
14 changes: 5 additions & 9 deletions .flowconfig
Original file line number Diff line number Diff line change
@@ -1,11 +1,7 @@
[ignore]

.*elm.json

[include]

[libs]

[options]
include_warnings=true
exact_by_default=true

module.ignore_non_literal_requires=true
[lints]
all=error
untyped-import=off
18 changes: 0 additions & 18 deletions flow-typed/Result.js

This file was deleted.

39 changes: 13 additions & 26 deletions lib/Compile.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
//@flow

const spawn = require('cross-spawn');
const path = require('path');
const packageInfo = require('../package.json');
const ElmCompiler = require('./ElmCompiler');
const Report = require('./Report');

function compile(
cwd /*: string */,
testFile /*: string */,
dest /*: string */,
pathToElmBinary /*: string */,
Expand All @@ -15,7 +14,7 @@ function compile(
return new Promise((resolve, reject) => {
const compileProcess = ElmCompiler.compile([testFile], {
output: dest,
spawn: spawnCompiler({ ignoreStdout: true }),
spawn: spawnCompiler({ ignoreStdout: true, cwd }),
pathToElm: pathToElmBinary,
processOpts: processOptsForReporter(report),
});
Expand All @@ -30,21 +29,6 @@ function compile(
});
}

function getGeneratedCodeDir(projectRootDir /*: string */) /*: string */ {
return path.join(
projectRootDir,
'elm-stuff',
'generated-code',
'elm-community',
'elm-test',
packageInfo.version
);
}

function getTestRootDir(projectRootDir /*: string */) /*: string */ {
return path.resolve(path.join(projectRootDir, 'tests'));
}

function compileSources(
testFilePaths /*: Array<string> */,
projectRootDir /*: string */,
Expand All @@ -57,7 +41,7 @@ function compileSources(
const compileProcess = ElmCompiler.compile(testFilePaths, {
output: '/dev/null',
cwd: projectRootDir,
spawn: spawnCompiler({ ignoreStdout: false }),
spawn: spawnCompiler({ ignoreStdout: false, cwd: projectRootDir }),
pathToElm: pathToElmBinary,
report: compilerReport,
processOpts: processOptsForReporter(report),
Expand All @@ -73,25 +57,30 @@ function compileSources(
});
}

function spawnCompiler({ ignoreStdout }) {
function spawnCompiler({ ignoreStdout, cwd }) {
return (
pathToElm /*: string */,
processArgs /*: Array<string> */,
processOpts /*: Object */
processOpts /*: child_process$spawnOpts */
) => {
const finalOpts = Object.assign({ env: process.env }, processOpts, {
const finalOpts = {
env: process.env,
...processOpts,
cwd,
stdio: [
process.stdin,
ignoreStdout ? 'ignore' : process.stdout,
process.stderr,
],
});
};

return spawn(pathToElm, processArgs, finalOpts);
};
}

function processOptsForReporter(report /*: typeof Report.Report */) {
function processOptsForReporter(
report /*: typeof Report.Report */
) /*: child_process$spawnOpts */ {
if (Report.isMachineReadable(report)) {
return { env: process.env, stdio: ['ignore', 'ignore', process.stderr] };
} else {
Expand All @@ -102,6 +91,4 @@ function processOptsForReporter(report /*: typeof Report.Report */) {
module.exports = {
compile,
compileSources,
getTestRootDir,
getGeneratedCodeDir,
};
193 changes: 193 additions & 0 deletions lib/ElmJson.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,193 @@
// @flow

const fs = require('fs');
const path = require('path');

// Poor man’s type alias. We can’t use /*:: type Dependencies = ... */ because of:
// https://github.com/prettier/prettier/issues/2597
const Dependencies /*: { [string]: string } */ = {};

const DirectAndIndirectDependencies /*: {
direct: typeof Dependencies,
indirect: typeof Dependencies,
} */ = { direct: {}, indirect: {} };

const ElmJson /*:
| {
type: 'application',
'source-directories': Array<string>,
dependencies: typeof DirectAndIndirectDependencies,
'test-dependencies': typeof DirectAndIndirectDependencies,
[string]: mixed,
}
| {
type: 'package',
dependencies: typeof Dependencies,
'test-dependencies': typeof Dependencies,
[string]: mixed,
} */ = {
type: 'package',
dependencies: Dependencies,
'test-dependencies': Dependencies,
};

function getPath(dir /*: string */) /*: string */ {
return path.join(dir, 'elm.json');
}

function write(dir /*: string */, elmJson /*: typeof ElmJson */) /*: void */ {
const elmJsonPath = getPath(dir);

try {
fs.writeFileSync(elmJsonPath, JSON.stringify(elmJson, null, 4) + '\n');
} catch (error) {
throw new Error(
`${elmJsonPath}\nFailed to write elm.json:\n${error.message}`
);
}
}

function read(dir /*: string */) /*: typeof ElmJson */ {
const elmJsonPath = getPath(dir);

try {
return readHelper(elmJsonPath);
} catch (error) {
throw new Error(
`${elmJsonPath}\nFailed to read elm.json:\n${error.message}`
);
}
}

function readHelper(elmJsonPath /*: string */) /*: typeof ElmJson */ {
const json = parseObject(
JSON.parse(fs.readFileSync(elmJsonPath, 'utf8')),
'the file'
);

switch (json.type) {
case 'application':
return {
...json,
type: 'application',
'source-directories': parseSourceDirectories(
json['source-directories']
),
dependencies: parseDirectAndIndirectDependencies(
json.dependencies,
'dependencies'
),
'test-dependencies': parseDirectAndIndirectDependencies(
json['test-dependencies'],
'test-dependencies'
),
};

case 'package':
return {
...json,
type: 'package',
dependencies: parseDependencies(json.dependencies, 'dependencies'),
'test-dependencies': parseDependencies(
json['test-dependencies'],
'test-dependencies'
),
};

default:
throw new Error(
`Expected "type" to be "application" or "package", but got: ${stringify(
json.type
)}`
);
}
}

function parseSourceDirectories(json /*: mixed */) /*: Array<string> */ {
if (!Array.isArray(json)) {
throw new Error(
`Expected "source-directories" to be an array, but got: ${stringify(
json
)}`
);
}

const result = [];

for (const [index, item] of json.entries()) {
if (typeof item !== 'string') {
throw new Error(
`Expected "source-directories"->${index} to be a string, but got: ${stringify(
item
)}`
);
}
result.push(item);
}

if (result.length === 0) {
throw new Error(
'Expected "source-directories" to contain at least one item, but it is empty.'
);
}

return result;
}

function parseDirectAndIndirectDependencies(
json /*: mixed */,
what /*: string */
) /*: typeof DirectAndIndirectDependencies */ {
const jsonObject = parseObject(json, what);
return {
direct: parseDependencies(jsonObject.direct, `${what}->"direct"`),
indirect: parseDependencies(jsonObject.indirect, `${what}->"indirect"`),
};
}

function parseDependencies(
json /*: mixed */,
what /*: string */
) /*: typeof Dependencies */ {
const jsonObject = parseObject(json, what);
const result = {};

for (const [key, value] of Object.entries(jsonObject)) {
if (typeof value !== 'string') {
throw new Error(
`Expected ${what}->${stringify(
key
)} to be a string, but got: ${stringify(value)}`
);
}
result[key] = value;
}

return result;
}

function parseObject(
json /*: mixed */,
what /*: string */
) /*: { +[string]: mixed } */ {
if (json == null || typeof json !== 'object' || Array.isArray(json)) {
throw new Error(
`Expected ${what} to be an object, but got: ${stringify(json)}`
);
}
return json;
}

function stringify(json /*: mixed */) /*: string */ {
const maybeString = JSON.stringify(json);
return maybeString === undefined ? 'undefined' : maybeString;
}

module.exports = {
DirectAndIndirectDependencies,
ElmJson,
getPath,
parseDirectAndIndirectDependencies,
read,
write,
};
Loading

0 comments on commit 45c5245

Please sign in to comment.