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

Investigate disparate behavior in unit tests and real world use #704

Open
wise-king-sullyman opened this issue Jul 18, 2024 · 3 comments
Open

Comments

@wise-king-sullyman
Copy link
Collaborator

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.

@thatblindgeye thatblindgeye added enhancement New feature or request codemod labels Jul 18, 2024
@thatblindgeye
Copy link
Collaborator

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

@thatblindgeye thatblindgeye removed the enhancement New feature or request label Jul 18, 2024
@wise-king-sullyman
Copy link
Collaborator Author

Made #706 for the previous comment

@adamviktora
Copy link
Contributor

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.

@tlabaj tlabaj added the post v6 label Aug 22, 2024
@tlabaj tlabaj added this to the 2023.Q3 milestone Aug 22, 2024
@kmcfaul kmcfaul modified the milestones: 2023.Q3, 2024.Q4 Aug 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Backlog
Development

No branches or pull requests

5 participants