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

[test] Move Firefox tests to CircleCI #34764

Merged

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Oct 14, 2022

Firefox seems to disconnect quite often looking at the CI runs in https://github.com/mui/material-ui/commits/master, e.g. https://app.circleci.com/pipelines/github/mui/material-ui/82679/workflows/2da72430-6074-47cb-8bbe-e959f815d794/jobs/437658 so why not run Firefox in CircleCI. The one limitation is that we run the latest version (so no correct version compatibility test) but 🤷‍♂️

Firefox is also slow on BrowserStack relative to the others, maybe why it's unstable:

Screenshot 2022-10-15 at 00 49 49

https://automate.browserstack.com/dashboard/v2/builds/db108dc8dfacc0894749d3eac1271e4d8da8ac4c

@mui-bot
Copy link

mui-bot commented Oct 14, 2022

Messages
📖 Netlify deploy preview: https://deploy-preview-34764--material-ui.netlify.app/

No bundle size changes

Generated by 🚫 dangerJS against 707d727

Comment on lines -37 to +43
['default', 'primary'].forEach((color) => {
it(`can disable the ripple and hover effect for color ${color}`, () => {
const { container, getByRole } = render(
<IconButton disableRipple color={color} TouchRippleProps={{ className: 'touch-ripple' }}>
book
</IconButton>,
);
expect(container.querySelector('.touch-ripple')).to.equal(null);
expect(getComputedStyle(getByRole('button'), ':hover').backgroundColor).to.equal(
getComputedStyle(getByRole('button')).backgroundColor,
);
});
it('can disable the ripple and hover effect', () => {
const { container } = render(
<IconButton disableRipple TouchRippleProps={{ className: 'touch-ripple' }}>
book
</IconButton>,
);
expect(container.querySelector('.touch-ripple')).to.equal(null);
Copy link
Member Author

@oliviertassinari oliviertassinari Oct 14, 2022

Choose a reason for hiding this comment

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

See #29298 (comment) for why some of the assertions are gone. The test was a tautology, so brings no value.

@michaldudak
Copy link
Member

Do you know why the test-browser task fails primarily on the master branch? I haven't seen any problems in PRs, but it's red on a large number of master builds.

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Oct 18, 2022

I haven't seen any problems in PRs

@michaldudak It's because of this, we only run BrowserStack on master:

const browserStack = {
// |commits in PRs| >> |Merged commits|.
// Since we have limited ressources on BrowserStack we often time out on PRs.
// However, BrowserStack rarely fails with a true-positive so we use it as a stop gap for release not merge.
// But always enable it locally since people usually have to explicitly have to expose their BrowserStack access key anyway.
enabled: !CI || !isPR || process.env.BROWSERSTACK_FORCE === 'true',

@oliviertassinari oliviertassinari merged commit bb84c5a into mui:master Oct 29, 2022
@oliviertassinari oliviertassinari deleted the skip-browser-stack-firefox branch October 29, 2022 15:07
@oliviertassinari
Copy link
Member Author

daniel-rabe pushed a commit to daniel-rabe/material-ui that referenced this pull request Nov 29, 2022
feliperli pushed a commit to jesrodri/material-ui that referenced this pull request Dec 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants