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

core(fr): add supportedModes filter to categories #13161

Merged
merged 8 commits into from
Oct 4, 2021
Merged

core(fr): add supportedModes filter to categories #13161

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
179f931
start
adamraine Oct 1, 2021
de84c2a
update flow
adamraine Oct 1, 2021
01a4f48
category test
adamraine Oct 1, 2021
458ced3
update sample
adamraine Oct 4, 2021
6b4e384
test
adamraine Oct 4, 2021
ab494d8
proto
adamraine Oct 4, 2021
8e8d70e
fix tests
adamraine Oct 4, 2021
467b56c
api test
adamraine Oct 4, 2021
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
10 changes: 2 additions & 8 deletions flow-report/src/summary/category.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,7 @@ export const SummaryTooltip: FunctionComponent<{
</div>
{
totalWeight !== 0 &&
<div
className={`SummaryTooltip__rating SummaryTooltip__rating--${rating}`}
data-testid="SummaryTooltip__rating"
>
<div className={`SummaryTooltip__rating SummaryTooltip__rating--${rating}`}>
<span>{getCategoryRating(rating, strings)}</span>
{
!displayAsFraction && category.score && <>
Expand Down Expand Up @@ -90,10 +87,7 @@ export const SummaryCategory: FunctionComponent<{
/>
<SummaryTooltip category={category} gatherMode={gatherMode}/>
</div> :
<div
className="SummaryCategory__null"
data-testid="SummaryCategory__null"
/>
<div className="SummaryCategory__null"/>
}
</div>
);
Expand Down
62 changes: 37 additions & 25 deletions flow-report/test/summary/category-test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@ import {FunctionComponent} from 'preact';
import {render} from '@testing-library/preact';

import {SummaryTooltip} from '../../src/summary/category';
import {Util} from '../../../report/renderer/util';
import {flowResult} from '../sample-flow';
import {I18nProvider} from '../../src/i18n/i18n';
import {FlowResultContext} from '../../src/util';

let wrapper: FunctionComponent;

beforeEach(() => {
// Include sample flowResult for locale in I18nProvider.
Copy link
Member Author

Choose a reason for hiding this comment

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

Drive by change to this test so the expectations don't change every time the sample is updated.

Copy link
Collaborator

Choose a reason for hiding this comment

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

quite a few lines for a drive by ;)

wrapper = ({children}) => (
<FlowResultContext.Provider value={flowResult}>
<I18nProvider>
Expand All @@ -27,50 +27,62 @@ beforeEach(() => {

describe('SummaryTooltip', () => {
it('renders tooltip with rating', async () => {
// Snapshot SEO
const reportResult = Util.prepareReportResult(flowResult.steps[2].lhr);
const category = reportResult.categories['seo'];
const category: any = {
score: 1,
auditRefs: [
{result: {score: 1, scoreDisplayMode: 'binary'}, weight: 1},
{result: {score: 1, scoreDisplayMode: 'binary'}, weight: 1},
{result: {score: 0, scoreDisplayMode: 'binary'}, weight: 1},
],
};

const root = render(
<SummaryTooltip category={category} gatherMode={reportResult.gatherMode}/>,
<SummaryTooltip category={category} gatherMode="snapshot"/>,
{wrapper}
);

const rating = root.getByTestId('SummaryTooltip__rating');
expect(rating.classList).toContain('SummaryTooltip__rating--average');

expect(root.getByText('9 audits passed / 11 audits run')).toBeTruthy();
expect(root.getByText('Average')).toBeTruthy();
expect(() => root.getByText(/^[0-9]+$/)).toThrow();
expect(root.getByText('2 audits passed / 3 audits run')).toBeTruthy();
});

it('renders tooltip without rating', async () => {
// Snapshot performance
const reportResult = Util.prepareReportResult(flowResult.steps[2].lhr);
const category = reportResult.categories['performance'];
const category: any = {
score: 1,
auditRefs: [
{result: {score: 1, scoreDisplayMode: 'binary'}, weight: 0},
{result: {score: 1, scoreDisplayMode: 'binary'}, weight: 0},
{result: {score: 0, scoreDisplayMode: 'binary'}, weight: 0},
],
};

const root = render(
<SummaryTooltip category={category} gatherMode={reportResult.gatherMode}/>,
<SummaryTooltip category={category} gatherMode="snapshot"/>,
{wrapper}
);

expect(() => root.getByTestId('SummaryTooltip__rating')).toThrow();
expect(root.getByText('2 audits passed / 4 audits run')).toBeTruthy();
expect(() => root.getByText(/^(Average|Good|Poor)$/)).toThrow();
expect(() => root.getByText(/^[0-9]+$/)).toThrow();
expect(root.getByText('2 audits passed / 3 audits run')).toBeTruthy();
});

it('renders scored category tooltip with score', async () => {
// Navigation performance
const reportResult = Util.prepareReportResult(flowResult.steps[0].lhr);
const category = reportResult.categories['performance'];
const category: any = {
score: 1,
auditRefs: [
{result: {score: 1, scoreDisplayMode: 'binary'}, weight: 1},
{result: {score: 1, scoreDisplayMode: 'binary'}, weight: 1},
{result: {score: 0, scoreDisplayMode: 'binary'}, weight: 1},
],
};

const root = render(
<SummaryTooltip category={category} gatherMode={reportResult.gatherMode}/>,
<SummaryTooltip category={category} gatherMode="navigation"/>,
{wrapper}

);

const rating = root.getByTestId('SummaryTooltip__rating');
expect(rating.classList).toContain('SummaryTooltip__rating--pass');
expect(rating.textContent).toEqual('Good · 94');

expect(root.getByText('41 audits passed / 58 audits run')).toBeTruthy();
expect(root.getByText('Good')).toBeTruthy();
expect(root.getByText('100')).toBeTruthy();
expect(root.getByText('2 audits passed / 3 audits run')).toBeTruthy();
});
});
5 changes: 5 additions & 0 deletions lighthouse-core/config/default-config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,7 @@ const defaultConfig = {
categories: {
'performance': {
title: str_(UIStrings.performanceCategoryTitle),
supportedModes: ['navigation', 'timespan', 'snapshot'],
auditRefs: [
{id: 'first-contentful-paint', weight: 10, group: 'metrics', acronym: 'FCP', relevantAudits: m2a.fcpRelevantAudits},
{id: 'speed-index', weight: 10, group: 'metrics', acronym: 'SI'},
Expand Down Expand Up @@ -490,6 +491,7 @@ const defaultConfig = {
title: str_(UIStrings.a11yCategoryTitle),
description: str_(UIStrings.a11yCategoryDescription),
manualDescription: str_(UIStrings.a11yCategoryManualDescription),
supportedModes: ['navigation', 'snapshot'],
// Audit weights are meant to match the aXe scoring system of
// minor, moderate, serious, and critical.
// See the audits listed at dequeuniversity.com/rules/axe/4.1.
Expand Down Expand Up @@ -554,6 +556,7 @@ const defaultConfig = {
},
'best-practices': {
title: str_(UIStrings.bestPracticesCategoryTitle),
supportedModes: ['navigation', 'timespan', 'snapshot'],
auditRefs: [
// Trust & Safety
{id: 'is-on-https', weight: 1, group: 'best-practices-trust-safety'},
Expand Down Expand Up @@ -584,6 +587,7 @@ const defaultConfig = {
title: str_(UIStrings.seoCategoryTitle),
description: str_(UIStrings.seoCategoryDescription),
manualDescription: str_(UIStrings.seoCategoryManualDescription),
supportedModes: ['navigation', 'snapshot'],
auditRefs: [
{id: 'viewport', weight: 1, group: 'seo-mobile'},
{id: 'document-title', weight: 1, group: 'seo-content'},
Expand All @@ -607,6 +611,7 @@ const defaultConfig = {
title: str_(UIStrings.pwaCategoryTitle),
description: str_(UIStrings.pwaCategoryDescription),
manualDescription: str_(UIStrings.pwaCategoryManualDescription),
supportedModes: ['navigation'],
auditRefs: [
// Installable
{id: 'installable-manifest', weight: 2, group: 'pwa-installable'},
Expand Down
24 changes: 21 additions & 3 deletions lighthouse-core/fraggle-rock/config/filters.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,23 @@ function filterAuditsByGatherMode(audits, mode) {
});
}

/**
* Optional `supportedModes` property can explicitly exclude a category even if some audits are available.
*
* @param {LH.Config.Config['categories']} categories
* @param {LH.Gatherer.GatherMode} mode
* @return {LH.Config.Config['categories']}
*/
function filterCategoriesByGatherMode(categories, mode) {
if (!categories) return null;

const categoriesToKeep = Object.entries(categories)
.filter(([_, category]) => {
return !category.supportedModes || category.supportedModes.includes(mode);
});
return Object.fromEntries(categoriesToKeep);
}

/**
* Filters a categories object and their auditRefs down to the specified category ids.
*
Expand Down Expand Up @@ -225,9 +242,10 @@ function filterCategoriesByAvailableAudits(categories, availableAudits) {
*/
function filterConfigByGatherMode(config, mode) {
const artifacts = filterArtifactsByGatherMode(config.artifacts, mode);
const availableAudits = filterAuditsByAvailableArtifacts(config.audits, artifacts || []);
const audits = filterAuditsByGatherMode(availableAudits, mode);
const categories = filterCategoriesByAvailableAudits(config.categories, audits || []);
const supportedAudits = filterAuditsByGatherMode(config.audits, mode);
const audits = filterAuditsByAvailableArtifacts(supportedAudits, artifacts || []);
const supportedCategories = filterCategoriesByGatherMode(config.categories, mode);
const categories = filterCategoriesByAvailableAudits(supportedCategories, audits || []);

return {
...config,
Expand Down
5 changes: 4 additions & 1 deletion lighthouse-core/scripts/update-flow-fixtures.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@ import {LH_ROOT} from '../../root.js';
import UserFlow from '../fraggle-rock/user-flow.js';

(async () => {
const browser = await puppeteer.launch({headless: false});
const browser = await puppeteer.launch({
executablePath: process.env.CHROME_PATH,
Copy link
Member Author

Choose a reason for hiding this comment

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

I was noticing the FPS was rendered incorrectly on the default puppeteer chrome. We could also update the puppeteer version in this PR. Not sure it started showing up now though.

headless: false,
});

try {
const page = await browser.newPage();
Expand Down
Loading