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

core: extract BootupTime task summary methods #13971

Merged
merged 2 commits into from
May 6, 2022
Merged

Conversation

brendankenny
Copy link
Member

part of #13916

The responsiveness debugging audit will be the fourth audit using these functions, so it's just about time we spin them off into their own file.

They're not quite main-thread-task functions, nor are they just about task-groups, so a new file seemed appropriate. I'm not attached to their new location, however, so I'm happy to bikeshed.

@brendankenny brendankenny requested a review from a team as a code owner May 6, 2022 17:09
@brendankenny brendankenny requested review from adamraine and removed request for a team May 6, 2022 17:09
return MainThreadTasks.getMainThreadTasks(mainThreadEvents, frames, timestamps.traceEnd);
}

describe('Task Summaries', () => {
Copy link
Member Author

Choose a reason for hiding this comment

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

these functions are quite well tested by the existing BootupTime et al tests, but only as integrated into the audit results, no unit tests, so it seemed appropriate to give some basic coverage since they're often used as units.

it('returns the script URLs from a set of network records', () => {
const records = NetworkRecorder.recordsFromLogs(ampDevtoolsLog);
const urls = getJavaScriptURLs(records);
expect(urls.size).toEqual(13);
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: when the size of an array changes it creates very unhelpful test failures messages. maybe just move this length check to after the for loop... or change toMatch to check an array in one-go?

Copy link
Member Author

Choose a reason for hiding this comment

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

SG. It shouldn't ever change (until the trace does) but it would be more helpful.

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

Successfully merging this pull request may close these issues.

ยุทธยง วงศ์แสงมณี
4 participants