Skip to content
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

Improve testability #105

Merged
merged 4 commits into from
Feb 27, 2019
Merged

Improve testability #105

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5bb8cd7
Improve testability
Feb 22, 2019
844543b
Only rerun failed when --rerun-failed parameter
Feb 22, 2019
eb13718
Smarter start and revert test of build
Feb 22, 2019
f02bf84
Merge branch 'master' into improve-testability
kasperisager Feb 26, 2019
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
41 changes: 30 additions & 11 deletions scripts/helpers/typescript.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,29 +71,48 @@ function isTestable(file, project = workspace) {
exports.isTestable = isTestable;

/**
* Return the path to specification file matching specified file
*
* @param {string} file
* @return {boolean}
* @return {string?}
*/
function hasSpecification(file) {
function getSpecification(file) {
const base = path
.dirname(file)
.split(path.sep)
.map(part => (part === "src" ? "test" : part))
.map((part, index) =>
part === "src"
? "test"
: part === "scripts" && index === 0
? `${part}${path.sep}test`
: part
)
.join(path.sep);

const extensions = [".ts", ".tsx"];

for (const n of extensions) {
for (const m of extensions) {
const spec = path.join(base, `${path.basename(file, n)}.spec${m}`);
for (const extension of extensions) {
const spec = path.join(
base,
`${path.basename(file, path.extname(file))}.spec${extension}`
);

if (isFile(spec)) {
return true;
}
if (isFile(spec)) {
return spec;
}
}

return false;
return null;
}

exports.getSpecification = getSpecification;

/**
* Does the specified file have a corresponding specification
* @param {string} file
* @return {boolean}
*/
function hasSpecification(file) {
return getSpecification(file) !== null;
}

exports.hasSpecification = hasSpecification;
55 changes: 44 additions & 11 deletions scripts/start.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,38 +2,71 @@ const { watchFiles } = require("./helpers/file-system");
const { endsWith } = require("./helpers/predicates");
const { format, now } = require("./helpers/time");
const notify = require("./helpers/notify");
const { getSpecification } = require("./helpers/typescript");
const { default: chalk } = require("chalk");

const { build } = require("./tasks/build");
const { diagnose } = require("./tasks/diagnose");
const { test } = require("./tasks/test");

const isSpec = endsWith(".spec.ts", ".spec.tsx");

/**
* These are files which failed at last run
* Saved so we can process them again when next file changes
*
* @type {Array<string>}
*/
let failed = [];
Copy link
Contributor

Choose a reason for hiding this comment

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

While I do see the point of introducing something like this, I'm also a bit concerned of the impact it will have. I for one would very much dislike having my feedback stream littered with errors I already know exist because I've seen them previously. If I make a change to file A and compilation fails because I also need to change something in file B, I don't want to be reminded that file A fails every time I write to file B. Once I'm finished with file B I'll head back and check for myself if file A still fails. In the event that I forgot to do so and it still fails, CI will remind me in the worst case.

Copy link
Contributor

Choose a reason for hiding this comment

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

To clarify, I'd actually love to be reminded about file A, I just don't want to see a bunch of errors unrelated to file B if that's the file I'm writing to.

let rerunFailed = process.argv.some(v => v.toLowerCase() === "--rerun-failed");

watchFiles(
[
"packages/**/*.ts",
"packages/**/*.tsx",
"packages/**/scripts/**/*.js",
"scripts/**/*.js",
"scripts/test/**/*.ts",
"!scripts/test/**/*.js",
"docs/**/*.ts",
"docs/**/*.tsx"
],
(event, file) => {
const start = now();
failed = failed.filter(v => v !== file);
const fileList = [file, ...(rerunFailed ? failed : [])];
if (rerunFailed) {
failed = [];
}

let success = diagnose(file) && build(file);
fileList.forEach(file => {
const start = now();

if (isSpec(file)) {
success = success && test(file);
}
let success = diagnose(file) && build(file);

const duration = now(start);
if (isSpec(file)) {
success = success && test(file);
} else {
const spec = getSpecification(file);
if (spec !== null) {
success = success && test(spec);
}
}

if (success) {
notify.success(
`${file} ${format(duration, { color: "yellow", threshold: 400 })}`
);
}
const duration = now(start);

if (success) {
notify.success(
`${file} ${format(duration, { color: "yellow", threshold: 400 })}`
);
if (!rerunFailed && failed.length > 0) {
failed.forEach(file => {
notify.warn(`${chalk.gray(file)} failed previously`);
});
}
} else {
failed.push(file);
}
});
}
);

Expand Down