Skip to content

Commit

Permalink
chore: fix unit tests and review issues
Browse files Browse the repository at this point in the history
  • Loading branch information
shadowusr committed Sep 6, 2023
1 parent 880bd4b commit e10d2fe
Show file tree
Hide file tree
Showing 23 changed files with 30,003 additions and 24,274 deletions.
10 changes: 4 additions & 6 deletions hermione.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +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 {HermioneTestAdapter} = require('./lib/test-adapter');
const {TestStatus} = require('./lib/constants');
const {FAIL, SUCCESS} = require('./lib/constants');
const {hasDiff} = require('./lib/common-utils');
const {formatTestResult} = require('./lib/server-utils');

let workers;

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

const failHandler = async (testResult) => {
const formattedResult = new HermioneTestAdapter(
testResult, {status: TestStatus.FAIL, imagesInfoFormatter: reportBuilder.imageHandler});
const formattedResult = formatTestResult(testResult, FAIL, reportBuilder);
const actions = [imageHandler.saveTestImages(formattedResult, workers)];

if (formattedResult.errorDetails) {
Expand All @@ -60,8 +59,7 @@ async function prepare(hermione, reportBuilder, pluginConfig) {

hermione.on(hermione.events.TEST_PASS, testResult => {
promises.push(queue.add(async () => {
const formattedResult = new HermioneTestAdapter(
testResult, {status: TestStatus.SUCCESS, imagesInfoFormatter: reportBuilder.imageHandler});
const formattedResult = formatTestResult(testResult, SUCCESS, reportBuilder);
await imageHandler.saveTestImages(formattedResult, workers);

return reportBuilder.addSuccess(formattedResult);
Expand Down
8 changes: 5 additions & 3 deletions lib/common-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,13 @@ export const getAbsoluteUrl = (url: string | undefined, baseUrl: string | undefi
}
};

export const getRelativeUrl = (absoluteUrl: string): string | null => {
export const getRelativeUrl = (absoluteUrl: string): string => {
try {
return new URL(absoluteUrl).pathname;
const urlObj = new URL(absoluteUrl);

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

Expand Down
14 changes: 13 additions & 1 deletion lib/db-utils/server.ts
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);
};
13 changes: 6 additions & 7 deletions lib/gui/tool-runner/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const {DATABASE_URLS_JSON_NAME, LOCAL_DATABASE_NAME} = require('../../constants/
const {getShortMD5} = require('../../common-utils');
const {formatId, mkFullTitle, mergeDatabasesForReuse, filterByEqualDiffSizes} = require('./utils');
const {getTestsTreeFromDatabase} = require('../../db-utils/server');
const {HermioneTestAdapter} = require('../../test-adapter');
const {formatTestResult} = require('../../server-utils');

module.exports = class ToolRunner {
static create(paths, hermione, configs) {
Expand Down Expand Up @@ -99,8 +99,7 @@ module.exports = class ToolRunner {
updateReferenceImage(tests) {
return Promise.map(tests, (test) => {
const updateResult = this._prepareTestResult(test);
const adapterOptions = {status: UPDATED, imagesInfoFormatter: this._reportBuilder.imageHandler};
const formattedResult = new HermioneTestAdapter(updateResult, adapterOptions);
const formattedResult = formatTestResult(updateResult, UPDATED, this._reportBuilder);
const failResultId = formattedResult.id;
const updateAttempt = this._reportBuilder.getUpdatedAttempt(formattedResult);

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

await Promise.map(tests, async (test) => {
const updateResult = this._prepareTestResult(test);
const formattedResult = new HermioneTestAdapter(updateResult, {status: UPDATED, imagesInfoFormatter: this._reportBuilder.imageHandler});
const formattedResult = formatTestResult(updateResult, UPDATED, this._reportBuilder);

formattedResult.attempt = updateResult.attempt;

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

test.pending
? this._reportBuilder.addSkipped(new HermioneTestAdapter(test, {status: SKIPPED, imagesInfoFormatter: this._reportBuilder.imageHandler}))
: this._reportBuilder.addIdle(new HermioneTestAdapter(test, {status: IDLE, imagesInfoFormatter: this._reportBuilder.imageHandler}));
? this._reportBuilder.addSkipped(formatTestResult(test, SKIPPED, this._reportBuilder))
: this._reportBuilder.addIdle(formatTestResult(test, IDLE, this._reportBuilder));
});

await this._fillTestsTree();
Expand Down
16 changes: 7 additions & 9 deletions lib/gui/tool-runner/report-subscriber.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,15 @@ const PQueue = require('p-queue');
const clientEvents = require('../constants/client-events');
const {getSuitePath} = require('../../plugin-utils');
const {createWorkers} = require('../../workers/create-workers');
const {logError} = require('../../server-utils');
const {HermioneTestAdapter} = require('../../test-adapter');
const {logError, formatTestResult} = require('../../server-utils');
const {hasDiff} = require('../../common-utils');
const {TestStatus} = require('../../constants');
const {TestStatus, RUNNING, SUCCESS, SKIPPED} = require('../../constants');

let workers;

module.exports = (hermione, reportBuilder, client, reportPath) => {
const queue = new PQueue({concurrency: os.cpus().length});
const {imageHandler} = reportBuilder;
const imagesInfoFormatter = imageHandler;

function failHandler(testResult, formattedResult) {
const actions = [imageHandler.saveTestImages(formattedResult, workers)];
Expand Down Expand Up @@ -43,7 +41,7 @@ module.exports = (hermione, reportBuilder, client, reportPath) => {
});

hermione.on(hermione.events.TEST_BEGIN, (data) => {
const formattedResult = new HermioneTestAdapter(data, {status: TestStatus.RUNNING, imagesInfoFormatter});
const formattedResult = formatTestResult(data, RUNNING, reportBuilder);
formattedResult.attempt = reportBuilder.getCurrAttempt(formattedResult);

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

hermione.on(hermione.events.TEST_PASS, (testResult) => {
queue.add(async () => {
const formattedResult = new HermioneTestAdapter(testResult, {status: TestStatus.SUCCESS, imagesInfoFormatter});
const formattedResult = formatTestResult(testResult, SUCCESS, reportBuilder);
formattedResult.attempt = reportBuilder.getCurrAttempt(formattedResult);

await imageHandler.saveTestImages(formattedResult, workers);
Expand All @@ -68,7 +66,7 @@ module.exports = (hermione, reportBuilder, client, reportPath) => {
hermione.on(hermione.events.RETRY, (testResult) => {
queue.add(async () => {
const status = hasDiff(testResult.assertViewResults) ? TestStatus.FAIL : TestStatus.ERROR;
const formattedResult = new HermioneTestAdapter(testResult, {status, imagesInfoFormatter});
const formattedResult = formatTestResult(testResult, status, reportBuilder);
formattedResult.attempt = reportBuilder.getCurrAttempt(formattedResult);

await failHandler(testResult, formattedResult);
Expand All @@ -82,7 +80,7 @@ module.exports = (hermione, reportBuilder, client, reportPath) => {
hermione.on(hermione.events.TEST_FAIL, (testResult) => {
queue.add(async () => {
const status = hasDiff(testResult.assertViewResults) ? TestStatus.FAIL : TestStatus.ERROR;
const formattedResult = new HermioneTestAdapter(testResult, {status, imagesInfoFormatter});
const formattedResult = formatTestResult(testResult, status, reportBuilder);
formattedResult.attempt = reportBuilder.getCurrAttempt(formattedResult);

await failHandler(testResult, formattedResult);
Expand All @@ -97,7 +95,7 @@ module.exports = (hermione, reportBuilder, client, reportPath) => {

hermione.on(hermione.events.TEST_PENDING, async (testResult) => {
queue.add(async () => {
const formattedResult = new HermioneTestAdapter(testResult, {status: TestStatus.SKIPPED, imagesInfoFormatter});
const formattedResult = formatTestResult(testResult, SKIPPED, reportBuilder);
formattedResult.attempt = reportBuilder.getCurrAttempt(formattedResult);

await failHandler(testResult, formattedResult);
Expand Down
21 changes: 10 additions & 11 deletions lib/image-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,7 @@ export class ImageHandler extends EventEmitter2 implements ImagesInfoFormatter {
return testResult.error?.screenshot;
}

getImagesFor(testResult: ReporterTestResultPlain, stateName?: string): ImageInfo | undefined {
const {status} = testResult;
getImagesFor(testResult: ReporterTestResultPlain, assertViewStatus: TestStatus, stateName?: string): ImageInfo | undefined {
const refImg = ImageHandler.getRefImg(testResult.assertViewResults, stateName);
const currImg = ImageHandler.getCurrImg(testResult.assertViewResults, stateName);
const errImg = ImageHandler.getScreenshot(testResult);
Expand All @@ -67,7 +66,7 @@ export class ImageHandler extends EventEmitter2 implements ImagesInfoFormatter {
const currPath = utils.getCurrentPath({attempt: testResult.attempt, browserId: testResult.browserId, imageDir: testResult.imageDir, stateName});
const diffPath = utils.getDiffPath({attempt: testResult.attempt, browserId: testResult.browserId, imageDir: testResult.imageDir, stateName});

if ((status === SUCCESS || status === UPDATED) && refImg) {
if ((assertViewStatus === SUCCESS || assertViewStatus === UPDATED) && refImg) {
const result: ImageInfo = {
expectedImg: {path: this._getImgFromStorage(refPath), size: refImg.size}
};
Expand All @@ -78,7 +77,7 @@ export class ImageHandler extends EventEmitter2 implements ImagesInfoFormatter {
return result;
}

if (status === FAIL && refImg && currImg) {
if (assertViewStatus === FAIL && refImg && currImg) {
return {
expectedImg: {
path: this._getImgFromStorage(refPath),
Expand All @@ -98,10 +97,10 @@ export class ImageHandler extends EventEmitter2 implements ImagesInfoFormatter {
};
}

if (status === ERROR && errImg) {
if (assertViewStatus === ERROR && errImg) {
return {
actualImg: {
path: testResult.state.name ? this._getImgFromStorage(currPath) : '',
path: testResult.state?.name ? this._getImgFromStorage(currPath) : '',
size: currImg?.size || errImg.size
}
};
Expand All @@ -112,33 +111,33 @@ export class ImageHandler extends EventEmitter2 implements ImagesInfoFormatter {

getImagesInfo(testResult: ReporterTestResultPlain): ImageInfoFull[] {
const imagesInfo: ImageInfoFull[] = testResult.assertViewResults?.map((assertResult): ImageInfoFull => {
let status: TestStatus | undefined, error: {message: string; stack: string;} | undefined;
let status: TestStatus, error: {message: string; stack: string;} | undefined;

if (testResult.isUpdated === true) {
status = UPDATED;
} else if (!(assertResult instanceof Error)) {
status = SUCCESS;
} else if (isImageDiffError(assertResult)) {
status = FAIL;
} else if (isNoRefImageError(assertResult)) {
status = ERROR;
error = _.pick(assertResult, ['message', 'stack']);
} else {
status = SUCCESS;
}

const {stateName, refImg} = assertResult;
const diffClusters = (assertResult as ImageDiffError).diffClusters;

return _.extend(
{stateName, refImg, status: status, error, diffClusters},
this.getImagesFor(testResult, stateName)
this.getImagesFor(testResult, status, stateName)
) as ImageInfoFull;
}) ?? [];

// common screenshot on test fail
if (ImageHandler.getScreenshot(testResult)) {
const errorImage = _.extend(
{status: ERROR, error: getError(testResult.error)},
this.getImagesFor(testResult)
this.getImagesFor(testResult, ERROR)
) as ImageInfoError;

imagesInfo.push(errorImage);
Expand Down
6 changes: 5 additions & 1 deletion lib/plugin-utils.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import {HermioneSuite} from './types';

export const getSuitePath = (suite: HermioneSuite): string[] => {
export const getSuitePath = (suite?: HermioneSuite): string[] => {
if (!suite) {
return [];
}

return (suite as HermioneSuite).root ?
[] :
([] as string[]).concat(getSuitePath(suite.parent as HermioneSuite)).concat(suite.title);
Expand Down
17 changes: 6 additions & 11 deletions lib/report-builder/static.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
SUCCESS,
TestStatus,
LOCAL_DATABASE_NAME,
PluginEvents, DB_COLUMNS
PluginEvents
} from '../constants';
import {PreparedTestResult, SqliteAdapter} from '../sqlite-adapter';
import {ReporterTestResult} from '../test-adapter';
Expand All @@ -23,6 +23,7 @@ import {HtmlReporter} from '../plugin-api';
import {ImageHandler} from '../image-handler';
import {SqliteImageStore} from '../image-store';
import {getAbsoluteUrl, getError, getRelativeUrl, hasDiff} from '../common-utils';
import {getTestFromDb} from '../db-utils/server';

const ignoredStatuses = [RUNNING, IDLE];

Expand Down Expand Up @@ -129,13 +130,7 @@ export class StaticReportBuilder {
}

// To prevent skips duplication on reporter startup
const isPreviouslySkippedTest = testResult.status === SKIPPED && this._sqliteAdapter.query({
select: '*',
where: `${DB_COLUMNS.SUITE_PATH} = ? AND ${DB_COLUMNS.NAME} = ? AND ${DB_COLUMNS.STATUS} = ?`,
orderBy: DB_COLUMNS.TIMESTAMP,
orderDescending: true,
limit: 1
}, JSON.stringify(formattedResult.testPath), formattedResult.browserId, TestStatus.SKIPPED);
const isPreviouslySkippedTest = testResult.status === SKIPPED && getTestFromDb(this._sqliteAdapter, formattedResult);

if (!ignoredStatuses.includes(testResult.status) && !isPreviouslySkippedTest) {
this._writeTestResultToDb(testResult, formattedResult);
Expand All @@ -144,7 +139,7 @@ export class StaticReportBuilder {
return formattedResult;
}

_createTestResult(result: ReporterTestResult, props: {status: TestStatus} & Partial<PreparedTestResult>): PreparedTestResult {
protected _createTestResult(result: ReporterTestResult, props: {status: TestStatus} & Partial<PreparedTestResult>): PreparedTestResult {
const {
browserId, file, sessionId, description, history,
imagesInfo = [], screenshot, multipleTabs, errorDetails
Expand All @@ -166,14 +161,14 @@ export class StaticReportBuilder {
return testResult;
}

_writeTestResultToDb(testResult: PreparedTestResult, formattedResult: ReporterTestResult): void {
protected _writeTestResultToDb(testResult: PreparedTestResult, formattedResult: ReporterTestResult): void {
const suiteName = formattedResult.state.name;
const suitePath = formattedResult.testPath;

this._sqliteAdapter.write({testResult, suitePath, suiteName});
}

_deleteTestResultFromDb(...args: Parameters<typeof this._sqliteAdapter.delete>): void {
protected _deleteTestResultFromDb(...args: Parameters<typeof this._sqliteAdapter.delete>): void {
this._sqliteAdapter.delete(...args);
}

Expand Down
9 changes: 7 additions & 2 deletions lib/server-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,11 @@ import {logger} from './common-utils';
import {UPDATED, RUNNING, IDLE, SKIPPED, IMAGES_PATH, TestStatus} from './constants';
import type {HtmlReporter} from './plugin-api';
import type {ReporterTestResult} from './test-adapter';
import {CustomGuiItem, ReporterConfig} from './types';
import {CustomGuiItem, HermioneTestResult, ReporterConfig} from './types';
import type Hermione from 'hermione';
import crypto from 'crypto';
import {ImageHandler} from './image-handler';
import {ImageHandler, ImagesInfoFormatter} from './image-handler';
import {HermioneTestAdapter} from './test-adapter';

const DATA_FILE_NAME = 'data.js';

Expand Down Expand Up @@ -296,3 +297,7 @@ export function mapPlugins<T>(plugins: ReporterConfig['plugins'], callback: (nam
forEachPlugin(plugins, pluginName => result.push(callback(pluginName)));
return result;
}

export const formatTestResult = (rawResult: HermioneTestResult, status: TestStatus, {imageHandler}: {imageHandler: ImagesInfoFormatter}): ReporterTestResult => {
return new HermioneTestAdapter(rawResult, {status, imagesInfoFormatter: imageHandler});
};
1 change: 1 addition & 0 deletions lib/test-adapter/cache/hermione.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const testsAttempts: Map<string, number> = new Map();
3 changes: 1 addition & 2 deletions lib/test-adapter/hermione.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@ import {
import {ImagesInfoFormatter} from '../image-handler';
import {ReporterTestResult} from './index';
import {getSuitePath} from '../plugin-utils';

const testsAttempts: Map<string, number> = new Map();
import {testsAttempts} from './cache/hermione';

const getSkipComment = (suite: HermioneTestResult | HermioneSuite): string | null | undefined => {
return suite.skipReason || suite.parent && getSkipComment(suite.parent);
Expand Down
2 changes: 1 addition & 1 deletion lib/test-adapter/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,6 @@ export interface ReporterTestResult {
readonly state: { name: string };
readonly status: TestStatus;
readonly testPath: string[];
timestamp: number | undefined;
readonly timestamp: number | undefined;
readonly url?: string;
}
Loading

0 comments on commit e10d2fe

Please sign in to comment.