Skip to content

Commit

Permalink
feat: added pathPattern parameter to takeScreenshot method (#8093)
Browse files Browse the repository at this point in the history
<!--
Thank you for your contribution.

Before making a PR, please read our contributing guidelines at

https://github.com/DevExpress/testcafe/blob/master/CONTRIBUTING.md#code-contribution

We recommend creating a *draft* PR, so that you can mark it as 'ready
for review' when you are done.
-->

## Purpose
Add new a parameter pathPattern to takeScreenshot method. It allows the
use of placeholders for specific takeScreenshot calls.

## Approach
I added a new parameter pathPattern to TakeScreenshotCommand .
In src/screenshots/capturer.js _capture method retrieve screenshotPath
from pathPattern.
Show warning if both path and pathPattern present.

## References
 close #8086

## Pre-Merge TODO
- [x] Write tests for your proposed changes
- [x] Make sure that existing tests do not fail

---------

Co-authored-by: aleks-pro <alexander.prokhorov@hotmail.com>
Co-authored-by: Gene <43554315+titerman@users.noreply.github.com>
Co-authored-by: Bayheck <adil.rakhaliyev@devexpress.com>
  • Loading branch information
4 people committed Dec 22, 2023
1 parent c782239 commit 51ad3bc
Show file tree
Hide file tree
Showing 10 changed files with 97 additions and 30 deletions.
1 change: 1 addition & 0 deletions src/notifications/warning-message.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ export default {
screenshotMarkNotFound: 'Unable to locate the page area in the browser window screenshot at {screenshotPath}, because the page area mark with ID {markId} is not found in the screenshot.',
screenshotsFullPageNotSupported: 'TestCafe does not support full-page screenshots in {browserAlias}.',
screenshotRewritingError: 'The file at "{screenshotPath}" already exists. It has just been rewritten with a recent screenshot. This situation can possibly cause issues. To avoid them, make sure that each screenshot has a unique path. If a test runs in multiple browsers, consider including the user agent in the screenshot path or generate a unique identifier in another way.',
screenshotPathOverridesPathPattern: 'The t.takeScreenshot method includes two conflicting parameters: "path" ({customPath}) and "pathPattern" ({customPathPattern}). TestCafe applied the "path" parameter, and ignored the "pathPattern" parameter. Remove one of the parameters to dismiss this warning.',
browserManipulationsOnRemoteBrowser: 'The screenshot and window resize functionalities are not supported in a remote browser. They can function only if the browser is running on the same machine and in the same environment as the TestCafe server.',
screenshotNotSupportedByBrowserProvider: 'The screenshot functionality is not supported by the "{providerName}" browser provider.',
videoNotSupportedByBrowser: 'Video recording is not supported by the "{browserAlias}" browser.',
Expand Down
12 changes: 8 additions & 4 deletions src/screenshots/capturer.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,8 @@ export default class Capturer {
return this._joinWithBaseScreenshotPath(correctedCustomPath);
}

_getScreenshotPath (forError) {
const path = this.pathPattern.getPath(forError);
_getScreenshotPath (forError, customPathPattern) {
const path = this.pathPattern.getPath(forError, customPathPattern);

this._incrementFileIndexes(forError);

Expand All @@ -122,15 +122,19 @@ export default class Capturer {
await this.provider.takeScreenshot(this.browserId, filePath, pageWidth, pageHeight, fullPage);
}

async _capture (forError, { actionId, failedActionId, pageDimensions, cropDimensions, markSeed, customPath, fullPage, thumbnails } = {}) {
async _capture (forError, { actionId, failedActionId, pageDimensions, cropDimensions, markSeed, customPath, customPathPattern, fullPage, thumbnails } = {}) {
if (!this.enabled)
return null;

thumbnails = thumbnails === void 0 ? this.thumbnails : thumbnails;

const screenshotPath = customPath ? this._getCustomScreenshotPath(customPath) : this._getScreenshotPath(forError);
const screenshotPath = customPath ? this._getCustomScreenshotPath(customPath) : this._getScreenshotPath(forError, customPathPattern);

const thumbnailPath = this._getThumbnailPath(screenshotPath);

if (customPath && customPathPattern)
this.warningLog.addWarning(WARNING_MESSAGE.screenshotPathOverridesPathPattern, customPath, customPathPattern);

if (isInQueue(screenshotPath))
this.warningLog.addWarning(WARNING_MESSAGE.screenshotRewritingError, screenshotPath);

Expand Down
15 changes: 8 additions & 7 deletions src/test-run/browser-manipulation-queue.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,14 @@ export default class BrowserManipulationQueue {
case COMMAND_TYPE.takeElementScreenshot:
case COMMAND_TYPE.takeScreenshot:
return await this._takeScreenshot(() => this.screenshotCapturer.captureAction({
actionId: command.actionId,
customPath: command.path,
pageDimensions: driverMsg.pageDimensions,
cropDimensions: driverMsg.cropDimensions,
markSeed: command.markSeed,
fullPage: command.fullPage,
thumbnails: command.thumbnails,
actionId: command.actionId,
customPath: command.path,
customPathPattern: command.pathPattern,
pageDimensions: driverMsg.pageDimensions,
cropDimensions: driverMsg.cropDimensions,
markSeed: command.markSeed,
fullPage: command.fullPage,
thumbnails: command.thumbnails,
}));

case COMMAND_TYPE.takeScreenshotOnFail:
Expand Down
1 change: 1 addition & 0 deletions src/test-run/commands/browser-manipulation.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ export class TakeScreenshotCommand extends TakeScreenshotBaseCommand {
getAssignableProperties () {
return [
{ name: 'path', type: screenshotPathArgument, defaultValue: '' },
{ name: 'pathPattern', type: stringArgument, defaultValue: '' },
{ name: 'fullPage', type: booleanArgument, defaultValue: void 0 },
{ name: 'thumbnails', type: booleanArgument, defaultValue: void 0 },
];
Expand Down
7 changes: 4 additions & 3 deletions src/utils/path-pattern.js
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,10 @@ export default class PathPattern extends EventEmitter {
return resultFilePath;
}

getPath (forError) {
const pattern = this.patternOnFails && forError ? this.patternOnFails : this.pattern;
const path = this._buildPath(pattern, this.placeholderToDataMap, forError);
getPath (forError, customPathPattern) {
const pathPattern = customPathPattern || this.pattern;
const pattern = this.patternOnFails && forError ? this.patternOnFails : pathPattern;
const path = this._buildPath(pattern, this.placeholderToDataMap, forError);

return correctFilePath(path, this.fileExtension);
}
Expand Down
26 changes: 26 additions & 0 deletions test/functional/fixtures/api/es-next/take-screenshot/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,32 @@ describe('[API] t.takeScreenshot()', function () {
});
});

it('Should take a screenshot with a custom pathPattern', function () {
return runTests('./testcafe-fixtures/take-screenshot.js', 'Take a screenshot with a pathPattern')
.then(function () {
const screenshotPath = path.join('screenshots', 'Take a screenshot with a pathPattern');
const screenshotsCheckingOptions = { baseDir: 'screenshots', forError: false, screenshotsCount: 2, customPath: screenshotPath };

expect(assertionHelper.checkScreenshotsCreated(screenshotsCheckingOptions)).eql(true);
return assertionHelper.removeScreenshotDir('screenshots');
});
});

it('Should create a warning if pathPattern is used with path parameter', function () {
const { reporter, assertReporterWarnings, warningResult } = createWarningReporter();

return runTests('./testcafe-fixtures/take-screenshot.js', 'Should create a warning and use path parameter', { reporter })
.then(function () {
const screenshotPath = path.join('screenshots', 'screenshot-path');
const screenshotsCheckingOptions = { baseDir: 'screenshots', forError: false, screenshotsCount: 2, customPath: screenshotPath };

expect(assertionHelper.checkScreenshotsCreated(screenshotsCheckingOptions)).eql(true);
expect(warningResult.warnings[0].message).eql('The t.takeScreenshot method includes two conflicting parameters: "path" (screenshot-path/custom) and "pathPattern" (${TEST}/custom). TestCafe applied the "path" parameter, and ignored the "pathPattern" parameter. Remove one of the parameters to dismiss this warning.');
assertReporterWarnings('takeScreenshot');
return assertionHelper.removeScreenshotDir('screenshots');
});
});

it('Should save screenshots to default dir if screenshotPath is not specified', function () {
return runTests('./testcafe-fixtures/take-screenshot.js', 'Take a screenshot')
.then(function () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,3 +137,11 @@ test('Rewrite a screenshot with warning', async t => {
await t.takeScreenshot('custom/duplicate.png');
await t.takeScreenshot('custom/duplicate.png');
});

test('Take a screenshot with a pathPattern', async t => {
await t.takeScreenshot({ pathPattern: '${TEST}/custom' });
});

test('Should create a warning and use path parameter', async t => {
await t.takeScreenshot({ path: 'screenshot-path/custom', pathPattern: '${TEST}/custom' });
});
11 changes: 6 additions & 5 deletions test/server/data/test-controller-reporter-expected/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -433,11 +433,12 @@ module.exports = {
testRunId: 'test-run-id',
name: 'takeScreenshot',
command: {
path: 'screenshotPath',
fullPage: true,
thumbnails: undefined,
type: 'take-screenshot',
actionId: 'TakeScreenshotCommand',
path: 'screenshotPath',
pathPattern: '',
fullPage: true,
thumbnails: undefined,
type: 'take-screenshot',
actionId: 'TakeScreenshotCommand',
},
test: {
id: 'test-id',
Expand Down
42 changes: 31 additions & 11 deletions test/server/test-run-commands-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -853,12 +853,13 @@ describe('Test run commands', () => {
let command = createCommand(commandObj);

expect(JSON.parse(JSON.stringify(command))).eql({
type: TYPE.takeScreenshot,
actionId: TYPE.takeScreenshot,
markData: '',
markSeed: null,
path: 'custom',
fullPage: true,
type: TYPE.takeScreenshot,
actionId: TYPE.takeScreenshot,
markData: '',
markSeed: null,
path: 'custom',
fullPage: true,
pathPattern: '',
});

commandObj = {
Expand All @@ -869,11 +870,30 @@ describe('Test run commands', () => {
command = createCommand(commandObj);

expect(JSON.parse(JSON.stringify(command))).eql({
type: TYPE.takeScreenshot,
actionId: TYPE.takeScreenshot,
markData: '',
markSeed: null,
path: '',
type: TYPE.takeScreenshot,
actionId: TYPE.takeScreenshot,
markData: '',
markSeed: null,
path: '',
pathPattern: '',
});

commandObj = {
type: TYPE.takeScreenshot,
fullPage: true,
pathPattern: 'custom',
};

command = createCommand(commandObj);

expect(JSON.parse(JSON.stringify(command))).eql({
type: TYPE.takeScreenshot,
actionId: TYPE.takeScreenshot,
fullPage: true,
markData: '',
markSeed: null,
path: '',
pathPattern: 'custom',
});
});

Expand Down
4 changes: 4 additions & 0 deletions ts-defs-src/test-api/action-options.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ interface TakeScreenshotOptions {
* Specifies that TestCafe should take full-page screenshots.
*/
fullPage?: boolean;
/**
* Specifies the custom naming pattern for the screenshot.
*/
pathPattern?: string;
}

interface TakeElementScreenshotOptions extends ActionOptions {
Expand Down

0 comments on commit 51ad3bc

Please sign in to comment.