Skip to content

Commit 51b54c7

Browse files
PavelMor25PavelMor25
authored
feat: Added path-pattern-on-fails for screenshots (closes #7014) (#8055)
## Approach - added support new configuration screenshot option ## References [issue 7014](#7014) ## Pre-Merge TODO - [x] Write tests for your proposed changes - [x] Make sure that existing tests do not fail
1 parent 640cf6b commit 51b54c7

File tree

11 files changed

+82
-34
lines changed

11 files changed

+82
-34
lines changed

‎src/cli/argument-parser/index.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ interface CommandLineOptions {
8080
reporter?: string | ReporterOption[];
8181
screenshots?: Dictionary<string | number | boolean> | string;
8282
screenshotPathPattern?: string;
83+
screenshotPathPatternOnFails?: string;
8384
screenshotsOnFails?: boolean;
8485
videoOptions?: string | Dictionary<number | string | boolean>;
8586
videoEncodingOptions?: string | Dictionary<number | string | boolean>;
@@ -402,6 +403,9 @@ export default class CLIArgumentParser {
402403
if (!has(this.opts.screenshots, SCREENSHOT_OPTION_NAMES.pathPattern) && this.opts.screenshotPathPattern)
403404
this.opts.screenshots[SCREENSHOT_OPTION_NAMES.pathPattern] = this.opts.screenshotPathPattern;
404405

406+
if (!has(this.opts.screenshots, SCREENSHOT_OPTION_NAMES.pathPatternOnFails) && this.opts.screenshotPathPatternOnFails)
407+
this.opts.screenshots[SCREENSHOT_OPTION_NAMES.pathPatternOnFails] = this.opts.screenshotPathPatternOnFails;
408+
405409
if (!has(this.opts.screenshots, SCREENSHOT_OPTION_NAMES.takeOnFails) && this.opts.screenshotsOnFails)
406410
this.opts.screenshots[SCREENSHOT_OPTION_NAMES.takeOnFails] = this.opts.screenshotsOnFails;
407411
}

‎src/configuration/option-names.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ enum OptionNames {
1111
screenshots = 'screenshots',
1212
screenshotPath = 'screenshotPath',
1313
screenshotPathPattern = 'screenshotPathPattern',
14+
screenshotPathPatternOnFails = 'screenshotPathPatternOnFails',
1415
takeScreenshotsOnFails = 'takeScreenshotsOnFails',
1516
proxyBypass = 'proxyBypass',
1617
appCommand = 'appCommand',

‎src/configuration/screenshot-option-names.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@ enum SCREENSHOT_OPTION_NAMES {
33
takeOnFails = 'takeOnFails',
44
pathPattern = 'pathPattern',
55
fullPage = 'fullPage',
6-
thumbnails = 'thumbnails'
6+
thumbnails = 'thumbnails',
7+
pathPatternOnFails = 'pathPatternOnFails'
78
}
89

910
export default SCREENSHOT_OPTION_NAMES;

‎src/configuration/types.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ interface ScreenshotOptionValue {
77
path: string;
88
takeOnFails?: boolean;
99
pathPattern?: string;
10+
pathPatternOnFails?: string;
1011
fullPage?: boolean;
1112
thumbnails?: boolean;
1213
}

‎src/runner/index.js‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -351,19 +351,22 @@ export default class Runner extends EventEmitter {
351351
}
352352

353353
_getScreenshotOptions () {
354-
let { path, pathPattern } = this.configuration.getOption(OPTION_NAMES.screenshots) || {};
354+
let { path, pathPattern, pathPatternOnFails } = this.configuration.getOption(OPTION_NAMES.screenshots) || {};
355355

356356
if (!path)
357357
path = this.configuration.getOption(OPTION_NAMES.screenshotPath);
358358

359359
if (!pathPattern)
360360
pathPattern = this.configuration.getOption(OPTION_NAMES.screenshotPathPattern);
361361

362-
return { path, pathPattern };
362+
if (!pathPatternOnFails)
363+
pathPatternOnFails = this.configuration.getOption(OPTION_NAMES.screenshotPathPatternOnFails);
364+
365+
return { path, pathPattern, pathPatternOnFails };
363366
}
364367

365368
_validateScreenshotOptions () {
366-
const { path, pathPattern } = this._getScreenshotOptions();
369+
const { path, pathPattern, pathPatternOnFails } = this._getScreenshotOptions();
367370

368371
const disableScreenshots = this.configuration.getOption(OPTION_NAMES.disableScreenshots) || !path;
369372

@@ -383,6 +386,12 @@ export default class Runner extends EventEmitter {
383386

384387
this.configuration.mergeOptions({ [OPTION_NAMES.screenshots]: { pathPattern } });
385388
}
389+
390+
if (pathPatternOnFails) {
391+
this._validateScreenshotPath(pathPatternOnFails, 'screenshots path pattern on fails');
392+
393+
this.configuration.mergeOptions({ [OPTION_NAMES.screenshots]: { pathPatternOnFails } });
394+
}
386395
}
387396

388397
async _validateVideoOptions () {
@@ -740,12 +749,13 @@ export default class Runner extends EventEmitter {
740749
screenshots (...options) {
741750
let fullPage;
742751
let thumbnails;
752+
let pathPatternOnFails;
743753
let [path, takeOnFails, pathPattern] = options;
744754

745755
if (options.length === 1 && options[0] && typeof options[0] === 'object')
746-
({ path, takeOnFails, pathPattern, fullPage, thumbnails } = options[0]);
756+
({ path, takeOnFails, pathPattern, pathPatternOnFails, fullPage, thumbnails } = options[0]);
747757

748-
this._options.screenshots = { path, takeOnFails, pathPattern, fullPage, thumbnails };
758+
this._options.screenshots = { path, takeOnFails, pathPattern, pathPatternOnFails, fullPage, thumbnails };
749759

750760
return this;
751761
}

‎src/runner/task/index.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,12 +59,13 @@ export default class Task extends AsyncEventEmitter {
5959

6060
this.warningLog.copyFrom(runnerWarningLog);
6161

62-
const { path, pathPattern, fullPage, thumbnails } = this.opts.screenshots as ScreenshotOptionValue;
62+
const { path, pathPattern, pathPatternOnFails, fullPage, thumbnails } = this.opts.screenshots as ScreenshotOptionValue;
6363

6464
this.screenshots = new Screenshots({
6565
enabled: !this.opts.disableScreenshots,
6666
path,
6767
pathPattern,
68+
pathPatternOnFails,
6869
fullPage,
6970
thumbnails,
7071
});

‎src/screenshots/index.js‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,15 @@ import DEFAULT_SCREENSHOT_EXTENSION from './default-extension';
77

88

99
export default class Screenshots {
10-
constructor ({ enabled, path, pathPattern, fullPage, thumbnails }) {
11-
this.enabled = enabled;
12-
this.screenshotsPath = path;
13-
this.screenshotsPattern = pathPattern;
14-
this.fullPage = fullPage;
15-
this.thumbnails = thumbnails;
16-
this.testEntries = [];
17-
this.now = moment();
10+
constructor ({ enabled, path, pathPattern, pathPatternOnFails, fullPage, thumbnails }) {
11+
this.enabled = enabled;
12+
this.screenshotsPath = path;
13+
this.screenshotsPattern = pathPattern;
14+
this.screenshotsPatternOnFails = pathPatternOnFails;
15+
this.fullPage = fullPage;
16+
this.thumbnails = thumbnails;
17+
this.testEntries = [];
18+
this.now = moment();
1819
}
1920

2021
_addTestEntry (test) {
@@ -66,7 +67,7 @@ export default class Screenshots {
6667
fixture: test.fixture.name,
6768
test: test.name,
6869
parsedUserAgent: connection.browserInfo.parsedUserAgent,
69-
});
70+
}, this.screenshotsPatternOnFails);
7071

7172
return new Capturer(this.screenshotsPath, testEntry, connection, pathPattern, this.fullPage, this.thumbnails, warningLog);
7273
}

‎src/utils/path-pattern.js‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,11 @@ const TEST_ID_TEMPLATE = data => data.testIndex ? `test-${data.testIndex}` : '';
3434
const RUN_ID_TEMPLATE = data => data.quarantineAttempt ? `run-${data.quarantineAttempt}` : '';
3535

3636
export default class PathPattern extends EventEmitter {
37-
constructor (pattern, fileExtension, data) {
37+
constructor (pattern, fileExtension, data, patternOnFails) {
3838
super();
3939

4040
this.pattern = this._ensurePattern(pattern);
41+
this.patternOnFails = patternOnFails;
4142
this.data = this._addDefaultFields(data);
4243
this.placeholderToDataMap = this._createPlaceholderToDataMap();
4344
this.fileExtension = fileExtension;
@@ -94,7 +95,7 @@ export default class PathPattern extends EventEmitter {
9495
const getFileIndexFn = placeholderToDataMap[placeholder];
9596
let result = getFileIndexFn(forError);
9697

97-
if (forError)
98+
if (!this.patternOnFails && forError)
9899
result = `${ERRORS_FOLDER}\\${result}`;
99100

100101
return result;
@@ -125,7 +126,8 @@ export default class PathPattern extends EventEmitter {
125126
}
126127

127128
getPath (forError) {
128-
const path = this._buildPath(this.pattern, this.placeholderToDataMap, forError);
129+
const pattern = this.patternOnFails && forError ? this.patternOnFails : this.pattern;
130+
const path = this._buildPath(pattern, this.placeholderToDataMap, forError);
129131

130132
return correctFilePath(path, this.fileExtension);
131133
}

‎test/functional/fixtures/screenshots-on-fails/test.js‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
const { expect } = require('chai');
22
const config = require('../../config.js');
33
const assertionHelper = require('../../assertion-helper.js');
4+
const path = require('path');
5+
const fs = require('fs');
46

7+
8+
const SCREENSHOTS_PATH = path.resolve(assertionHelper.SCREENSHOTS_PATH);
59
const SCREENSHOT_PATH_MESSAGE_TEXT = 'Screenshot: ___test-screenshots___';
610
const REPORT_SCREENSHOT_PATH_TEXT_RE = /___test-screenshots___[\\/]\d{4,4}-\d{2,2}-\d{2,2}_\d{2,2}-\d{2,2}-\d{2,2}[\\/]test-1/;
711
const ERROR_SCREENSHOT_PATH_RE = /Screenshot: .*?___test-screenshots___[\\/]\d{4,4}-\d{2,2}-\d{2,2}_\d{2,2}-\d{2,2}-\d{2,2}[\\/]test-1[\\/]\S+[\\/]errors[\\/]\d.png/;
@@ -140,6 +144,22 @@ describe('Screenshots on fails', function () {
140144
expect(result).eql(true);
141145
});
142146
});
147+
148+
it('Should have file name respect the mask inside error folder', function () {
149+
return runTests('./testcafe-fixtures/screenshots-on-fails.js', 'Screenshot on the assertion fail',
150+
{
151+
shouldFail: true,
152+
screenshotsOnFails: true,
153+
setScreenshotPath: true,
154+
screenshotPathPattern: '${TEST}/test-${FILE_INDEX}',
155+
screenshotPathPatternOnFails: '${TEST}/errors/test-${FILE_INDEX}',
156+
})
157+
.catch(() => {
158+
const PATH_SCREENSHOOT_ERROR = path.join(SCREENSHOTS_PATH, `Screenshot on the assertion fail`, `errors`, `test-1.png`);
159+
160+
expect(fs.existsSync(PATH_SCREENSHOOT_ERROR)).eql(true);
161+
});
162+
});
143163
}
144164
else if (!config.useLocalBrowsers) {
145165
it('Should show a warning on an attempt to capture a screenshot for a remote browser', () => {

‎test/functional/setup.js‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,7 @@ before(function () {
268268
skipJsErrors,
269269
quarantineMode,
270270
screenshotPathPattern,
271+
screenshotPathPatternOnFails,
271272
screenshotsOnFails,
272273
screenshotsFullPage,
273274
videoOptions,
@@ -344,10 +345,11 @@ before(function () {
344345
})
345346
.src(fixturePath)
346347
.screenshots({
347-
path: screenshotPath,
348-
takeOnFails: screenshotsOnFails,
349-
pathPattern: screenshotPathPattern,
350-
fullPage: screenshotsFullPage,
348+
path: screenshotPath,
349+
takeOnFails: screenshotsOnFails,
350+
pathPattern: screenshotPathPattern,
351+
pathPatternOnFails: screenshotPathPatternOnFails,
352+
fullPage: screenshotsFullPage,
351353
})
352354
.video(videoPath, videoOptions, videoEncodingOptions)
353355
.startApp(appCommand, appInitDelay)

0 commit comments

Comments
 (0)