From 232ba8cf857ef2a9a1706c31bbe918fdd06beb5a Mon Sep 17 00:00:00 2001 From: shadowusr Date: Wed, 27 Sep 2023 18:01:54 +0300 Subject: [PATCH] fix: display common page screenshot in pwt report when possible --- lib/common-utils.ts | 2 +- lib/image-handler.ts | 48 +++++++++++++------ lib/image-store.ts | 2 +- lib/static/components/prop-types.ts | 9 ++++ .../section/body/page-screenshot.tsx | 18 +++++++ lib/static/components/section/body/result.jsx | 28 ++++++++--- lib/static/components/section/body/tabs.jsx | 28 ++++++++--- lib/static/components/state/state-error.jsx | 17 ++----- lib/test-adapter/index.ts | 4 +- lib/test-adapter/playwright.ts | 13 +++-- lib/tests-tree-builder/base.ts | 3 +- lib/types.ts | 10 +++- playwright.ts | 9 ++-- .../fixtures/playwright/playwright.config.ts | 4 ++ 14 files changed, 137 insertions(+), 58 deletions(-) create mode 100644 lib/static/components/prop-types.ts create mode 100644 lib/static/components/section/body/page-screenshot.tsx diff --git a/lib/common-utils.ts b/lib/common-utils.ts index 76f2fef9a..c0619d16c 100644 --- a/lib/common-utils.ts +++ b/lib/common-utils.ts @@ -130,7 +130,7 @@ export const hasDiff = (assertViewResults: AssertViewResult[]): boolean => { return assertViewResults.some((result) => isImageDiffError(result as {name?: string})); }; -export const isBase64Image = (image: ImageData | ImageBase64 | undefined): image is ImageBase64 => { +export const isBase64Image = (image: ImageData | ImageBase64 | null | undefined): image is ImageBase64 => { return Boolean((image as ImageBase64 | undefined)?.base64); }; diff --git a/lib/image-handler.ts b/lib/image-handler.ts index ac808ec08..aa988d54a 100644 --- a/lib/image-handler.ts +++ b/lib/image-handler.ts @@ -15,7 +15,7 @@ import { ImageInfoFail, ImageInfoFull, ImagesSaver, - ImageSize + ImageInfoPageSuccess } from './types'; import {ERROR, FAIL, PluginEvents, SUCCESS, TestStatus, UPDATED} from './constants'; import { @@ -66,19 +66,30 @@ export class ImageHandler extends EventEmitter2 implements ImagesInfoFormatter { return _.get(_.find(assertViewResults, {stateName}), 'refImg'); } - static getScreenshot(testResult: ReporterTestResultPlain): ImageBase64 | ImageData | undefined { - return testResult.error?.screenshot; + static getScreenshot(testResult: ReporterTestResultPlain): ImageBase64 | ImageData | null | undefined { + return testResult.screenshot; } 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); + + const pageImg = ImageHandler.getScreenshot(testResult); const {path: refPath} = this._getExpectedPath(testResult, stateName); 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}); + // Handling whole page common screenshots + if (!stateName && pageImg) { + return { + actualImg: { + path: this._getImgFromStorage(currPath), + size: pageImg.size + } + }; + } + if ((assertViewStatus === SUCCESS || assertViewStatus === UPDATED) && refImg) { const result: ImageInfo = { expectedImg: {path: this._getImgFromStorage(refPath), size: refImg.size} @@ -110,11 +121,11 @@ export class ImageHandler extends EventEmitter2 implements ImagesInfoFormatter { }; } - if (assertViewStatus === ERROR) { + if (assertViewStatus === ERROR && currImg) { return { actualImg: { - path: testResult.state?.name ? this._getImgFromStorage(currPath) : '', - size: (currImg?.size || errImg?.size) as ImageSize + path: this._getImgFromStorage(currPath), + size: currImg.size } }; } @@ -146,14 +157,21 @@ export class ImageHandler extends EventEmitter2 implements ImagesInfoFormatter { ) as ImageInfoFull; }) ?? []; - // common screenshot on test fail + // Common page screenshot if (ImageHandler.getScreenshot(testResult)) { - const errorImage = _.extend( - {status: ERROR, error: getError(testResult.error)}, - this.getImagesFor(testResult, ERROR) - ) as ImageInfoError; + const error = getError(testResult.error); - imagesInfo.push(errorImage); + if (!_.isEmpty(error)) { + imagesInfo.push(_.extend( + {status: ERROR, error}, + this.getImagesFor(testResult, ERROR) + ) as ImageInfoError); + } else { + imagesInfo.push(_.extend( + {status: SUCCESS}, + this.getImagesFor(testResult, SUCCESS) + ) as ImageInfoPageSuccess); + } } return imagesInfo; @@ -201,7 +219,7 @@ export class ImageHandler extends EventEmitter2 implements ImagesInfoFormatter { })); if (ImageHandler.getScreenshot(testResult)) { - await this._saveErrorScreenshot(testResult); + await this._savePageScreenshot(testResult); } await this.emitAsync(PluginEvents.TEST_SCREENSHOTS_SAVED, { @@ -303,7 +321,7 @@ export class ImageHandler extends EventEmitter2 implements ImagesInfoFormatter { cacheDiffImages.set(hash, destPath); } - private async _saveErrorScreenshot(testResult: ReporterTestResultPlain): Promise { + private async _savePageScreenshot(testResult: ReporterTestResultPlain): Promise { const screenshot = ImageHandler.getScreenshot(testResult); if (!(screenshot as ImageBase64)?.base64 && !(screenshot as ImageData)?.path) { logger.warn('Cannot save screenshot on reject'); diff --git a/lib/image-store.ts b/lib/image-store.ts index 2117048f4..bdbb6db79 100644 --- a/lib/image-store.ts +++ b/lib/image-store.ts @@ -27,6 +27,6 @@ export class SqliteImageStore implements ImageStore { }, suitePathString, browserName); const imagesInfo: ImageInfoFull[] = imagesInfoResult && JSON.parse(imagesInfoResult[DB_COLUMNS.IMAGES_INFO as keyof Pick]) || []; - return imagesInfo.find(info => info.stateName === stateName); + return imagesInfo.find(info => (info as {stateName?: string}).stateName === stateName); } } diff --git a/lib/static/components/prop-types.ts b/lib/static/components/prop-types.ts new file mode 100644 index 000000000..a06bf884f --- /dev/null +++ b/lib/static/components/prop-types.ts @@ -0,0 +1,9 @@ +import PropTypes from 'prop-types'; + +export const ImageData = PropTypes.shape({ + path: PropTypes.string.isRequired, + size: PropTypes.shape({ + height: PropTypes.number.isRequired, + width: PropTypes.number.isRequired + }).isRequired +}); diff --git a/lib/static/components/section/body/page-screenshot.tsx b/lib/static/components/section/body/page-screenshot.tsx new file mode 100644 index 000000000..f2e87424b --- /dev/null +++ b/lib/static/components/section/body/page-screenshot.tsx @@ -0,0 +1,18 @@ +import React, {Component} from 'react'; +import Details from '../../details'; +import ResizedScreenshot from '../../state/screenshot/resized'; +import {ImageData} from '../../../../types'; + +interface PageScreenshotProps { + image: ImageData; +} + +export class PageScreenshot extends Component { + render(): JSX.Element { + return
} + extendClassNames="details_type_image" + />; + } +} diff --git a/lib/static/components/section/body/result.jsx b/lib/static/components/section/body/result.jsx index 607f23222..94b850fa7 100644 --- a/lib/static/components/section/body/result.jsx +++ b/lib/static/components/section/body/result.jsx @@ -1,3 +1,4 @@ +import {pick} from 'lodash'; import React, {Component, Fragment} from 'react'; import {connect} from 'react-redux'; import PropTypes from 'prop-types'; @@ -7,6 +8,8 @@ import Description from './description'; import Tabs from './tabs'; import ExtensionPoint from '../../extension-point'; import {RESULT_META} from '../../../../constants/extension-points'; +import {PageScreenshot} from './page-screenshot'; +import * as projectPropTypes from '../../prop-types'; class Result extends Component { static propTypes = { @@ -17,25 +20,36 @@ class Result extends Component { status: PropTypes.string.isRequired, imageIds: PropTypes.array.isRequired, description: PropTypes.string - }).isRequired + }).isRequired, + pageScreenshot: PropTypes.shape({ + actualImg: projectPropTypes.ImageData.isRequired + }) }; render() { - const {result, resultId, testName} = this.props; + const {result, resultId, testName, pageScreenshot} = this.props; return ( - - + + - {result.description && } - + {result.description && } + + {pageScreenshot &&
} + {pageScreenshot && } ); } } export default connect( - ({tree}, {resultId}) => ({result: tree.results.byId[resultId]}) + ({tree}, {resultId}) => { + const result = tree.results.byId[resultId]; + const images = Object.values(pick(tree.images.byId, result.imageIds)); + const pageScreenshot = images.find(image => !image.stateName && image.actualImg); + + return {result, pageScreenshot}; + } )(Result); diff --git a/lib/static/components/section/body/tabs.jsx b/lib/static/components/section/body/tabs.jsx index c414748ca..2b5777502 100644 --- a/lib/static/components/section/body/tabs.jsx +++ b/lib/static/components/section/body/tabs.jsx @@ -1,17 +1,19 @@ import React, {Component} from 'react'; +import {connect} from 'react-redux'; import PropTypes from 'prop-types'; import {isEmpty} from 'lodash'; import State from '../../state'; -import {isSuccessStatus, isErrorStatus} from '../../../../common-utils'; +import {isSuccessStatus, isErrorStatus, isSkippedStatus} from '../../../../common-utils'; -export default class Tabs extends Component { +class Tabs extends Component { static propTypes = { result: PropTypes.shape({ id: PropTypes.string.isRequired, status: PropTypes.string.isRequired, imageIds: PropTypes.array.isRequired, multipleTabs: PropTypes.bool.isRequired, - screenshot: PropTypes.bool.isRequired + screenshot: PropTypes.bool.isRequired, + error: PropTypes.object }).isRequired }; @@ -38,9 +40,14 @@ export default class Tabs extends Component { const errorTabId = `${result.id}_error`; if (isEmpty(result.imageIds)) { - return isSuccessStatus(result.status) - ? null - : this._drawTab({key: errorTabId}); + if (isSuccessStatus(result.status)) { + return null; + } + if (isSkippedStatus(result.status) && isEmpty(result.error)) { + return null; + } + + return this._drawTab({key: errorTabId}); } const tabs = result.imageIds.map((imageId) => this._drawTab({key: imageId, imageId})); @@ -50,3 +57,12 @@ export default class Tabs extends Component { : tabs; } } + +export default connect( + ({tree}, {result}) => { + const filteredResult = {...result}; + filteredResult.imageIds = filteredResult.imageIds.filter(imageId => tree.images.byId[imageId].stateName); + + return {result: filteredResult}; + } +)(Tabs); diff --git a/lib/static/components/state/state-error.jsx b/lib/static/components/state/state-error.jsx index 226541695..0b7e88b01 100644 --- a/lib/static/components/state/state-error.jsx +++ b/lib/static/components/state/state-error.jsx @@ -39,24 +39,13 @@ class StateError extends Component { _drawImage() { const {image, error} = this.props; - if (!image.actualImg) { - return null; + if (image.actualImg && isNoRefImageError(error)) { + return ; } - return isNoRefImageError(error) - ? - :
} - extendClassNames="details_type_image" - onClick={this.onTogglePageScreenshot} - />; + return null; } - onTogglePageScreenshot = () => { - this.props.actions.togglePageScreenshot(); - }; - _errorToElements(error) { return map(error, (value, key) => { if (!value) { diff --git a/lib/test-adapter/index.ts b/lib/test-adapter/index.ts index 23ec0aa85..9ea46ce21 100644 --- a/lib/test-adapter/index.ts +++ b/lib/test-adapter/index.ts @@ -1,5 +1,5 @@ import {TestStatus} from '../constants'; -import {AssertViewResult, ErrorDetails, ImageBase64, ImageInfoFull, TestError} from '../types'; +import {AssertViewResult, ErrorDetails, ImageBase64, ImageData, ImageInfoFull, TestError} from '../types'; export * from './hermione'; @@ -20,7 +20,7 @@ export interface ReporterTestResult { readonly isUpdated?: boolean; readonly meta: Record; readonly multipleTabs: boolean; - readonly screenshot: ImageBase64 | undefined; + readonly screenshot: ImageBase64 | ImageData | null | undefined; readonly sessionId: string; readonly skipReason?: string; readonly state: { name: string }; diff --git a/lib/test-adapter/playwright.ts b/lib/test-adapter/playwright.ts index 574bdfdbe..d732003e6 100644 --- a/lib/test-adapter/playwright.ts +++ b/lib/test-adapter/playwright.ts @@ -9,7 +9,6 @@ import {FAIL, TestStatus, PWT_TITLE_DELIMITER} from '../constants'; import { AssertViewResult, ErrorDetails, - ImageBase64, ImageData, ImageInfoFull, ImageSize, @@ -36,7 +35,7 @@ export enum ImageTitleEnding { Previous = '-previous.png' } -const ANY_IMAGE_ENDING_REGEXP = new RegExp(Object.values(ImageTitleEnding).join('|')); +const ANY_IMAGE_ENDING_REGEXP = new RegExp(Object.values(ImageTitleEnding).map(ending => `${ending}$`).join('|')); const getStatus = (result: PlaywrightTestResult): TestStatus => { if (result.status === PwtTestStatus.PASSED) { @@ -205,8 +204,10 @@ export class PlaywrightTestAdapter implements ReporterTestResult { return true; } - get screenshot(): ImageBase64 | undefined { - return undefined; + get screenshot(): ImageData | null { + const pageScreenshot = this._testResult.attachments.find(a => a.contentType === 'image/png' && a.name === 'screenshot'); + + return getImageData(pageScreenshot); } get sessionId(): string { @@ -245,7 +246,9 @@ export class PlaywrightTestAdapter implements ReporterTestResult { } private get _attachmentsByState(): Record { - const imageAttachments = this._testResult.attachments.filter(a => a.contentType === 'image/png'); + // Filtering out only images. Page screenshots on reject are named "screenshot", we don't want them in state either. + const imageAttachments = this._testResult.attachments.filter( + a => a.contentType === 'image/png' && ANY_IMAGE_ENDING_REGEXP.test(a.name)); return _.groupBy(imageAttachments, a => a.name.replace(ANY_IMAGE_ENDING_REGEXP, '')); } diff --git a/lib/tests-tree-builder/base.ts b/lib/tests-tree-builder/base.ts index f2a650260..27ffb6335 100644 --- a/lib/tests-tree-builder/base.ts +++ b/lib/tests-tree-builder/base.ts @@ -128,7 +128,8 @@ export class BaseTestsTreeBuilder { const browserId = this._buildId(suiteId, browserName); const testResultId = this._buildId(browserId, attempt.toString()); const imageIds = imagesInfo - .map((image: ImageInfoFull, i: number) => this._buildId(testResultId, image.stateName || `${image.status}_${i}`)); + .map((image: ImageInfoFull, i: number) => + this._buildId(testResultId, (image as {stateName?: string}).stateName || `${image.status}_${i}`)); this._addSuites(testPath, browserId); this._addBrowser({id: browserId, parentId: suiteId, name: browserName, version: browserVersion}, testResultId, attempt); diff --git a/lib/types.ts b/lib/types.ts index 1398c5528..706a9fefd 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -75,6 +75,11 @@ export interface ImageInfoSuccess { actualImg?: ImageData; } +export interface ImageInfoPageSuccess { + status: TestStatus.SUCCESS; + actualImg: ImageData; +} + export interface ImageInfoError { status: TestStatus.ERROR; error?: {message: string; stack: string;} @@ -84,12 +89,13 @@ export interface ImageInfoError { actualImg: ImageData; } -export type ImageInfoFull = ImageInfoFail | ImageInfoSuccess | ImageInfoError; +export type ImageInfoFull = ImageInfoFail | ImageInfoSuccess | ImageInfoError | ImageInfoPageSuccess; export type ImageInfo = | Omit | Omit - | Omit; + | Omit + | Omit; export type AssertViewResult = AssertViewSuccess | ImageDiffError | NoRefImageError; diff --git a/playwright.ts b/playwright.ts index 7357da111..516fa93b7 100644 --- a/playwright.ts +++ b/playwright.ts @@ -7,7 +7,7 @@ import {StaticReportBuilder} from './lib/report-builder/static'; import {HtmlReporter} from './lib/plugin-api'; import {ReporterConfig} from './lib/types'; import {parseConfig} from './lib/config'; -import {TestStatus, ToolName} from './lib/constants'; +import {PluginEvents, TestStatus, ToolName} from './lib/constants'; import {RegisterWorkers} from './lib/workers/create-workers'; import {EventEmitter} from 'events'; import {PlaywrightTestAdapter} from './lib/test-adapter/playwright'; @@ -72,17 +72,18 @@ class MyReporter implements Reporter { } else { this._staticReportBuilder.addError(formattedResult); } - this._addTask(this._staticReportBuilder.imageHandler.saveTestImages(formattedResult, this._workers).then()); } else if (status === TestStatus.SUCCESS) { this._staticReportBuilder.addSuccess(formattedResult); } else if (status === TestStatus.SKIPPED) { this._staticReportBuilder.addSkipped(formattedResult); } + this._addTask(this._staticReportBuilder.imageHandler.saveTestImages(formattedResult, this._workers).then()); } async onEnd(): Promise { - this._addTask(this._staticReportBuilder.finalize()); - // TODO: emit report saved event or not? + await this._resultPromise; + await this._staticReportBuilder.finalize(); + await this._htmlReporter.emitAsync(PluginEvents.REPORT_SAVED); return this._resultPromise; } diff --git a/test/func/fixtures/playwright/playwright.config.ts b/test/func/fixtures/playwright/playwright.config.ts index 62e49d8d8..453ebf438 100644 --- a/test/func/fixtures/playwright/playwright.config.ts +++ b/test/func/fixtures/playwright/playwright.config.ts @@ -25,6 +25,10 @@ export default defineConfig({ use: { actionTimeout: 0, baseURL: `http://${serverHost}:${serverPort}/fixtures/hermione/index.html`, + screenshot: { + mode: 'on', + fullPage: true + } }, projects: [