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

refactor: introduce adapter pattern for test results #493

Merged
merged 3 commits into from
Sep 6, 2023
Merged

refactor: introduce adapter pattern for test results #493

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
7a66764
chore: rename test-adapter to hermione
shadowusr Sep 1, 2023
950b809
refactor: introduce adapter pattern and improve its interface
shadowusr Sep 5, 2023
a679d08
chore: fix unit tests and review issues
shadowusr Sep 6, 2023
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
13 changes: 8 additions & 5 deletions hermione.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ const os = require('os');
const PQueue = require('p-queue');
const {PluginAdapter} = require('./lib/plugin-adapter');
const {createWorkers} = require('./lib/workers/create-workers');
const {FAIL, SUCCESS} = require('./lib/constants');
const {hasDiff} = require('./lib/common-utils');
const {formatTestResult} = require('./lib/server-utils');

let workers;

Expand Down Expand Up @@ -32,8 +35,8 @@ async function prepare(hermione, reportBuilder, pluginConfig) {
const {imageHandler} = reportBuilder;

const failHandler = async (testResult) => {
const formattedResult = reportBuilder.format(testResult);
const actions = [imageHandler.saveTestImages(testResult, formattedResult.attempt, workers)];
const formattedResult = formatTestResult(testResult, FAIL, reportBuilder);
const actions = [imageHandler.saveTestImages(formattedResult, workers)];

if (formattedResult.errorDetails) {
actions.push(formattedResult.saveErrorDetails(reportPath));
Expand All @@ -45,7 +48,7 @@ async function prepare(hermione, reportBuilder, pluginConfig) {
};

const addFail = (formattedResult) => {
return formattedResult.hasDiff()
return hasDiff(formattedResult.assertViewResults)
? reportBuilder.addFail(formattedResult)
: reportBuilder.addError(formattedResult);
};
Expand All @@ -56,8 +59,8 @@ async function prepare(hermione, reportBuilder, pluginConfig) {

hermione.on(hermione.events.TEST_PASS, testResult => {
promises.push(queue.add(async () => {
const formattedResult = reportBuilder.format(testResult);
await imageHandler.saveTestImages(testResult, formattedResult.attempt, workers);
const formattedResult = formatTestResult(testResult, SUCCESS, reportBuilder);
await imageHandler.saveTestImages(formattedResult, workers);

return reportBuilder.addSuccess(formattedResult);
}).catch(reject));
Expand Down
42 changes: 35 additions & 7 deletions lib/common-utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import crypto from 'crypto';
import {pick} from 'lodash';
import {pick, isEmpty} from 'lodash';
import url from 'url';
import axios, {AxiosRequestConfig} from 'axios';
import {SUCCESS, FAIL, ERROR, SKIPPED, UPDATED, IDLE, RUNNING, QUEUED, TestStatus} from './constants';

import {UNCHECKED, INDETERMINATE, CHECKED} from './constants/checked-statuses';
import {AssertViewResult, TestResult} from './types';
import {AssertViewResult, TestError} from './types';
import {ErrorName, ImageDiffError, NoRefImageError} from './errors';
export const getShortMD5 = (str: string): string => {
return crypto.createHash('md5').update(str, 'ascii').digest('hex').substr(0, 7);
Expand Down Expand Up @@ -46,24 +46,52 @@ export const determineStatus = (statuses: TestStatus[]): TestStatus | null => {
return null;
};

export const getAbsoluteUrl = (url: string | undefined, baseUrl: string | undefined): string => {
try {
return new URL(url ?? '', isEmpty(baseUrl) ? undefined : baseUrl).href;
} catch {
return baseUrl || url || '';
}
};

export const getRelativeUrl = (absoluteUrl: string): string => {
try {
const urlObj = new URL(absoluteUrl);

return urlObj.pathname + urlObj.search;
} catch {
return absoluteUrl;
}
};

export const wrapLinkByTag = (text: string): string => {
return text.replace(/https?:\/\/[^\s]*/g, (url) => {
return `<a target="_blank" href="${url}">${url}</a>`;
});
};

export const mkTestId = (fullTitle: string, browserId: string): string => {
return fullTitle + '.' + browserId;
};

export const isImageDiffError = (assertResult: AssertViewResult): assertResult is ImageDiffError => {
return assertResult.name === ErrorName.IMAGE_DIFF;
return (assertResult as ImageDiffError).name === ErrorName.IMAGE_DIFF;
};

export const isNoRefImageError = (assertResult: AssertViewResult): assertResult is NoRefImageError => {
return assertResult.name === ErrorName.NO_REF_IMAGE;
return (assertResult as NoRefImageError).name === ErrorName.NO_REF_IMAGE;
};

export const getError = (testResult: TestResult): undefined | {message?: string; stack?: string; stateName?: string} => {
if (!testResult.err) {
export const getError = (error?: TestError): undefined | Pick<TestError, 'message' | 'stack' | 'stateName'> => {
if (!error) {
return undefined;
}

return pick(testResult.err, ['message', 'stack', 'stateName']);
return pick(error, ['message', 'stack', 'stateName']);
};

export const hasDiff = (assertViewResults: AssertViewResult[]): boolean => {
return assertViewResults.some((result) => isImageDiffError(result));
};

export const isUrl = (str: string): boolean => {
Expand Down
14 changes: 13 additions & 1 deletion lib/db-utils/server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@ import NestedError from 'nested-error-stacks';
import {StaticTestsTreeBuilder} from '../tests-tree-builder/static';
import * as commonSqliteUtils from './common';
import {isUrl, fetchFile, normalizeUrls, logger} from '../common-utils';
import {DATABASE_URLS_JSON_NAME, LOCAL_DATABASE_NAME} from '../constants';
import {DATABASE_URLS_JSON_NAME, DB_COLUMNS, LOCAL_DATABASE_NAME, TestStatus} from '../constants';
import {DbLoadResult, HandleDatabasesOptions} from './common';
import {DbUrlsJsonData, RawSuitesRow, ReporterConfig} from '../types';
import {Tree} from '../tests-tree-builder/base';
import {ReporterTestResult} from '../test-adapter';
import {SqliteAdapter} from '../sqlite-adapter';

export * from './common';

Expand Down Expand Up @@ -117,3 +119,13 @@ async function rewriteDatabaseUrls(dbPaths: string[], mainDatabaseUrls: string,
jsonUrls: []
});
}

export const getTestFromDb = <T = unknown>(sqliteAdapter: SqliteAdapter, testResult: ReporterTestResult): T | undefined => {
return sqliteAdapter.query<T>({
select: '*',
where: `${DB_COLUMNS.SUITE_PATH} = ? AND ${DB_COLUMNS.NAME} = ? AND ${DB_COLUMNS.STATUS} = ?`,
orderBy: DB_COLUMNS.TIMESTAMP,
orderDescending: true,
limit: 1
}, JSON.stringify(testResult.testPath), testResult.browserId, TestStatus.SKIPPED);
};
15 changes: 9 additions & 6 deletions lib/errors/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
import {CoordBounds} from 'looks-same';
import {DiffOptions, ImageData} from '../types';
import {ValueOf} from 'type-fest';

export enum ErrorName {
IMAGE_DIFF = 'ImageDiffError',
NO_REF_IMAGE = 'NoRefImageError'
}
export const ErrorName = {
IMAGE_DIFF: 'ImageDiffError',
NO_REF_IMAGE: 'NoRefImageError'
} as const;
export type ErrorName = ValueOf<typeof ErrorName>;
export type ErrorNames = typeof ErrorName;

export interface ImageDiffError {
name: ErrorName.IMAGE_DIFF;
name: ErrorNames['IMAGE_DIFF'];
message: string;
stack: string;
stateName: string;
Expand All @@ -19,7 +22,7 @@ export interface ImageDiffError {
}

export interface NoRefImageError {
name: ErrorName.NO_REF_IMAGE;
name: ErrorNames['NO_REF_IMAGE'];
stateName: string;
message: string;
stack: string;
Expand Down
13 changes: 7 additions & 6 deletions lib/gui/tool-runner/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,12 @@ const GuiReportBuilder = require('../../report-builder/gui');
const EventSource = require('../event-source');
const {logger} = require('../../common-utils');
const reporterHelper = require('../../reporter-helpers');
const {UPDATED} = require('../../constants/test-statuses');
const {UPDATED, SKIPPED, IDLE} = require('../../constants/test-statuses');
const {DATABASE_URLS_JSON_NAME, LOCAL_DATABASE_NAME} = require('../../constants/database');
const {getShortMD5} = require('../../common-utils');
const {formatId, mkFullTitle, mergeDatabasesForReuse, filterByEqualDiffSizes} = require('./utils');
const {getTestsTreeFromDatabase} = require('../../db-utils/server');
const {formatTestResult} = require('../../server-utils');

module.exports = class ToolRunner {
static create(paths, hermione, configs) {
Expand Down Expand Up @@ -98,7 +99,7 @@ module.exports = class ToolRunner {
updateReferenceImage(tests) {
return Promise.map(tests, (test) => {
const updateResult = this._prepareTestResult(test);
const formattedResult = this._reportBuilder.format(updateResult, UPDATED);
const formattedResult = formatTestResult(updateResult, UPDATED, this._reportBuilder);
const failResultId = formattedResult.id;
const updateAttempt = this._reportBuilder.getUpdatedAttempt(formattedResult);

Expand All @@ -115,7 +116,7 @@ module.exports = class ToolRunner {
});
})
.then(() => {
this._reportBuilder.addUpdated(updateResult, failResultId);
this._reportBuilder.addUpdated(formatTestResult(updateResult, UPDATED, this._reportBuilder), failResultId);
return this._reportBuilder.getTestBranch(formattedResult.id);
});
});
Expand All @@ -126,7 +127,7 @@ module.exports = class ToolRunner {

await Promise.map(tests, async (test) => {
const updateResult = this._prepareTestResult(test);
const formattedResult = this._reportBuilder.format(updateResult);
const formattedResult = formatTestResult(updateResult, UPDATED, this._reportBuilder);

formattedResult.attempt = updateResult.attempt;

Expand Down Expand Up @@ -215,8 +216,8 @@ module.exports = class ToolRunner {
this._tests[testId] = _.extend(test, {browserId});

test.pending
? this._reportBuilder.addSkipped(test)
: this._reportBuilder.addIdle(test);
? this._reportBuilder.addSkipped(formatTestResult(test, SKIPPED, this._reportBuilder))
: this._reportBuilder.addIdle(formatTestResult(test, IDLE, this._reportBuilder));
});

await this._fillTestsTree();
Expand Down
25 changes: 14 additions & 11 deletions lib/gui/tool-runner/report-subscriber.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@
const os = require('os');
const PQueue = require('p-queue');
const clientEvents = require('../constants/client-events');
const {RUNNING} = require('../../constants/test-statuses');
const {getSuitePath} = require('../../plugin-utils');
const {createWorkers} = require('../../workers/create-workers');
const {logError} = require('../../server-utils');
const {logError, formatTestResult} = require('../../server-utils');
const {hasDiff} = require('../../common-utils');
const {TestStatus, RUNNING, SUCCESS, SKIPPED} = require('../../constants');

let workers;

Expand All @@ -15,7 +16,7 @@ module.exports = (hermione, reportBuilder, client, reportPath) => {
const {imageHandler} = reportBuilder;

function failHandler(testResult, formattedResult) {
const actions = [imageHandler.saveTestImages(testResult, formattedResult.attempt, workers)];
const actions = [imageHandler.saveTestImages(formattedResult, workers)];

if (formattedResult.errorDetails) {
actions.push(formattedResult.saveErrorDetails(reportPath));
Expand All @@ -35,12 +36,12 @@ module.exports = (hermione, reportBuilder, client, reportPath) => {

client.emit(clientEvents.BEGIN_SUITE, {
suiteId: getSuitePath(suite).join(' '),
status: RUNNING
status: TestStatus.RUNNING
});
});

hermione.on(hermione.events.TEST_BEGIN, (data) => {
const formattedResult = reportBuilder.format(data, RUNNING);
const formattedResult = formatTestResult(data, RUNNING, reportBuilder);
formattedResult.attempt = reportBuilder.getCurrAttempt(formattedResult);

reportBuilder.addRunning(formattedResult);
Expand All @@ -51,10 +52,10 @@ module.exports = (hermione, reportBuilder, client, reportPath) => {

hermione.on(hermione.events.TEST_PASS, (testResult) => {
queue.add(async () => {
const formattedResult = reportBuilder.format(testResult, hermione.events.TEST_PASS);
const formattedResult = formatTestResult(testResult, SUCCESS, reportBuilder);
formattedResult.attempt = reportBuilder.getCurrAttempt(formattedResult);

await imageHandler.saveTestImages(testResult, formattedResult.attempt, workers);
await imageHandler.saveTestImages(formattedResult, workers);
reportBuilder.addSuccess(formattedResult);

const testBranch = reportBuilder.getTestBranch(formattedResult.id);
Expand All @@ -64,7 +65,8 @@ module.exports = (hermione, reportBuilder, client, reportPath) => {

hermione.on(hermione.events.RETRY, (testResult) => {
queue.add(async () => {
const formattedResult = reportBuilder.format(testResult, hermione.events.RETRY);
const status = hasDiff(testResult.assertViewResults) ? TestStatus.FAIL : TestStatus.ERROR;
const formattedResult = formatTestResult(testResult, status, reportBuilder);
formattedResult.attempt = reportBuilder.getCurrAttempt(formattedResult);

await failHandler(testResult, formattedResult);
Expand All @@ -77,11 +79,12 @@ module.exports = (hermione, reportBuilder, client, reportPath) => {

hermione.on(hermione.events.TEST_FAIL, (testResult) => {
queue.add(async () => {
const formattedResult = reportBuilder.format(testResult, hermione.events.TEST_FAIL);
const status = hasDiff(testResult.assertViewResults) ? TestStatus.FAIL : TestStatus.ERROR;
Copy link
Member

Choose a reason for hiding this comment

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

я понимаю, что эта логика была и до этого PR-а, но почему мы выставляем fail именно при наличии диффа? чем это отличается от обычного провалившегося ассерта?

//cc @DudaGod

Copy link
Member

Choose a reason for hiding this comment

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

Я сам недавно задавался этим вопросом когда интеграцию pwt с testcop пилил. Сходу не помню почему так было сделано.

const formattedResult = formatTestResult(testResult, status, reportBuilder);
formattedResult.attempt = reportBuilder.getCurrAttempt(formattedResult);

await failHandler(testResult, formattedResult);
formattedResult.hasDiff()
status === TestStatus.FAIL
? reportBuilder.addFail(formattedResult)
: reportBuilder.addError(formattedResult);

Expand All @@ -92,7 +95,7 @@ module.exports = (hermione, reportBuilder, client, reportPath) => {

hermione.on(hermione.events.TEST_PENDING, async (testResult) => {
queue.add(async () => {
const formattedResult = reportBuilder.format(testResult, hermione.events.TEST_PENDING);
const formattedResult = formatTestResult(testResult, SKIPPED, reportBuilder);
formattedResult.attempt = reportBuilder.getCurrAttempt(formattedResult);

await failHandler(testResult, formattedResult);
Expand Down
Loading
Loading