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.

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

Conversation

camillobruni
Copy link
Contributor

@camillobruni camillobruni commented Jan 28, 2025

Add more test logging to make it easier to debug a failing test.

  • Log setup status
  • Log incremental browser results
  • Add JetStream.isDone for easier test handling
  • Add JetStream.incrementalResults so we can easily access the results (firefox doesn't easily allow us to modify global state via selenium it seems)

We're currently still lacking basic result validation and checking whether any errors have been thrown.

}

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants