Skip to content

Commit

Permalink
Add --only-changed feature
Browse files Browse the repository at this point in the history
  • Loading branch information
ghengeveld committed Mar 10, 2021
1 parent 3a86d4c commit eefd42a
Show file tree
Hide file tree
Showing 11 changed files with 1,390 additions and 1,447 deletions.
6 changes: 6 additions & 0 deletions bin/git/git.js
Expand Up @@ -318,6 +318,12 @@ export async function getBaselineCommits(
];
}

export async function getChangedFiles(baseCommit, headCommit = '') {
// Note that an empty headCommit will include uncommitted (staged or unstaged) changes.
const files = await execGitCommand(`git diff --name-only ${baseCommit} ${headCommit}`);
return files.split(EOL).filter(Boolean);
}

/**
* Returns a boolean indicating whether the workspace is up-to-date (neither ahead nor behind) with
* the remote.
Expand Down
94 changes: 94 additions & 0 deletions bin/lib/getDependentStoryFiles.js
@@ -0,0 +1,94 @@
const isUserCode = (mod) => !mod.name.match(/(node_modules|webpack\/runtime)/);

const modToParts = (m) => [
m.name,
m.modules && m.modules.map(modToParts),
[...new Set(m.reasons.map((r) => r.moduleName))],
];

// TODO -- obviously this can depend on (-C)
const STORYBOOK_DIR = './.storybook';
// NOTE: this only works with `main:stories` -- if stories are imported from files in `.storybook/preview.js`
// we'll need a different approach to figure out CSF files (maybe the user should pass a glob?).
const STORIES_ENTRY = `${STORYBOOK_DIR}/generated-stories-entry.js`;

export function statsToDependencies(stats) {
const { modules } = stats;
// console.dir(modules.filter(isUserCode).map(modToParts), { depth: null });

const idsMap = {}; // Map module name to id
const reasonsMap = {}; // A reason is a dependent ==> map id to reasons
const isCsfGlob = {}; // Is a given module name a CSF glob specified in a `require.context()`

modules.filter(isUserCode).forEach((mod) => {
if (mod.id) {
idsMap[mod.name] = mod.id;
(mod.modules ? mod.modules.map((m) => m.name) : []).forEach((name) => {
idsMap[name] = mod.id;
});
}

reasonsMap[mod.id] = mod.reasons
.map((r) => r.moduleName)
.filter(Boolean)
.filter((n) => n !== mod.name);

if (reasonsMap[mod.id].includes(STORIES_ENTRY)) {
isCsfGlob[mod.name] = true;
}
});

return { idsMap, reasonsMap, isCsfGlob };
}

export function getDependentStoryFiles(changedFiles, stats) {
const { idsMap, reasonsMap, isCsfGlob } = statsToDependencies(stats);
const reverseIdsMap = Object.fromEntries(Object.entries(idsMap).map(([name, id]) => [id, name]));

const checkedIds = {};
const toCheck = [];
const allChangedNames = new Set();
let bailFile; // We need to bail out and check everything
const changedCsfIds = new Set();

function reachName(name) {
// Don't look any further, we've reached the CSF glob.
if (isCsfGlob[name]) {
return;
}

allChangedNames.add(name);
if (name.startsWith(STORYBOOK_DIR) && name !== STORIES_ENTRY) {
bailFile = name;
}

const id = idsMap[name];
if (!id) {
return;
// throw new Error(`Didn't find module ${name}`);
}

if (checkedIds[id]) {
return;
}

// Schedule this module to be checked
toCheck.push(id);

const isCsf = !!reasonsMap[id].find((reasonName) => isCsfGlob[reasonName]);
if (isCsf) {
changedCsfIds.add(id);
}
}

changedFiles.map(reachName);

while (toCheck.length > 0) {
const id = toCheck.pop();

checkedIds[id] = true;
reasonsMap[id].map(reachName);
}

return bailFile ? false : [...changedCsfIds].map((id) => reverseIdsMap[id]);
}
5 changes: 5 additions & 0 deletions bin/lib/getOptions.js
Expand Up @@ -30,6 +30,7 @@ export default async function getOptions({ argv, env, flags, log, packageJson })
projectToken: takeLast(flags.projectToken || flags.appCode) || env.CHROMATIC_PROJECT_TOKEN, // backwards compatibility

only: flags.only,
onlyChanged: flags.onlyChanged === '' ? true : flags.onlyChanged,
list: flags.list,
fromCI,
skip: flags.skip === '' ? true : flags.skip,
Expand Down Expand Up @@ -115,6 +116,10 @@ export default async function getOptions({ argv, env, flags, log, packageJson })
throw new Error(invalidSingularOptions(foundSingularOpts.map((key) => singularOpts[key])));
}

if (options.only && options.onlyChanged) {
throw new Error(invalidSingularOptions(['--only', '--only-changed']));
}

// No need to start or build Storybook if we're going to fetch from a URL
if (storybookUrl) {
noStart = true;
Expand Down
2 changes: 2 additions & 0 deletions bin/lib/parseArgs.js
Expand Up @@ -22,6 +22,7 @@ export default function parseArgs(argv) {
--exit-zero-on-changes [branch] If all snapshots render but there are visual changes, exit with code 0 rather than the usual exit code 1. Only for [branch], if specified. Globs are supported via picomatch.
--ignore-last-build-on-branch <branch> Do not use the last build on this branch as a baseline if it is no longer in history (i.e. branch was rebased). Globs are supported via picomatch.
--only <storypath> Only run a single story or a subset of stories. Story paths typically look like "Path/To/Story". Globs are supported via picomatch. This option implies --preserve-missing.
--only-changed [branch] Only run stories affected by files changed since the baseline build. Only for [branch], if specified. Globs are supported via picomatch.

This comment has been minimized.

Copy link
@zol

zol May 21, 2021

Member

@ghengeveld can we tweak this working to "Only snapshot stories..." please

This comment has been minimized.

Copy link
@zol

zol May 21, 2021

Member

Same with --only

--patch-build <headbranch...basebranch> Create a patch build to fix a missing PR comparison.
--preserve-missing Treat missing stories as unchanged rather than deleted when comparing to the baseline.
--skip [branch] Skip Chromatic tests, but mark the commit as passing. Avoids blocking PRs due to required merge checks. Only for [branch], if specified. Globs are supported via picomatch.
Expand Down Expand Up @@ -53,6 +54,7 @@ export default function parseArgs(argv) {
exitZeroOnChanges: { type: 'string' },
ignoreLastBuildOnBranch: { type: 'string' },
only: { type: 'string' },
onlyChanged: { type: 'string' },
branchName: { type: 'string' },
patchBuild: { type: 'string' },
preserveMissing: { type: 'boolean' },
Expand Down
12 changes: 9 additions & 3 deletions bin/tasks/gitInfo.js
@@ -1,14 +1,14 @@
import picomatch from 'picomatch';

import { getCommitAndBranch } from '../git/getCommitAndBranch';
import { getBaselineCommits, getSlug, getVersion } from '../git/git';
import { getBaselineCommits, getChangedFiles, getSlug, getVersion } from '../git/git';
import { createTask, transitionTo } from '../lib/tasks';
import {
initial,
pending,
skipFailed,
skippedForCommit,
skippingBuild,
skippedForCommit,
success,
} from '../ui/tasks/gitInfo';

Expand Down Expand Up @@ -47,7 +47,13 @@ export const setGitInfo = async (ctx, task) => {
ignoreLastBuildOnBranch: matchesBranch(ctx.options.ignoreLastBuildOnBranch),
});
ctx.git.baselineCommits = baselineCommits;
ctx.log.debug(`Found baselineCommits: ${baselineCommits}`);
ctx.log.debug(`Found baselineCommits: ${baselineCommits.join(', ')}`);

if (baselineCommits.length && matchesBranch(ctx.options.onlyChanged)) {
const results = await Promise.all(baselineCommits.map((c) => getChangedFiles(c)));
ctx.git.changedFiles = [...new Set(results.flat())].map((f) => `./${f}`);
ctx.log.debug(`Found changedFiles:\n${ctx.git.changedFiles.map((f) => ` ${f}`).join('\n')}`);
}

return transitionTo(success, true)(ctx, task);
};
Expand Down
13 changes: 10 additions & 3 deletions bin/tasks/upload.js
Expand Up @@ -63,9 +63,15 @@ function getOutputDir(buildLog) {

function getFileInfo(sourceDir) {
const lengths = getPathsInDir(sourceDir).map((o) => ({ ...o, knownAs: slash(o.pathname) }));
const paths = lengths.map(({ knownAs }) => knownAs);
const total = lengths.map(({ contentLength }) => contentLength).reduce((a, b) => a + b, 0);
return { lengths, paths, total };
const paths = [];
let statsPath;
// eslint-disable-next-line no-restricted-syntax
for (const { knownAs } of lengths) {
if (knownAs.endsWith('preview-stats.json')) statsPath = knownAs;
else paths.push(knownAs);
}
return { lengths, paths, statsPath, total };
}

const isValidStorybook = ({ paths, total }) =>
Expand Down Expand Up @@ -94,7 +100,7 @@ export const uploadStorybook = async (ctx, task) => {

task.output = preparing(ctx).output;

const { lengths, paths, total } = fileInfo;
const { lengths, paths, statsPath, total } = fileInfo;
const { getUploadUrls } = await ctx.client.runQuery(TesterGetUploadUrlsMutation, { paths });
const { domain, urls } = getUploadUrls;
const files = urls.map(({ path, url, contentType }) => ({
Expand All @@ -121,6 +127,7 @@ export const uploadStorybook = async (ctx, task) => {
}

ctx.uploadedBytes = total;
ctx.statsPath = join(ctx.sourceDir, statsPath);
ctx.isolatorUrl = new URL('/iframe.html', domain).toString();
};

Expand Down
36 changes: 26 additions & 10 deletions bin/tasks/verify.js
@@ -1,10 +1,13 @@
import { readJson } from 'fs-extra';

import { createTask, transitionTo } from '../lib/tasks';
import { getDependentStoryFiles } from '../lib/getDependentStoryFiles';
import listingStories from '../ui/messages/info/listingStories';
import storybookPublished from '../ui/messages/info/storybookPublished';
import buildLimited from '../ui/messages/warnings/buildLimited';
import paymentRequired from '../ui/messages/warnings/paymentRequired';
import snapshotQuotaReached from '../ui/messages/warnings/snapshotQuotaReached';
import { initial, pending, runOnly, success } from '../ui/tasks/verify';
import { initial, pending, runOnly, runOnlyFiles, tracing, success } from '../ui/tasks/verify';

const TesterCreateBuildMutation = `
mutation TesterCreateBuildMutation($input: CreateBuildInput!, $isolatorUrl: String!) {
Expand Down Expand Up @@ -66,27 +69,40 @@ export const setEnvironment = async (ctx) => {
ctx.log.debug(`Got environment ${ctx.environment}`);
};

export const traceChangedFiles = async (ctx, task) => {
const { statsPath } = ctx;
const { changedFiles } = ctx.git;
if (!statsPath || !changedFiles) return;

transitionTo(tracing)(ctx, task);

const stats = await readJson(statsPath);
ctx.onlyStoryFiles = getDependentStoryFiles(changedFiles, stats);
ctx.log.debug(`Testing only story files:\n${ctx.onlyStoryFiles.map((f) => ` ${f}`).join('\n')}`);
};

export const createBuild = async (ctx, task) => {
const { client, environment, git, log, pkg, cachedUrl, isolatorUrl, options } = ctx;
const { client, git, log, isolatorUrl, options, onlyStoryFiles } = ctx;
const { list, only, patchBaseRef, patchHeadRef, preserveMissingSpecs } = options;
const { version, matchesBranch, ...commitInfo } = git; // omit some fields
const { version, matchesBranch, changedFiles, ...commitInfo } = git; // omit some fields
const autoAcceptChanges = matchesBranch(options.autoAcceptChanges);

if (only) {
transitionTo(runOnly)(ctx, task);
}
// It's not possible to set both --only and --only-changed
if (only) transitionTo(runOnly)(ctx, task);
else if (onlyStoryFiles) transitionTo(runOnlyFiles)(ctx, task);

const { createBuild: build } = await client.runQuery(TesterCreateBuildMutation, {
input: {
...commitInfo,
...(only && { only }),
...(onlyStoryFiles && { onlyStoryFiles }),
autoAcceptChanges,
cachedUrl,
environment,
cachedUrl: ctx.cachedUrl,
environment: ctx.environment,
patchBaseRef,
patchHeadRef,
preserveMissingSpecs,
packageVersion: pkg.version,
packageVersion: ctx.pkg.version,
storybookVersion: ctx.storybook.version,
viewLayer: ctx.storybook.viewLayer,
addons: ctx.storybook.addons,
Expand Down Expand Up @@ -129,5 +145,5 @@ export const createBuild = async (ctx, task) => {
export default createTask({
title: initial.title,
skip: (ctx) => ctx.skip,
steps: [transitionTo(pending), setEnvironment, createBuild],
steps: [transitionTo(pending), setEnvironment, traceChangedFiles, createBuild],
});
13 changes: 12 additions & 1 deletion bin/ui/tasks/verify.js
Expand Up @@ -9,9 +9,20 @@ export const pending = (ctx) => ({
output: 'This may take a few minutes',
});

export const tracing = (ctx) => ({
status: 'pending',
title: 'Retrieving story files affected by recent changes',
output: `Traversing dependencies for ${ctx.git.changedFiles.length} files that changed since the last build`,
});

export const runOnly = (ctx) => ({
status: 'pending',
title: `Running only stories matching '${ctx.options.only}'`,
title: `Tests will be limited to stories matching '${ctx.options.only}'`,
});

export const runOnlyFiles = (ctx) => ({
status: 'pending',
title: `Tests will be limited to ${ctx.onlyStoryFiles.length} story files affected by recent changes`,
});

export const success = (ctx) => ({
Expand Down
6 changes: 5 additions & 1 deletion bin/ui/tasks/verify.stories.js
@@ -1,5 +1,5 @@
import task from '../components/task';
import { failed, initial, pending, runOnly, success } from './verify';
import { failed, initial, pending, tracing, runOnly, runOnlyFiles, success } from './verify';

export default {
title: 'CLI/Tasks/Verify',
Expand All @@ -16,8 +16,12 @@ export const Initial = () => initial;

export const Pending = () => pending();

export const Tracing = () => tracing({ git: { changedFiles: new Array(3) } });

export const RunOnly = () => runOnly({ options: { only: 'MyComponent/MyStory' } });

export const RunOnlyFiles = () => runOnlyFiles({ onlyStoryFiles: new Array(12) });

export const Started = () => success({ build });

export const Published = () => success({ isPublishOnly: true, build });
Expand Down
11 changes: 6 additions & 5 deletions package.json
Expand Up @@ -30,11 +30,11 @@
"scripts": {
"prebuild": "rm -rf ./dist",
"build": "npm-run-all --serial -l build:**",
"build-storybook": "build-storybook -s static",
"build-storybook": "build-storybook -s static --webpack-stats-json storybook-static",
"build:action": "tsc",
"build:bin": "cross-env BABEL_ENV=build babel -s -d ./dist ./src -D",
"chromatic": "node ./bin/register.js",
"chromatic-prebuild": "node ./bin/register.js --storybook-build-dir=\"storybook-static\"",
"chromatic-prebuilt": "node ./bin/register.js --storybook-build-dir=\"storybook-static\"",
"chromatic-staging": "CHROMATIC_INDEX_URL=https://www.staging-chromatic.com node ./bin/register.js",
"chromatic-verbose": "cross-env LOG_LEVEL=verbose node ./bin/register.js",
"dev": "npm-run-all --parallel -l 'build:** -- --watch'",
Expand Down Expand Up @@ -102,15 +102,16 @@
"devDependencies": {
"@babel/cli": "^7.12.13",
"@babel/core": "^7.12.13",
"@babel/plugin-transform-runtime": "^7.12.15",
"@babel/preset-env": "7.12.13",
"@babel/plugin-transform-runtime": "^7.12.13",
"@babel/preset-env": "^7.12.13",
"@storybook/eslint-config-storybook": "^3.0.0",
"@storybook/linter-config": "^3.0.0",
"@storybook/react": "6.1.17",
"@storybook/react": "^6.2.0-beta.13",
"@types/node": "^14.14.25",
"@typescript-eslint/eslint-plugin": "^4.15.0",
"@typescript-eslint/parser": "^4.15.0",
"ansi-html": "0.0.7",
"babel-preset-minify": "^0.5.1",
"cpy": "^8.1.1",
"cross-env": "^7.0.3",
"eslint": "^7.19.0",
Expand Down

0 comments on commit eefd42a

Please sign in to comment.