Return structured objects from parsers instead of concatenated JS code

[notriddle]

instructions should, instead of containing the JavaScript code, contain objects that get serialized on the harness side and deserialized in the browser. For example, parseAssertCssInner is currently implemented like this:

browser-UI-test/src/commands/assert.js

Lines 103 to 142 in 09259bf

	instructions.push(`\
	const { checkCssProperty } = require('command-helpers.js');
	async function checkElem(elem) {
	const nonMatchingProps = [];
	const jsHandle = await elem.evaluateHandle(e => {
	const ${varDict} = [${keys}];
	const assertComputedStyle = window.getComputedStyle(e${pseudo});
	const simple = [];
	const computed = [];
	const keys = [];
	for (const entry of ${varDict}) {
	simple.push(e.style[entry]);
	computed.push(assertComputedStyle[entry]);
	keys.push(entry);
	}
	return [keys, simple, computed];
	});
	const [keys, simple, computed] = await jsHandle.jsonValue();
	const values = [${values}];
	for (const [i, key] of keys.entries()) {
	const localErr = [];
	checkCssProperty(key, values[i], simple[i], computed[i], localErr);
	${indentString(assertCheck, 3)}
	}
	if (nonMatchingProps.length !== 0) {
	const props = nonMatchingProps.join("; ");
	throw "The following errors happened (for ${xpath}\`${selector.value}\`): [" + props + "]";
	}
	}
	${getAndSetElements(selector, varName, checkAllElements)}
	${extra}`);
	return {
	'instructions': instructions,
	'wait': false,
	'checkResult': true,
	};

The idea is that, instead of returning actual JS code, the parser returns a plain old javascript object:

return {
    'instructions': {
        'type': 'assertCssInner',
        checkAllElements,
        varName,
        assertFalse,
        propertyDict,
        varDict,
        selector,
    },
    'wait': false,
    'checkResult': true,
};

And then, on the browser side, the actual implementation is a simple function that contains most of what the parser currently returns.

browserUITestInstructions = {};
function dispatch(instruction) {
    return browserUITestInstructions[instruction.type](instruction);
}
browserUITestInstructions.assertCssInner = function({
    checkAllElements,
    varName,
    assertFalse,
    propertyDict,
    varDict,
    selector,
}) {
    const { checkCssProperty } = require('command-helpers.js');
    async function checkElem(elem) {
        const nonMatchingProps = [];
        const jsHandle = await elem.evaluateHandle(e => {
            const varDict = [...propertyDict.keys()];
            const pseudo = !selector.isXPath && selector.pseudo !== null ? selector.pseudo : undefined;
            const assertComputedStyle = window.getComputedStyle(e, pseudo);
            const simple = [];
            const computed = [];
            const keys = [];

           for (const entry of varDict) {
                simple.push(e.style[entry]);
                computed.push(assertComputedStyle[entry]);
                keys.push(entry);
            }
            return [keys, simple, computed];
        });
        const [keys, simple, computed] = await jsHandle.jsonValue();
        const values = [...propertyDict.values()];

        for (const [i, key] of keys.entries()) {
            const localErr = [];
            checkCssProperty(key, values[i], simple[i], computed[i], localErr);
            if (assertFalse) {
                if (localErr.length === 0) {
                    nonMatchingProps.push("assert didn't fail for key `" + key + '`');
                }
            } else {
                nonMatchingProps.push(...localErr);
            }
        }
        if (nonMatchingProps.length !== 0) {
            const props = nonMatchingProps.join("; ");
            const xpath = selector.isXPath ? 'XPath ' : 'selector ';
            throw "The following errors happened (for ${xpath}\`${selector.value}\`): [" + props + "]";
        }
    }
    // deal with getAndSetElements and extras here
}

This means all functions are exported and used by puppeteer directly, and the test harness merely sends serialized POJOs for each instruction. This means sending less code, but more importantly means responsibility for string escaping can be concentrated in one place (you're less likely to invite Bobby Tables into your test case).