Skip to content

Cleanup/shared #513

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions resources/shared/benchmark.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export class BenchmarkStep {
this.run = run;
}

async runAndRecord(params, suite, test, callback) {
async runAndRecordStep(params, suite, test, callback) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this one be "Test" instead of "Step"??

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After discussing this: let's incrementally move towards using "steps" everywhere.

const testRunner = new TestRunner(null, null, params, suite, test, callback);
const result = await testRunner.runTest();
return result;
Expand All @@ -26,9 +26,9 @@ export class BenchmarkStep {
* A single test suite that contains one or more test steps.
*/
export class BenchmarkSuite {
constructor(name, tests) {
constructor(name, steps) {
this.name = name;
this.tests = tests;
this.steps = steps;
}

record(_test, syncTime, asyncTime) {
Expand All @@ -41,7 +41,7 @@ export class BenchmarkSuite {
return results;
}

async runAndRecord(params, onProgress) {
async runAndRecordSuite(params, onProgress) {
const measuredValues = {
tests: {},
total: 0,
Expand All @@ -51,11 +51,11 @@ export class BenchmarkSuite {

performance.mark(suiteStartLabel);

for (const test of this.tests) {
const result = await test.runAndRecord(params, this, test, this.record);
measuredValues.tests[test.name] = result;
for (const step of this.steps) {
const result = await step.runAndRecordStep(params, this, step, this.record);
measuredValues.tests[step.name] = result;
measuredValues.total += result.total;
onProgress?.(test.name);
onProgress?.(step.name);
}

performance.mark(suiteEndLabel);
Expand Down Expand Up @@ -103,7 +103,7 @@ export class BenchmarkConnector {
const suite = this.suites[event.data.name];
if (!suite)
console.error(`Suite with the name of "${event.data.name}" not found!`);
const { result } = await suite.runAndRecord(params, (test) => this.sendMessage({ type: "step-complete", status: "success", appId: this.appId, name: this.name, test }));
const { result } = await suite.runAndRecordSuite(params, (test) => this.sendMessage({ type: "step-complete", status: "success", appId: this.appId, name: this.name, test }));
this.sendMessage({ type: "suite-complete", status: "success", appId: this.appId, result });
this.disconnect();
break;
Expand Down
39 changes: 32 additions & 7 deletions resources/shared/helpers.mjs
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the new comments, that's super useful!

Original file line number Diff line number Diff line change
@@ -1,28 +1,53 @@
/**
* Helper Methods
* Recursively queries the DOM for an element, traversing through shadow DOMs.
*
* Various methods that are extracted from the Page class.
* @param {Element|ShadowRoot} lookupStartNode The node or shadow root to start the lookup from.
* @param {string[]} path An array of CSS selectors representing the path to the target element.
* @returns {Element|null} The target element if found, otherwise null.
*/
export function getParent(lookupStartNode, path) {
export function recursivelyQuerySelector(lookupStartNode, path) {
lookupStartNode = lookupStartNode.shadowRoot ?? lookupStartNode;
const parent = path.reduce((root, selector) => {
const target = path.reduce((root, selector) => {
const node = root.querySelector(selector);
return node.shadowRoot ?? node;
}, lookupStartNode);

return parent;
return target;
}

/**
* Retrieves a single DOM element, optionally traversing through shadow DOMs to a specified path before the final selection.
*
* @param {string} selector The CSS selector for the desired element.
* @param {string[]} [path=[]] An optional array of CSS selectors to reach the desired shadowRoot or parent element.
* @param {Element|ShadowRoot} [lookupStartNode=document] The starting node for the lookup.
* @returns {Element|null} The found element, or null if not found.
*/
export function getElement(selector, path = [], lookupStartNode = document) {
const element = getParent(lookupStartNode, path).querySelector(selector);
const element = recursivelyQuerySelector(lookupStartNode, path).querySelector(selector);
return element;
}

/**
* Retrieves all DOM elements matching a selector, optionally traversing through shadow DOMs to a specified path before the final selection.
*
* @param {string} selector The CSS selector for the desired elements.
* @param {string[]} [path=[]] An optional array of CSS selectors to reach the desired shadowRoot or parent element.
* @param {Element|ShadowRoot} [lookupStartNode=document] The starting node for the lookup.
* @returns {Element[]} An array of found elements.
*/
export function getAllElements(selector, path = [], lookupStartNode = document) {
const elements = Array.from(getParent(lookupStartNode, path).querySelectorAll(selector));
const elements = Array.from(recursivelyQuerySelector(lookupStartNode, path).querySelectorAll(selector));
return elements;
}

/**
* Forces a reflow/layout of the document by requesting a DOM property that triggers it.
* This can be useful for ensuring that styles and positions are up-to-date before
* performing measurements or animations. It also returns an element from the center of the viewport.
*
* @returns {Element|null} An element at the center of the viewport, or null if no element is there.
*/
export function forceLayout() {
const rect = document.body.getBoundingClientRect();
const e = document.elementFromPoint((rect.width / 2) | 0, (rect.height / 2) | 0);
Expand Down
6 changes: 2 additions & 4 deletions resources/shared/test-invoker.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@ class TestInvoker {
this._reportCallback = reportCallback;
this._params = params;
}
}

class BaseRAFTestInvoker extends TestInvoker {
start() {
return new Promise((resolve) => {
if (this._params.waitBeforeSync)
Expand All @@ -18,7 +16,7 @@ class BaseRAFTestInvoker extends TestInvoker {
}
}

class RAFTestInvoker extends BaseRAFTestInvoker {
class RAFTestInvoker extends TestInvoker {
_scheduleCallbacks(resolve) {
requestAnimationFrame(() => this._syncCallback());
requestAnimationFrame(() => {
Expand All @@ -33,7 +31,7 @@ class RAFTestInvoker extends BaseRAFTestInvoker {
}
}

class AsyncRAFTestInvoker extends BaseRAFTestInvoker {
class AsyncRAFTestInvoker extends TestInvoker {
static mc = new MessageChannel();
_scheduleCallbacks(resolve) {
let gotTimer = false;
Expand Down