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

Jump to bottom

[test] Improve regression test suite debugging #20194

Merged
merged 13 commits into from
Mar 20, 2020
Merged

[test] Improve regression test suite debugging #20194

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
bb4a6c3
remove useless comments
eps1lon Mar 20, 2020
c8de1a5
extract blacklist logic into custom function
eps1lon Mar 20, 2020
a04d805
log reason for ignoring
eps1lon Mar 20, 2020
99ce437
also log included tests
eps1lon Mar 20, 2020
321a4f1
warn on unused patterns
eps1lon Mar 20, 2020
52315aa
better readable ignore message
eps1lon Mar 20, 2020
3c86385
throw to see the issue in CI
eps1lon Mar 20, 2020
bf0cae6
Revert "throw to see the issue in CI"
eps1lon Mar 20, 2020
0b6de16
remove unused patterns
eps1lon Mar 20, 2020
b68aedd
fix: skip all docs-guides
eps1lon Mar 20, 2020
766b0ae
add standalone task for building regression suite
eps1lon Mar 20, 2020
e174f37
add more information
oliviertassinari Mar 20, 2020
6584a70
Update test/regressions/index.js
oliviertassinari Mar 20, 2020
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
3 changes: 2 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@
"test:coverage": "cross-env NODE_ENV=test BABEL_ENV=coverage nyc mocha 'packages/**/*.test.js' 'docs/**/*.test.js' --exclude '**/node_modules/**' && nyc report -r lcovonly",
"test:coverage:html": "cross-env NODE_ENV=test BABEL_ENV=coverage nyc mocha 'packages/**/**/*.test.js' --exclude '**/node_modules/**' && nyc report --reporter=html",
"test:karma": "cross-env NODE_ENV=test karma start test/karma.conf.js",
"test:regressions": "webpack --config test/regressions/webpack.config.js && rimraf test/regressions/screenshots/chrome/* && vrtest run --config test/vrtest.config.js --record",
"test:regressions": "yarn test:regressions:build && rimraf test/regressions/screenshots/chrome/* && vrtest run --config test/vrtest.config.js --record",
"test:regressions:build": "webpack --config test/regressions/webpack.config.js",
"test:umd": "node packages/material-ui/test/umd/run.js",
"test:unit": "cross-env NODE_ENV=test mocha 'packages/**/*.test.js' 'docs/**/*.test.js' 'scripts/**/*.test.js' --exclude '**/node_modules/**'",
"test:watch": "yarn test:unit --watch",
Expand Down
119 changes: 79 additions & 40 deletions test/regressions/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,21 +42,20 @@ const blacklist = [
'docs-components-badges/BadgeAlignment.png', // Redux isolation
'docs-components-badges/BadgeVisibility.png', // Needs interaction
'docs-components-breadcrumbs/ActiveLastBreadcrumb.png', // Redundant
'docs-components-buttons/ButtonBases.png', // Useless
'docs-components-buttons/FloatingActionButtonZoom.png', // Needs interaction
'docs-components-buttons/ButtonBases.png', // Flaky image loading
Copy link
Member Author

Choose a reason for hiding this comment

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

whoops

'docs-components-chips/ChipsPlayground.png', // Redux isolation
'docs-components-click-away-listener', // Needs interaction
'docs-components-container', // Not needed
'docs-components-container', // Can't see the impact
'docs-components-dialogs', // Needs interaction
'docs-components-drawers/SwipeableTemporaryDrawer.png', // Needs interaction
'docs-components-drawers/TemporaryDrawer.png', // Needs interaction
'docs-components-grid-list', // Not needed
'docs-components-grid-list/tileData.png', // No component
'docs-components-floating-action-button/FloatingActionButtonZoom.png', // Needs interaction
'docs-components-grid-list', // Image don't load
'docs-components-grid/InteractiveGrid.png', // Redux isolation
'docs-components-grid/SpacingGrid.png', // Needs interaction
'docs-components-hidden', // Not needed
'docs-components-hidden', // Need to dynamically resize to test
'docs-components-material-icons/synonyms.png', // No component
'docs-components-menus', // Not needed
'docs-components-menus', // Need interaction
'docs-components-modal/SimpleModal.png', // Needs interaction
'docs-components-modal/SpringModal.png', // Needs interaction
'docs-components-modal/TransitionsModal.png', // Needs interaction
Expand All @@ -74,7 +73,7 @@ const blacklist = [
'docs-components-popper/SpringPopper.png', // Needs interaction
'docs-components-popper/TransitionsPopper.png', // Needs interaction
'docs-components-portal/SimplePortal.png', // Needs interaction
'docs-components-progress', // Not needed
'docs-components-progress', // Flaky
'docs-components-selects/ControlledOpenSelect.png', // Needs interaction
'docs-components-selects/DialogSelect.png', // Needs interaction
'docs-components-selects/GroupedSelect.png', // Needs interaction
Expand All @@ -84,7 +83,7 @@ const blacklist = [
'docs-components-snackbars/ConsecutiveSnackbars.png', // Needs interaction
'docs-components-snackbars/CustomizedSnackbars.png', // Redundant
'docs-components-snackbars/DirectionSnackbar.png', // Needs interaction
'docs-components-snackbars/FabIntegrationSnackbar.png', // Not needed
'docs-components-snackbars/FabIntegrationSnackbar.png', // Needs interaction
'docs-components-snackbars/IntegrationNotistack.png', // Needs interaction
'docs-components-snackbars/PositionedSnackbar.png', // Needs interaction
'docs-components-snackbars/SimpleSnackbar.png', // Needs interaction
Expand All @@ -99,18 +98,18 @@ const blacklist = [
'docs-components-transitions', // Needs interaction
'docs-components-tree-view/ControlledTreeView.png', // Redundant
'docs-components-tree-view/CustomizedTreeView.png', // Flaky
'docs-components-use-media-query', // Not needed
'docs-customization-breakpoints', // Not needed
'docs-customization-color', // Not needed
'docs-components-use-media-query', // Need to dynamically resize to test
'docs-customization-breakpoints', // Need to dynamically resize to test
'docs-customization-color', // Escape viewport
'docs-customization-default-theme', // Redux isolation
'docs-customization-density/DensityTool.png', // Redux isolation
'docs-customization-typography/ResponsiveFontSizesChart.png', // Not needed
'docs-discover-more-languages', // Not needed
'docs-discover-more-showcase', // Not needed
'docs-discover-more-team', // Not needed
'docs-getting-started-templates', // Not needed
'docs-customization-typography/ResponsiveFontSizesChart.png',
'docs-discover-more-languages', // No public components
'docs-discover-more-showcase', // No public components
'docs-discover-more-team', // No public components
'docs-getting-started-templates', // No public components
'docs-getting-started-templates-album/Album.png', // Flaky image loading
'docs-getting-started-templates-blog', // Not needed
Copy link
Member

Choose a reason for hiding this comment

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

// Not needed Maybe we should rename it // Nothing to test? Basically, these pages are supposed to yield close to no value or a low ROI.

Copy link
Member Author

Choose a reason for hiding this comment

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

Why is there nothing to test? They're not empty as far as I know.

Copy link
Member

@oliviertassinari oliviertassinari Mar 20, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

In this case, I imagine the random images from unsplash make it flaky. So the "label" isn't correct. Happy to answer this question for any other test case if you have a doubt.

Copy link
Member Author

Choose a reason for hiding this comment

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

Feel free to add any descriptive label you have. The current ones I removed where classic "comments for comments sake"-noise.

Copy link
Member

@oliviertassinari oliviertassinari Mar 20, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Ok, cool, let's add more specific comments. They should add information. "No needed" is equivalent to having the test inside the blacklist array: noise.

Copy link
Member Author

Choose a reason for hiding this comment

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

Perfect. That helps a lot. Is this good to merge in your opinion? Would be nice to get this in before #20187

'docs-getting-started-templates-blog', // Flaky random images
'docs-getting-started-templates-checkout/AddressForm.png', // Already tested once assembled
'docs-getting-started-templates-checkout/PaymentForm.png', // Already tested once assembled
'docs-getting-started-templates-checkout/Review.png', // Already tested once assembled
Expand All @@ -119,21 +118,59 @@ const blacklist = [
'docs-getting-started-templates-dashboard/Orders.png', // Already tested once assembled
'docs-getting-started-templates-dashboard/Title.png', // Already tested once assembled
'docs-getting-started-templates-sign-in-side/SignInSide.png', // Flaky
'docs-getting-started-usage/Usage.png', // Not needed
'docs-guides', // Not needed
'docs-styles-advanced', // Not needed
'docs-system-borders', // Not needed
'docs-system-display', // Not needed
'docs-system-flexbox', // Not needed
'docs-system-palette', // Not needed
'docs-system-positions', // Not needed
'docs-system-shadows', // Not needed
'docs-system-sizing', // Not needed
'docs-system-spacing', // Not needed
'docs-system-typography', // Not needed
'docs-versions', // Not needed
'docs-getting-started-usage/Usage.png', // No public components
/^docs-guides-.*/, // No public components
'docs-styles-advanced', // Redudant
'docs-system-borders', // Unit tests are enough
'docs-system-display', // Unit tests are enough
'docs-system-flexbox', // Unit tests are enough
'docs-system-palette', // Unit tests are enough
'docs-system-positions', // Unit tests are enough
'docs-system-shadows', // Unit tests are enough
'docs-system-sizing', // Unit tests are enough
'docs-system-spacing', // Unit tests are enough
'docs-system-typography', // Unit tests are enough
'docs-versions', // No public components
];

const unusedBlacklistPatterns = new Set(blacklist);

function excludeTest(suite, name) {
if (/^docs-premium-themes(.*)/.test(suite)) {
// eslint-disable-next-line no-console
console.log('ignoring premium themes pages');
return true;
}

return blacklist.some(pattern => {
if (typeof pattern === 'string') {
if (pattern === suite) {
unusedBlacklistPatterns.delete(pattern);
// eslint-disable-next-line no-console
console.log(`suite exact match: ignoring '${suite}/${name}'`);
return true;
}
if (pattern === `${suite}/${name}.png`) {
unusedBlacklistPatterns.delete(pattern);
// eslint-disable-next-line no-console
console.log(`suite+name exact match: ignoring '${suite}/${name}'`);
return true;
}

return false;
}

// assume regex
if (pattern.test(suite)) {
unusedBlacklistPatterns.delete(pattern);
// eslint-disable-next-line no-console
console.log(`suite matches pattern '${pattern}': ignoring '${suite}/${name}'`);
return true;
}
return false;
});
}

// Also use some of the demos to avoid code duplication.
const requireDemos = require.context('docs/src/pages', true, /js$/);
const demos = requireDemos.keys().reduce((res, path) => {
Expand All @@ -144,17 +181,11 @@ const demos = requireDemos.keys().reduce((res, path) => {
.reverse();
const suite = `docs-${suiteArray.reverse().join('-')}`;

if (blacklist.includes(suite)) {
return res;
}

if (blacklist.includes(`${suite}/${name}.png`)) {
return res;
}

if (/^docs-premium-themes(.*)/.test(suite)) {
if (excludeTest(suite, name)) {
return res;
}
// eslint-disable-next-line no-console
console.log(`testing ${suite}/${name}`);

res.push({
path,
Expand Down Expand Up @@ -217,3 +248,11 @@ tests.forEach(test => {
);
});
});

if (unusedBlacklistPatterns.size > 0) {
console.warn(
`The following patterns are unused:\n\n${Array.from(unusedBlacklistPatterns)
.map(pattern => `- ${pattern}`)
.join('\n')}`,
);
}