Fix messages & review remarks

src/browser/provider/built-in/chrome/cdp.js

@@ -114,9 +114,9 @@ export async function updateMobileViewportSize (runtimeInfo) {
 }
 
 export async function getVideoFrameData ({ client }) {
-    const screenshotData = await getScreenshotData(client);
+    const frameData = await getScreenshotData(client);
 
-    return Buffer.from(screenshotData.data, 'base64');
+    return Buffer.from(frameData.data, 'base64');
 }
 
 export async function takeScreenshot (path, { client }) {

src/browser/provider/built-in/firefox/marionette-client/index.js

@@ -179,9 +179,9 @@ module.exports = class MarionetteClient {
     }
 
     async getVideoFrameData () {
-        const screenshotData = await this._getScreenshotData();
+        const frameData = await this._getScreenshotData();
 
-        return Buffer.from(screenshotData.value, 'base64');
+        return Buffer.from(frameData.value, 'base64');
     }
 
     async setWindowSize (width, height) {

src/cli/argument-parser.js

@@ -89,9 +89,9 @@ export default class CLIArgumentParser {
             .option('--proxy <host>', 'specify the host of the proxy server')
             .option('--proxy-bypass <rules>', 'specify a comma-separated list of rules that define URLs accessed bypassing the proxy server')
             .option('--ssl <options>', 'specify SSL options to run TestCafe proxy server over the HTTPS protocol')
-            .option('--video <path>', 'capture a video of the running tests')
-            .option('--video-options <option=value[,...]>', 'specify the way videos will be captured')
-            .option('--video-encoding-options <option=value[,...]>', 'specify encoding options for video capturing')
+            .option('--video <path>', ' record videos of test runs')
+            .option('--video-options <option=value[,...]>', 'specify video recording options')
+            .option('--video-encoding-options <option=value[,...]>', 'specify encoding options')
             .option('--disable-page-reloads', 'disable page reloads between tests')
             .option('--dev', 'enables mechanisms to log and diagnose errors')
             .option('--qr-code', 'outputs QR-code that repeats URLs used to connect the remote browsers')

src/errors/runtime/message.js

@@ -18,31 +18,53 @@ export default {
     multipleStdoutReporters:                                        'Multiple reporters attempting to write to stdout: "{reporters}". Only one reporter can write to stdout.',
     optionValueIsNotValidRegExp:                                    'The "{optionName}" option value is not a valid regular expression.',
     optionValueIsNotValidKeyValue:                                  'The "{optionName}" option value is not a valid key-value pair.',
-    testedAppFailedWithError:                                       'Tested app failed with an error:\n\n{errMessage}',
     invalidSpeedValue:                                              'Speed should be a number between 0.01 and 1.',
     invalidConcurrencyFactor:                                       'The concurrency factor should be an integer greater or equal to 1.',
     cannotDivideRemotesCountByConcurrency:                          'The number of remote browsers should be divisible by the factor of concurrency.',
     portsOptionRequiresTwoNumbers:                                  'The "--ports" option requires two numbers to be specified.',
     portIsNotFree:                                                  'The specified {portNum} port is already in use by another program.',
     invalidHostname:                                                'The specified "{hostname}" hostname cannot be resolved to the current machine.',
     cantFindSpecifiedTestSource:                                    'Cannot find a test source file at "{path}".',
-    cannotParseRawFile:                                             'Cannot parse a test source file in the raw format at "{path}" due to an error.\n\n{errMessage}',
-    cannotPrepareTestsDueToError:                                   'Cannot prepare tests due to an error.\n\n{errMessage}',
     clientFunctionCodeIsNotAFunction:                               '{#instantiationCallsiteName} code is expected to be specified as a function, but {type} was passed.',
     selectorInitializedWithWrongType:                               '{#instantiationCallsiteName} is expected to be initialized with a function, CSS selector string, another Selector, node snapshot or a Promise returned by a Selector, but {type} was passed.',
     clientFunctionCantResolveTestRun:                               "{#instantiationCallsiteName} cannot implicitly resolve the test run in context of which it should be executed. If you need to call {#instantiationCallsiteName} from the Node.js API callback, pass the test controller manually via {#instantiationCallsiteName}'s `.with({ boundTestRun: t })` method first. Note that you cannot execute {#instantiationCallsiteName} outside the test code.",
     regeneratorInClientFunctionCode:                                `{#instantiationCallsiteName} code, arguments or dependencies cannot contain generators or "async/await" syntax (use Promises instead).`,
     invalidClientFunctionTestRunBinding:                            'The "boundTestRun" option value is expected to be a test controller.',
     invalidValueType:                                               '{smthg} is expected to be a {type}, but it was {actual}.',
     unsupportedUrlProtocol:                                         'The specified "{url}" test page URL uses an unsupported {protocol}:// protocol. Only relative URLs or absolute URLs with http://, https:// and file:// protocols are supported.',
-    unableToOpenBrowser:                                            'Was unable to open the browser "{alias}" due to error.\n\n{errMessage}',
     testControllerProxyCantResolveTestRun:                          `Cannot implicitly resolve the test run in the context of which the test controller action should be executed. Use test function's 't' argument instead.`,
-    requestHookConfigureAPIError:                                   'There was an error while configuring the request hook:\n\n{requestHookName}: {errMsg}',
     timeLimitedPromiseTimeoutExpired:                               'Timeout expired for a time limited promise',
-    forbiddenCharatersInScreenshotPath:                             'There are forbidden characters in the "{screenshotPath}" {screenshotPathType}:\n {forbiddenCharsDescription}',
     cantUseScreenshotPathPatternWithoutBaseScreenshotPathSpecified: 'Cannot use the screenshot path pattern without a base screenshot path specified',
-    cantSetVideoOptionsWithoutBaseVideoPathSpecified:               'Cannot set the video and video encoding options without a base video path specified.',
-    cantFindFFMPEG:                                                 'Cannot find the FFMPEG executable required for video recording. Please add the FFMPEG installation directory to the PATH environment variable, provide a path to the executable through the FFMPEG_PATH environment variable, the --video-options ffmpegPath=<path> option or install the @ffmpeg-installer/ffmpeg package from NPM.',
+    cantSetVideoOptionsWithoutBaseVideoPathSpecified:               'Unable to set video or encoding options when video recording is disabled. Specify the base path where video files are stored to enable recording.',
     multipleAPIMethodCallForbidden:                                 'You cannot call the "{methodName}" method more than once. Pass an array of parameters to this method instead.',
-    invalidReporterOutput:                                          "Specify a file name or a writable stream as the reporter's output target."
+    invalidReporterOutput:                                          "Specify a file name or a writable stream as the reporter's output target.",
+
+    cannotPrepareTestsDueToError: 'Cannot prepare tests due to an error.\n' +
+                                  '\n' +
+                                  '{errMessage}',
+
+    cannotParseRawFile: 'Cannot parse a test source file in the raw format at "{path}" due to an error.\n' +
+                        '\n' +
+                        '{errMessage}',
+
+    testedAppFailedWithError: 'Tested app failed with an error:\n' +
+                              '\n' +
+                              '{errMessage}',
+
+    unableToOpenBrowser: 'Was unable to open the browser "{alias}" due to error.\n' +
+                         '\n' +
+                         '{errMessage}',
+
+    requestHookConfigureAPIError: 'There was an error while configuring the request hook:\n' +
+                                  '\n' +
+                                  '{requestHookName}: {errMsg}',
+
+    forbiddenCharatersInScreenshotPath: 'There are forbidden characters in the "{screenshotPath}" {screenshotPathType}:\n' +
+                                        ' {forbiddenCharsDescription}',
+
+    cantFindFFMPEG: 'Unable to locate the FFmpeg executable required to record videos. Do one of the following:\n' +
+                    '\n' +
+                    '* add the FFmpeg installation directory to the PATH environment variable,\n' +
+                    '* specify the path to the FFmpeg executable in the FFMPEG_PATH environment variable or the ffmpegPath video option,\n' +
+                    '* install the @ffmpeg-installer/ffmpeg package from npm.',
 };

src/utils/detect-ffmpeg.js

@@ -38,16 +38,8 @@ async function requireFFMPEGModule () {
 }
 
 export default async function () {
-    let ffmpegPath = process.env.FFMPEG_PATH;
-
-    if (!ffmpegPath)
-        ffmpegPath = await requireFFMPEGModuleFromCwd();
-
-    if (!ffmpegPath)
-        ffmpegPath = await requireFFMPEGModule();
-
-    if (!ffmpegPath)
-        ffmpegPath = await findFFMPEGinPath();
-
-    return ffmpegPath;
+    return process.env.FFMPEG_PATH ||
+        await requireFFMPEGModuleFromCwd() ||
+        await requireFFMPEGModule() ||
+        await findFFMPEGinPath();
 }

src/video-recorder/index.js

@@ -46,19 +46,16 @@ export default class VideoRecorder {
     }
 
     _getTargetVideoPath (testRunInfo) {
-        if (this.singleFile)
-            return join(this.basePath, 'video.mp4');
-
         const { quarantine, test, index, testRun } = testRunInfo;
 
         const connection = testRun.browserConnection;
 
         const pathPattern = new PathPattern(this.customPathPattern, VIDEO_EXTENSION, {
-            testIndex:         index,
-            quarantineAttempt: quarantine ? quarantine.getNextAttemptNumber() : null,
+            testIndex:         this.singleFile ? 1 : index,
+            quarantineAttempt: quarantine && !this.singleFile ? quarantine.getNextAttemptNumber() : null,
             now:               this.timeStamp,
-            fixture:           test.fixture.name,
-            test:              test.name,
+            fixture:           this.singleFile ? '' : test.fixture.name,
+            test:              this.singleFile ? '' : test.name,
             parsedUserAgent:   connection.browserInfo.parsedUserAgent,
         });
 

src/video-recorder/process.js

@@ -6,14 +6,30 @@ import delay from '../utils/delay';
 
 
 const DEFAULT_OPTIONS = {
-    'y':                           true,
+    // NOTE: don't ask confirmation for rewriting the output file
+    'y': true,
+
+    // NOTE: use the time when a frame is read from the source as its timestamp
+    // IMPORTANT: must be specified before configuring the source
     'use_wallclock_as_timestamps': 1,
-    'i':                           'pipe:0',
-    'c:v':                         'libx264',
-    'preset':                      'ultrafast',
-    'pix_fmt':                     'yuv420p',
-    'vf':                          'scale=trunc(iw/2)*2:trunc(ih/2)*2',
-    'r':                           30
+
+    // NOTE: use stdin as a source
+    'i': 'pipe:0',
+
+    // NOTE: use the H.264 video codec
+    'c:v': 'libx264',
+
+    // NOTE: use the 'ultrafast' compression preset
+    'preset': 'ultrafast',
+
+    // NOTE: use the yuv420p pixel format (the most widely supported)
+    'pix_fmt': 'yuv420p',
+
+    // NOTE: scale input frames to make the frame height divisible by 2 (yuv420p's requirement)
+    'vf': 'scale=trunc(iw/2)*2:trunc(ih/2)*2',
+
+    // NOTE: set the frame rate to 30 in the output video (the most widely supported)
+    'r': 30
 };
 
 const FFMPEG_START_DELAY = 500;

test/server/runner-test.js

@@ -285,7 +285,7 @@ describe('Runner', () => {
                     throw new Error('Promise rejection expected');
                 })
                 .catch(err => {
-                    expect(err.message).eql('Cannot set the video and video encoding options without a base video path specified.');
+                    expect(err.message).eql('Unable to set video or encoding options when video recording is disabled. Specify the base path where video files are stored to enable recording.');
                 });
         });
 
@@ -299,7 +299,7 @@ describe('Runner', () => {
                     throw new Error('Promise rejection expected');
                 })
                 .catch(err => {
-                    expect(err.message).eql('Cannot set the video and video encoding options without a base video path specified.');
+                    expect(err.message).eql('Unable to set video or encoding options when video recording is disabled. Specify the base path where video files are stored to enable recording.');
                 });
         });
     });