Skip to content
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.

Sign up for GitHub

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

Improve end2end runner log #41

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Improve end2end runner log #41

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
1a02912
wipi
camillobruni Jan 28, 2025
4634f88
reverti
camillobruni Jan 28, 2025
10ba50a
cleanup
camillobruni Jan 28, 2025
5fd1b6f
further cleanup
camillobruni Jan 28, 2025
6f59500
more cleanup
camillobruni Jan 28, 2025
4629828
cleanup
camillobruni Jan 28, 2025
1b2e81c
Merge branch 'main' into 2025-01-28_runner_log
camillobruni Jan 28, 2025
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
17 changes: 17 additions & 0 deletions JetStreamDriver.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,8 @@ const fileLoader = (function() {
class Driver {
constructor() {
this.isReady = false;
this.isDone = false;
this.incrementalResults = [];
this.benchmarks = [];
this.blobDataCache = { };
this.loadCache = { };
Expand Down Expand Up @@ -279,8 +281,16 @@ class Driver {
}

benchmark.updateUIAfterRun();
console.log(benchmark.name)

if (isInBrowser) {
this.incrementalResults.push({
name: benchmark.name,
results: {
Score: benchmark.score,
...benchmark.subScores(),
}
});
const cache = JetStream.blobDataCache;
for (const file of benchmark.plan.files) {
const blobData = cache[file];
Expand Down Expand Up @@ -334,13 +344,20 @@ class Driver {

this.reportScoreToRunBenchmarkRunner();
this.dumpJSONResultsIfNeeded();
this.isDone = true;
if (isInBrowser) {
globalThis.dispatchEvent(new CustomEvent("JetStreamDone", {
detail: this.resultsObject()
}));
}
}

drainIncrementalResults() {
const currentIncrementalResults = this.incrementalResults;
this.incrementalResults = [];
return currentIncrementalResults
}

runCode(string)
{
if (!isInBrowser) {
Expand Down
51 changes: 40 additions & 11 deletions tests/run.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import serve from "./server.mjs";
import { Builder, Capabilities } from "selenium-webdriver";
import commandLineArgs from "command-line-args";
import commandLineUsage from "command-line-usage";
import assert from "assert";

const optionDefinitions = [
{ name: "browser", type: String, description: "Set the browser to test, choices are [safari, firefox, chrome]. By default the $BROWSER env variable is used." },
Expand Down Expand Up @@ -81,24 +80,19 @@ async function testEnd2End() {
const driver = await new Builder().withCapabilities(capabilities).build();
let results;
try {
await driver.get(`http://localhost:${PORT}/index.html?worstCaseCount=2&iterationCount=3`);
const url = `http://localhost:${PORT}/index.html?worstCaseCount=2&iterationCount=3`;
console.log(`JetStream PREPARE ${url}`);
await driver.get(url);
await driver.executeAsyncScript((callback) => {
// callback() is explicitly called without the default event
// as argument to avoid serialization issues with chromedriver.
globalThis.addEventListener("JetStreamReady", () => callback());
// We might not get a chance to install the on-ready listener, thus
// we also check if the runner is ready synchronously.
if (globalThis?.JetStream?.isReady)
callback()
});
await driver.manage().setTimeouts({ script: 3 * 60_000 });
results = await driver.executeAsyncScript((callback) => {
globalThis.addEventListener("JetStreamDone", event => callback(event.detail));
JetStream.start();
});
results = await benchmarkResults(driver);
// FIXME: validate results;
console.log("\n✅ Tests completed!");
console.log("RESULTS:")
console.log(results)
} catch(e) {
console.error("\n❌ Tests failed!");
console.error(e);
Expand All @@ -109,4 +103,39 @@ async function testEnd2End() {
}
}

async function benchmarkResults(driver) {
console.log("JetStream START");
await driver.manage().setTimeouts({ script: 60_000 });
await driver.executeAsyncScript((callback) => {
globalThis.JetStream.start();
callback();
});
await new Promise(resolve => pollIncrementalResults(driver, resolve));
const resultString = await driver.executeScript(() => {
return JSON.stringify(globalThis.JetStream.resultsObject());
});
return JSON.parse(resultString);
}

const UPDATE_INTERVAL = 250;
async function pollIncrementalResults(driver, resolve) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why poll the incremental results rather than have the driver push them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How did you imagine pushing them from the page to the outer test runner?

I tried to just forward console.log to the outer test runner (this would have been the easiest solution IMO) but somehow that didn't work with selenium (most likely I'm missing something here, but I gave up and resorted to the current approach).

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure how to do it in Selenium. I'll ask internally though.

One option would be to have the JetStreamDriver send POST requests to server.mjs (like reportScoreToRunBenchmarkRunner) and either have the server log those messages or have the runner provide a callback to the server.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I can read console.log messages consistently that would be the prefered solution 👍 .

As for server logging: I could give a custom lws middleware a shot that just prints a posted json payload

Copy link
Contributor

Choose a reason for hiding this comment

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

Unfortunately, it doesn't look like Safari has a way to get the console.log messages. If reading an HTTP POST is annoying you could make a "JetStreamConsoleLog" custom event and do

const originalLog  = console.log.bind(console);
console.log = function log() {
    globalThis.dispatchEvent(new CustomEvent("JetStreamConsoleLog", {
        detail: arguments.toString();
    }));
    originalLog.call(...arguments);
}

In JetStreamDriver.js

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can give it a shot again with listening to events. The last time I tried you basically don't get any callback within selenium, since the main message loop is fully saturated with jetstream. The main blocking issue was that I couldn't add new globals in Firefox, but I didn't try modifying existing objects.

Probably the custom post-message middleware for LWS is going to be easiest choice here here.

const intervalId = setInterval(async function logResult() {
const {done, results} = await driver.executeScript(() => {
return {
done: globalThis.JetStream.isDone,
results: JSON.stringify(globalThis.JetStream.drainIncrementalResults()),
};
});
JSON.parse(results).forEach(logIncrementalResult);
if (done) {
clearInterval(intervalId);
resolve()
}
}, UPDATE_INTERVAL)
}

function logIncrementalResult(benchmarkResult) {
console.log(benchmarkResult.name, benchmarkResult.results)
}

setImmediate(testEnd2End);