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

tests: check for console errors and warnings in pptr tests #15516

Merged
merged 4 commits into from
Jan 16, 2024
Merged

tests: check for console errors and warnings in pptr tests #15516

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
05b8634
tests: check for console errors and warnings in pptr tests
connorjclark Oct 4, 2023
6ab28ff
do for treemap test
connorjclark Oct 4, 2023
562fc60
do for extension
connorjclark Oct 4, 2023
f791081
Merge remote-tracking branch 'origin/main' into ppt-errors
connorjclark Jan 16, 2024
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
32 changes: 28 additions & 4 deletions clients/test/extension/popup-test-pptr.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,20 @@ describe('Lighthouse chrome popup', function() {

let browser;
let page;
const pageErrors = [];
let pageErrors = [];

async function claimErrors() {
const theErrors = pageErrors;
pageErrors = [];
return await Promise.all(theErrors);
}

async function ensureNoErrors() {
await page.bringToFront();
await page.evaluate(() => new Promise(window.requestAnimationFrame));
const errors = await claimErrors();
expect(errors).toHaveLength(0);
}

before(async function() {
// start puppeteer
Expand All @@ -43,8 +56,19 @@ describe('Lighthouse chrome popup', function() {
});

page = await browser.newPage();
page.on('pageerror', err => {
pageErrors.push(err);
page.on('pageerror', e => pageErrors.push(`${e.message} ${e.stack}`));
page.on('console', (e) => {
if (e.type() === 'error' || e.type() === 'warning') {
const describe = (jsHandle) => {
return jsHandle.executionContext().evaluate((obj) => {
return JSON.stringify(obj, null, 2);
}, jsHandle);
};
const promise = Promise.all(e.args().map(describe)).then(args => {
return `${e.text()} ${args.join(' ')} ${JSON.stringify(e.location(), null, 2)}`;
});
pageErrors.push(promise);
}
});
await page.evaluateOnNewDocument((mockStorage) => {
Object.defineProperty(chrome, 'tabs', {
Expand Down Expand Up @@ -85,7 +109,7 @@ describe('Lighthouse chrome popup', function() {
});

it('should load without errors', async function() {
expect(pageErrors).toHaveLength(0);
await ensureNoErrors();
});

it('should generate the category checkboxes', async function() {
Expand Down
34 changes: 28 additions & 6 deletions treemap/test/treemap-test-pptr.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,16 +55,38 @@ describe('Lighthouse Treemap', () => {
});
}
page = await browser.newPage();
page.on('pageerror', pageError => pageErrors.push(pageError));
page.on('pageerror', e => pageErrors.push(`${e.message} ${e.stack}`));
page.on('console', (e) => {
if (e.type() === 'error' || e.type() === 'warning') {
const describe = (jsHandle) => {
return jsHandle.executionContext().evaluate((obj) => {
return JSON.stringify(obj, null, 2);
}, jsHandle);
};
const promise = Promise.all(e.args().map(describe)).then(args => {
return `${e.text()} ${args.join(' ')} ${JSON.stringify(e.location(), null, 2)}`;
});
pageErrors.push(promise);
}
});
});

async function claimErrors() {
const theErrors = pageErrors;
pageErrors = [];
return await Promise.all(theErrors);
}

async function ensureNoErrors() {
await page.bringToFront();
await page.evaluate(() => new Promise(window.requestAnimationFrame));
const errors = await claimErrors();
expect(errors).toHaveLength(0);
}

afterEach(async () => {
await ensureNoErrors();
await page.close();

// Fails if any unexpected errors ocurred.
// If a test expects an error, it will clear this array.
expect(pageErrors).toMatchObject([]);
pageErrors = [];
});

describe('Recieves options', () => {
Expand Down
6 changes: 5 additions & 1 deletion viewer/app/src/psi-api.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,11 @@ class PSIApi {
apiUrl.searchParams.append('category', singleCategory);
}
apiUrl.searchParams.append('key', PSI_KEY);
return fetch(apiUrl.href).then(res => res.json());
return fetch(apiUrl.href, {
headers: {
referer: 'googlechrome.github.io',
},
}).then(res => res.json());
}
}

Expand Down
2 changes: 2 additions & 0 deletions viewer/app/src/viewer-ui-features.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ export class ViewerUIFeatures extends ReportUIFeatures {
}

this._getI18nModule().then(async (i18nModule) => {
if (!report.i18n?.icuMessagePaths) return;

Copy link
Collaborator Author

@connorjclark connorjclark Oct 4, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

older LHRs were erroring, but in a non-fatal way since this is a loose promise.

I didn't prevent the loading of the i18n module because the test would need to exclude the warning Chrome emits on an unused preload directive, and this being a minor optimization for ancient LHRs I'd rather not further complicate the tests.

const locales = /** @type {LH.Locale[]} */ (
await i18nModule.format.getCanonicalLocales());
this._swapLocales.enable(locales);
Expand Down
45 changes: 34 additions & 11 deletions viewer/test/viewer-test-pptr.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,25 @@ describe('Lighthouse Viewer', () => {
executablePath: getChromePath(),
});
viewerPage = await browser.newPage();
viewerPage.on('pageerror', pageError => pageErrors.push(pageError));
viewerPage.on('pageerror', e => pageErrors.push(`${e.message} ${e.stack}`));
viewerPage.on('console', (e) => {
if (e.type() === 'error' || e.type() === 'warning') {
// TODO gotta upgrade our own stuff.
if (e.text().includes('Please adopt the new report API')) return;
// Rendering a report from localhost page will attempt to display unreachable resources.
if (e.location().url.includes('lighthouse-480x318.jpg')) return;

const describe = (jsHandle) => {
return jsHandle.executionContext().evaluate((obj) => {
return JSON.stringify(obj, null, 2);
}, jsHandle);
};
const promise = Promise.all(e.args().map(describe)).then(args => {
return `${e.text()} ${args.join(' ')} ${JSON.stringify(e.location(), null, 2)}`;
});
pageErrors.push(promise);
}
});
});

after(async function() {
Expand All @@ -81,16 +99,17 @@ describe('Lighthouse Viewer', () => {
]);
});

beforeEach(async function() {
async function claimErrors() {
const theErrors = pageErrors;
pageErrors = [];
});
return await Promise.all(theErrors);
}

async function ensureNoErrors() {
await viewerPage.bringToFront();
await viewerPage.evaluate(() => new Promise(window.requestAnimationFrame));
const theErrors = pageErrors;
pageErrors = [];
expect(theErrors).toHaveLength(0);
const errors = await claimErrors();
expect(errors).toHaveLength(0);
}

afterEach(async function() {
Expand Down Expand Up @@ -262,7 +281,7 @@ describe('Lighthouse Viewer', () => {

const savedPage = await browser.newPage();
const savedPageErrors = [];
savedPage.on('pageerror', pageError => savedPageErrors.push(pageError));
savedPage.on('pageerror', e => savedPageErrors.push(`${e.message} ${e.stack}`));
connorjclark marked this conversation as resolved.
Show resolved Hide resolved
const firstLogPromise =
new Promise(resolve => savedPage.once('console', e => resolve(e.text())));
await savedPage.goto(`file://${tmpDir}/${filename}`);
Expand All @@ -282,6 +301,8 @@ describe('Lighthouse Viewer', () => {
waitForAck,
new Promise((resolve, reject) => setTimeout(reject, 5_000)),
]);
// Give async work some time to happen (ex: SwapLocaleFeature.enable).
await new Promise(resolve => setTimeout(resolve, 3_000));
await ensureNoErrors();

const content = await viewerPage.$eval('main', el => el.textContent);
Expand Down Expand Up @@ -466,10 +487,12 @@ describe('Lighthouse Viewer', () => {
const errorMessage = await viewerPage.evaluate(errorEl => errorEl.textContent, errorEl);
expect(errorMessage).toBe('badPsiResponse error');

// One error.
expect(pageErrors).toHaveLength(1);
expect(pageErrors[0].message).toContain('badPsiResponse error');
pageErrors = [];
// Expected errors.
const errors = await claimErrors();
expect(errors).toHaveLength(3);
Copy link
Member

Choose a reason for hiding this comment

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

Are these new errors expected because of the new timeout above?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, they have always been printed to the console. Now the tests are checking console errors in addition to pageerror.

expect(errors[0]).toContain('500 (Internal Server Error)');
expect(errors[1]).toContain('badPsiResponse error');
expect(errors[2]).toContain('badPsiResponse error');
});
});
});