Skip to content

JS Streamer and Frontend Tests#266

Merged
mcottontensor merged 32 commits intoEpicGamesExt:masterfrom
mcottontensor:js-streamer
Sep 2, 2024
Merged

JS Streamer and Frontend Tests#266
mcottontensor merged 32 commits intoEpicGamesExt:masterfrom
mcottontensor:js-streamer

Conversation

@mcottontensor
Copy link
Copy Markdown
Collaborator

JS Streamer in Extras/JSStreamer
Frontend tests in Extras/FrontendTests

// sets up the streamer page to capture data channel messages
export function setupEventCapture(streamerPage: Page) {
return streamerPage.evaluate(() => {
window.data_messages = {};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe small comment here about attaching this js object and the listener to populate it to the window as (I imagine) this is the most convenient place that is visible to both caller and callee.

}
}

public normalizeQuantizeSigned(x: number, y: number): Coord {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could be potential to move somethings into common here and have both frontend and extra use it?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yeah I was thinking the same thing as I was doing this. I feel like the input stuff on frontend could be cleaned up a lot.

await page.getByText('Click to start').click();
await helpers.waitForVideo(page);

const player_box = await page.locator('#videoElementParent').boundingBox();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Any particular reason for underscored_variables - seems a departure from the rest of the codebase?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It was picked up from somewhere. Maybe when I started doing playwright tests, and was trying to be consistent and it became a habit when moving into this code base.
The joys of trying to be consistent with everyone.

// check we got the expected events
const first_player_id = Object.keys(events)[0];
const single_player_events = events[first_player_id];
expect(single_player_events).toContainActions(expected_actions);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

That's nice!

@mcottontensor mcottontensor marked this pull request as ready for review September 2, 2024 05:17
@mcottontensor mcottontensor merged commit 290bced into EpicGamesExt:master Sep 2, 2024
@mcottontensor mcottontensor deleted the js-streamer branch September 2, 2024 05:29
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