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.

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

treat metadata file differently locally from CI #103

Merged
merged 1 commit into from
Jul 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 0 additions & 16 deletions .vdiff.json
Copy link
Member Author

Choose a reason for hiding this comment

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

Deleting this for now. The first time the action runs, it'll re-create it!

Copy link
Contributor

Choose a reason for hiding this comment

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

I like that, let's us verify it works as expected!

This file was deleted.

6 changes: 4 additions & 2 deletions src/server/visual-diff-plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,15 @@ import { PNG } from 'pngjs';
const isCI = !!env['CI'];
const DEFAULT_MARGIN = 10;
const DEFAULT_TOLERANCE = 0; // TODO: Support tolerance override?
const METADATA_NAME = '.vdiff.json';
const ROOT_NAME = '.vdiff';
export const PATHS = {
FAIL: 'fail',
GOLDEN: 'golden',
PASS: 'pass',
METADATA: '.vdiff.json',
METADATA: isCI ? METADATA_NAME : join(ROOT_NAME, METADATA_NAME),
REPORT_ROOT: '.report',
VDIFF_ROOT: '.vdiff'
VDIFF_ROOT: ROOT_NAME
};

async function checkFileExists(fileName) {
Expand Down
22 changes: 13 additions & 9 deletions src/server/visual-diff-reporter.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import { cpSync, existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs';
import { dirname, join } from 'node:path';
import { getTestInfo, PATHS } from './visual-diff-plugin.js';
import { env } from 'node:process';
import { execSync } from 'node:child_process';
import { fileURLToPath } from 'node:url';

const __dirname = dirname(fileURLToPath(import.meta.url));
const isCI = !!env['CI'];

function createData(rootDir, sessions) {
function createData(rootDir, updateGoldens, sessions) {

let metadata = {};
const metadataPath = join(rootDir, PATHS.METADATA);
Expand Down Expand Up @@ -51,10 +53,12 @@ function createData(rootDir, sessions) {
});
});

metadata.browsers = Array.from(browsers.values()).map(b => {
return { name: b.name, version: b.version };
});
writeFileSync(metadataPath, JSON.stringify(metadata, undefined, '\t'));
if (isCI || updateGoldens) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume we're ok with the browser versions being slightly off if you're using --filter or --grep with --golden, meaning any old goldens you don't update could've been created by an older browser. There isn't really a way around it without storing the browser info with every png like we'd originally thought about. And it's only an issue locally, not in CI

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, but yeah not worried about it. This stuff is really only useful in CI anyway -- if you're reusing old goldens locally from an old browser... I'm not sure what you're doing. I've almost talked myself into only using this information in CI, period, haha.

metadata.browsers = Array.from(browsers.values()).map(b => {
return { name: b.name, version: b.version };
});
writeFileSync(metadataPath, JSON.stringify(metadata, undefined, '\t'));
}

return { browsers, files, numFailed, numTests };

Expand Down Expand Up @@ -99,7 +103,7 @@ function flattenResults(session, browserData, fileData) {

}

export function visualDiffReporter({ reportResults = true } = {}) {
export function visualDiffReporter({ updateGoldens } = {}) {
let rootDir;
return {
start({ config }) {
Expand All @@ -108,14 +112,14 @@ export function visualDiffReporter({ reportResults = true } = {}) {
},
stop({ sessions }) {

if (!reportResults) return;

const data = createData(rootDir, sessions);
const data = createData(rootDir, updateGoldens, sessions);
const json = JSON.stringify(data, (_key, val) => {
if (val instanceof Map) return [...val.values()].sort((a, b) => a.name.localeCompare(b.name));
return val;
}, '\t');

if (updateGoldens) return;

const inputDir = join(__dirname, 'report');
const reportDir = join(rootDir, PATHS.VDIFF_ROOT, PATHS.REPORT_ROOT);
const tempDir = join(reportDir, 'temp');
Expand Down
2 changes: 1 addition & 1 deletion src/server/wtr-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ export class WTRConfig {
});
} else if (group === 'vdiff') {
config.reporters ??= [ defaultReporter() ];
config.reporters.push(visualDiffReporter({ reportResults: !golden }));
config.reporters.push(visualDiffReporter({ updateGoldens: golden }));

config.plugins ??= [];
config.plugins.push(visualDiff({ updateGoldens: golden, runSubset: !!(filter || grep) }));
Expand Down