-
Notifications
You must be signed in to change notification settings - Fork 19
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
Investigate disparate behavior in unit tests and real world use #704
Comments
We should also update the codemod to check for TextProps and TextVariants imports and update them accordingly to ContentProps and ContentVariants, per updates made in https://github.com/patternfly/patternfly-react/pull/10643/files#diff-d30d90a7adc9371640f6e7ec412615f846adc6dd3bc39243b48d55f3506c2fd1 |
Made #706 for the previous comment |
I believe the tests are setup correctly. The issue lies in how the ESLint rules are executed. According to the documentation, after the fix is applied, it will run the rule (apply the fix) on the fixed code again and again until there is no diff between the files (no potential changes to fix) or until the rule runs 10 times https://eslint.org/docs/latest/extend/custom-rules#applying-fixes. The ESLint tests apply the fix only once. In case of nested components, I believe it is treated as a range conflict, so only one component can be renamed while applying the fix. This means that #675 (text to content codemod) will not work for case where some element is nested more than 10 times, e.g. some deeply nested list items. |
Follow up to #675
In order to land #675 I commented out some tests that were failing because of an issue with our test setup.
Nested test components are not updated in the unit tests, but are properly handled by the codemod in the real world.
Ideally we should dig into why we run into these false negative tests, and resolve the issue.
Additionally we should expand the text-replace-with-content test suite with coverage for alternative imports, aliasing, etc.
The text was updated successfully, but these errors were encountered: