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] Allow running all the tests in strict mode #684

Merged
merged 2 commits into from
Dec 8, 2020

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Dec 5, 2020

@DanailH Could you test that these changes allow removing the strict: false from the density pull request? We face the same issue when testing the filters and column selector feature.

ErrorBoundary,
// @ts-expect-error need to migrate helpers to TypeScript
createEvent,
} from 'test/utils';
Copy link
Member

Choose a reason for hiding this comment

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

How long would it take to migrate them?

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 don't know, I have never looked at it. I think that we should ask ourselves if it's the best use of our time. So far, I think that the main value proposition of writing the tests in TypeScript is to increase the types coverage of the public API of the components. To make sure the types are solid.

@DanailH
Copy link
Member

DanailH commented Dec 8, 2020

Sorry for the delay @oliviertassinari . I tested it and it seems that with these changes I can remove the strict: false from the tests.

@oliviertassinari
Copy link
Member Author

Now that your PR is merged, I can rebase mine and remove the strict: false, done. If the CI is green, I think that we can merge.

@oliviertassinari oliviertassinari merged commit b715df7 into mui:master Dec 8, 2020
@oliviertassinari oliviertassinari deleted the strict-mode-tests branch December 8, 2020 11:37
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.

None yet

3 participants