Simplify handling of "special" commands not having instructions and updating context and adding beginning of within-iframe command

GuillaumeGomez · GuillaumeGomez · commit 5eb2d6ff9c4f · 2025-01-22T18:48:02.000+01:00
diff --git a/src/commands.js b/src/commands.js
@@ -2,7 +2,6 @@ const { AstLoader } = require('./ast.js');
 const process = require('process');
 const commands = require('./commands/all.js');
 const consts = require('./consts.js');
-const path = require('path');
 const { stripCommonPathsPrefix } = require('./utils.js');
 
 const ORDERS = {
@@ -117,6 +116,7 @@ const ORDERS = {
     'wait-for-text-false': commands.parseWaitForTextFalse,
     'wait-for-window-property': commands.parseWaitForWindowProperty,
     'wait-for-window-property-false': commands.parseWaitForWindowPropertyFalse,
+    'within-iframe': commands.parseWithinIFrame,
     'write': commands.parseWrite,
     'write-into': commands.parseWriteInto,
 };
@@ -164,6 +164,7 @@ const FATAL_ERROR_COMMANDS = [
     'wait-for-property-false',
     'wait-for-text',
     'wait-for-text-false',
+    'within-iframe',
     'write',
     'write-into',
 ];
@@ -262,64 +263,6 @@ class ParserWithContext {
         }
     }
 
-    setup_user_function_call(ast) {
-        const ret = commands.parseCallFunction(this);
-        if (ret.error !== undefined) {
-            ret.line = this.get_current_command_line();
-            if (ast.length !== 0) {
-                ret.error += ` (from command \`${ast[0].getErrorText()}\`)`;
-            }
-            return ret;
-        }
-        const args = Object.create(null);
-        const func = this.definedFunctions[ret['function']];
-        for (const arg_name of func['arguments']) {
-            const index = ret['args'].findIndex(arg => arg.key.value === arg_name);
-            if (index === -1) {
-                return {
-                    'error': `Missing argument "${arg_name}"`,
-                    'line': this.get_current_command_line(),
-                    'fatal_error': true,
-                };
-            }
-            args[arg_name] = ret['args'][index].value;
-        }
-        const context = this.get_current_context();
-        this.pushNewContext({
-            'ast': context.ast,
-            'commands': func.commands,
-            'currentCommand': 0,
-            'functionArgs': Object.assign({}, context.functionArgs, args),
-            'filePath': func.filePath,
-        });
-        // We disable the `increasePos` in the context to prevent it to be done twice.
-        return this.get_next_command(false);
-    }
-
-    setup_include() {
-        const ret = commands.parseInclude(this);
-        if (ret.error !== undefined) {
-            ret.line = this.get_current_command_line();
-            if (ast.length !== 0) {
-                ret.error += ` (from command \`${ast[0].getErrorText()}\`)`;
-            }
-            return ret;
-        }
-        const dirPath = path.dirname(this.get_current_context().ast.absolutePath);
-        const ast = new AstLoader(ret.path, dirPath);
-        if (ast.hasErrors()) {
-            return {'errors': ast.errors};
-        }
-        this.pushNewContext({
-            'ast': ast,
-            'commands': ast.commands,
-            'currentCommand': 0,
-            'functionArgs': Object.create(null),
-        });
-        // We disable the `increasePos` in the context to prevent it to be done twice.
-        return this.get_next_command(false);
-    }
-
     getCurrentFile() {
         const context = this.get_current_context();
         if (context === null) {
@@ -335,14 +278,7 @@ class ParserWithContext {
         // through `ParserWithContext`.
         this.elems = ast;
 
-        if (order === 'call-function') {
-            // We need to special-case `call-function` since it needs to access variables of this
-            // class.
-            return this.setup_user_function_call(ast);
-        } else if (order === 'include') {
-            // We need to special-case `include` since it needs to parse a new file when called.
-            return this.setup_include();
-        } else if (!Object.prototype.hasOwnProperty.call(ORDERS, order)) {
+        if (!Object.prototype.hasOwnProperty.call(ORDERS, order)) {
             return {'error': `Unknown command "${order}"`, 'line': this.get_current_command_line()};
         }
         if (this.firstGotoParsed === false) {
@@ -367,10 +303,15 @@ class ParserWithContext {
         if (res.error !== undefined) {
             res.line = this.get_current_command_line();
             if (this.elems.length !== 0) {
-                res.error += ` (from command \`${this.elems[0].getErrorText()}\`)`;
+                res.error += ` (from command \`${order}: ${this.elems[0].getErrorText()}\`)`;
             }
             return res;
         }
+        if (res.skipInstructions) {
+            // We disable the `increasePos` in the context to prevent it to be done twice.
+            return this.get_next_command(false);
+        }
+
         return {
             'fatal_error': FATAL_ERROR_COMMANDS.indexOf(order) !== -1,
             'wait': res['wait'],
diff --git a/src/commands/all.js b/src/commands/all.js
@@ -125,6 +125,7 @@ module.exports = {
     'parseWaitForTextFalse': wait.parseWaitForTextFalse,
     'parseWaitForWindowProperty': wait.parseWaitForWindowProperty,
     'parseWaitForWindowPropertyFalse': wait.parseWaitForWindowPropertyFalse,
+    'parseWithinIFrame': context_setters.parseWithinIFrame,
     'parseWrite': input.parseWrite,
     'parseWriteInto': input.parseWriteInto,
 };
diff --git a/src/commands/context_setters.js b/src/commands/context_setters.js
@@ -213,6 +213,31 @@ if (oldValue !== true) {
     };
 }
 
+// Possible inputs:
+//
+// * (selector, block)
+function parseWithinIFrame(parser) {
+    const ret = validator(parser,
+        {
+            kind: 'tuple',
+            elements: [
+                {
+                    kind: 'selector',
+                },
+                {
+                    kind: 'block',
+                },
+            ],
+        },
+    );
+    if (hasError(ret)) {
+        return ret;
+    }
+    return {
+        'instructions': [],
+    };
+}
+
 module.exports = {
     'parseDebug': parseDebug,
     'parseExpectFailure': parseExpectFailure,
@@ -223,4 +248,5 @@ module.exports = {
     'parseScreenshotOnFailure': parseScreenshotOnFailure,
     'parseShowText': parseShowText,
     'parseSetTimeout': parseSetTimeout,
+    'parseWithinIFrame': parseWithinIFrame,
 };
diff --git a/src/commands/functions.js b/src/commands/functions.js
@@ -99,9 +99,30 @@ the \`define-function\` command`,
 ${plural('argument', expected_args)}, found ${args.length}`,
         };
     }
+
+    const funcArgs = Object.create(null);
+    const func = parser.definedFunctions[func_name];
+    for (const arg_name of func['arguments']) {
+        const index = args.findIndex(arg => arg.key.value === arg_name);
+        if (index === -1) {
+            return {
+                'error': `Missing argument "${arg_name}"`,
+                'line': parser.get_current_command_line(),
+                'fatal_error': true,
+            };
+        }
+        funcArgs[arg_name] = args[index].value;
+    }
+    const context = parser.get_current_context();
+    parser.pushNewContext({
+        'ast': context.ast,
+        'commands': func.commands,
+        'currentCommand': 0,
+        'functionArgs': Object.assign({}, context.functionArgs, funcArgs),
+        'filePath': func.filePath,
+    });
     return {
-        'function': func_name,
-        'args': args,
+        'skipInstructions': true,
     };
 }
 
diff --git a/src/commands/general.js b/src/commands/general.js
@@ -1,5 +1,6 @@
 // Contains commands which don't really have a "category".
 
+const { AstLoader } = require('../ast.js');
 const path = require('path');
 const { getAndSetElements, indentString } = require('./utils.js');
 const { validator } = require('../validator.js');
@@ -224,7 +225,22 @@ function parseInclude(parser) {
         return ret;
     }
 
-    return {'path': ret.value.value };
+    const includePath = ret.value.value;
+    const dirPath = path.dirname(parser.get_current_context().ast.absolutePath);
+    const ast = new AstLoader(includePath, dirPath);
+    if (ast.hasErrors()) {
+        return {'errors': ast.errors};
+    }
+    parser.pushNewContext({
+        'ast': ast,
+        'commands': ast.commands,
+        'currentCommand': 0,
+        'functionArgs': Object.create(null),
+    });
+
+    return {
+        'skipInstructions': true,
+    };
 }
 
 module.exports = {
diff --git a/tests/api-output/parseInclude/basic-1.toml b/tests/api-output/parseInclude/basic-1.toml
diff --git a/tests/api-output/parseInclude/basic-2.toml b/tests/api-output/parseInclude/basic-2.toml
diff --git a/tests/api-output/parseWithinIFrame/basic-3.toml b/tests/api-output/parseWithinIFrame/basic-3.toml
@@ -0,0 +1,11 @@
+instructions = [
+  """const iframe = await page.$(\"::-p-xpath(//a)\");
+if (iframe === null) { throw '\"//a\" not found'; }
+await iframe.evaluate(el => {
+    if (el.tagName !== \"IFRAME\") {
+        throw \"selector `//a` is not an `<iframe>` but a `<\" + el.tagName.toLowerCase() + \">`\";
+    }
+});
+pages.push(iframe);""",
+]
+noPosIncrease = true
diff --git a/tests/ui/device-pixel-ratio.output b/tests/ui/device-pixel-ratio.output
@@ -1,6 +1,6 @@
 => Starting doc-ui tests...
 
 device-pixel-ratio... FAILED
-[ERROR] `tests/ui/device-pixel-ratio.goml` line 7: expected numbers above 0, found `-2` (from command `-2`)
+[ERROR] `tests/ui/device-pixel-ratio.goml` line 7: expected numbers above 0, found `-2` (from command `set-device-pixel-ratio: -2`)
 
 <= doc-ui tests done: 0 succeeded, 1 failed
diff --git a/tests/ui/function-named-map-extra-argument.output b/tests/ui/function-named-map-extra-argument.output
@@ -1,6 +1,6 @@
 => Starting doc-ui tests...
 
 function-named-map-extra-argument... FAILED
-[ERROR] `tests/ui/function-named-map-extra-argument.goml` line 14: function `fn1` expected 2 arguments, found 3 (from command `("fn1", {"background_color": "a", "whole_check": {"color": "a"}, "k": {"color": "a"}})`)
+[ERROR] `tests/ui/function-named-map-extra-argument.goml` line 14: function `fn1` expected 2 arguments, found 3 (from command `call-function: ("fn1", {"background_color": "a", "whole_check": {"color": "a"}, "k": {"color": "a"}})`)
 
 <= doc-ui tests done: 0 succeeded, 1 failed
diff --git a/tests/ui/functions.output b/tests/ui/functions.output
@@ -18,6 +18,6 @@ functions... FAILED
     from `tests/ui/functions.goml` line 35: The following errors happened (for selector `header`): [expected `a` for key `background-color`, found `rgb(17, 17, 17)`]: for command `assert-css: ("header", {"background-color": |background_color|})`
 [ERROR] `tests/ui/functions.goml` line 31
     from `tests/ui/functions.goml` line 35: The following errors happened (for selector `header`): [expected `a` for key `color`, found `rgb(255, 255, 255)`]: for command `assert-css: ("header", |whole_check|)`
-[ERROR] `tests/ui/functions.goml` line 37: Missing argument "whole_check"
+[ERROR] `tests/ui/functions.goml` line 37: Missing argument "whole_check" (from command `call-function: ("fn1", {"background_color": "a", "k": {"color": "a"}})`)
 
 <= doc-ui tests done: 0 succeeded, 1 failed
diff --git a/tests/ui/size.output b/tests/ui/size.output
@@ -1,6 +1,6 @@
 => Starting doc-ui tests...
 
 size... FAILED
-[ERROR] `tests/ui/size.goml` line 7: expected only positive numbers, found `-10` (first element of the tuple) (from command `(-10, 0.2)`)
+[ERROR] `tests/ui/size.goml` line 7: expected only positive numbers, found `-10` (first element of the tuple) (from command `set-window-size: (-10, 0.2)`)
 
 <= doc-ui tests done: 0 succeeded, 1 failed
diff --git a/tools/api.js b/tools/api.js
@@ -963,8 +963,10 @@ call-function: ("hello",{})`,
     // Running `define-function`.
     x.assert(pcontext.get_next_command().error === undefined);
     // Running `call-function`.
-    x.assertError(pcontext.get_next_command(),
-        'function `hello` expected 1 argument, found 0 (from command `("hello",{})`)',
+    x.assertError(
+        pcontext.get_next_command(),
+        'function `hello` expected 1 argument, found 0 (from command `call-function: \
+("hello",{})`)',
     );
 
     function callFunc(x, data, toCheck) {
@@ -991,29 +993,7 @@ call-function: ("hello",{})`,
 define-function: ("hello", [a], block {})
 call-function: ("hello",{"a": "1"})`,
         {
-            function: 'hello',
-            args: [
-                {
-                    key: {
-                        kind: 'string',
-                        value: 'a',
-                        startPos: 67,
-                        endPos: 70,
-                        fullText: '"a"',
-                        line: 2,
-                        error: null,
-                    },
-                    value: {
-                        kind: 'string',
-                        value: '1',
-                        startPos: 72,
-                        endPos: 75,
-                        fullText: '"1"',
-                        line: 2,
-                        error: null,
-                    },
-                },
-            ],
+            'skipInstructions': true,
         },
     );
     callFunc(
@@ -1022,29 +1002,7 @@ call-function: ("hello",{"a": "1"})`,
 define-function: ("hello", [a], block {})
 call-function: ("hello",{"a": "1"})`,
         {
-            function: 'hello',
-            args: [
-                {
-                    key: {
-                        kind: 'string',
-                        value: 'a',
-                        startPos: 67,
-                        endPos: 70,
-                        fullText: '"a"',
-                        line: 2,
-                        error: null,
-                    },
-                    value: {
-                        kind: 'string',
-                        value: '1',
-                        startPos: 72,
-                        endPos: 75,
-                        fullText: '"1"',
-                        line: 2,
-                        error: null,
-                    },
-                },
-            ],
+            'skipInstructions': true,
         },
     );
 }
@@ -1434,9 +1392,6 @@ function checkHistory(x, func) {
 function checkInclude(x, func) {
     func('-12', 'err-1');
     func('(-12, "a")', 'err-2');
-
-    func('"a"', 'basic-1');
-    func('"/a"', 'basic-2');
 }
 
 function checkJavascript(x, func) {
