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

Exporting some utility helpers #6837

Open
Tracked by #7415
dej611 opened this issue Jun 9, 2023 · 4 comments
Open
Tracked by #7415

Exporting some utility helpers #6837

dej611 opened this issue Jun 9, 2023 · 4 comments
Labels
feature request task A task associated with a larger Meta issue testing Issues or PRs that only affect tests - will not need changelog entries

Comments

@dej611
Copy link
Contributor

dej611 commented Jun 9, 2023

Is your feature request related to a problem? Please describe.

While on terms of usability and accessibility the framework is improving, it is becoming challenging to test on these aspects as they are now computing at runtime.
For instance button colours are now computed at runtime with chroma and emotion to improve the contrast ratio: https://github.com/elastic/eui/blob/main/src/themes/amsterdam/global_styling/mixins/button.ts#L76

For instance in this Kibana test I had to give up with a static value check, like a snapshot test, as computing the actual value would just duplicate a logic here in EUI, which would be even worst: https://github.com/elastic/kibana/blob/66ae25d376ecc1ea8ca82d60e7fb4edfa09192d3/x-pack/plugins/lens/public/shared_components/dataview_picker/trigger.test.tsx#LL61C67-L61C83

But some can be applied to other components like the select component with multiple selections. Etc..

Describe the solution you'd like
It would be nice to have some helpers to simplify the testing on the consumer side.
For colors that could be publishing all computed variants in a euiThemeDebugVars json, or just provide a function helper.
For other component some other solution might help.

I think that some dog fooding here would help, as you are probably the best to find the right helper API to test the component on your side. Just exporting them would already make things much better.

@cee-chen cee-chen added the testing Issues or PRs that only affect tests - will not need changelog entries label Jun 9, 2023
@cee-chen
Copy link
Contributor

cee-chen commented Jun 9, 2023

In general, I'm 100% on board with the premise of exporting more test utility helpers, especially if it would help consumers move away from snapshots, and it feels like something we could absolutely do alongside our general Enzyme->RTL tech debt work.

Just to clarify the specific use case you linked/mentioned in your PR description - it's not clear to me what the goal is with the color checking assertions 🤔 TBH, with most things CSS-based I would have leaned towards a visual screenshot/diff rather than trying to check the specific CSS output.

In your opinion, would it be fine for an exported consumer assertion to check the rendered Emotion className (e.g. a danger class) rather than the actual rendered CSS, and have EUI's internal unit tests be the ones in charge of confirming that the rendered CSS outputs the expected color?

@dej611
Copy link
Contributor Author

dej611 commented Jun 12, 2023

TBH, with most things CSS-based I would have leaned towards a visual screenshot/diff rather than trying to check the specific CSS output.

Not sure we have an infrastructure for that in Kibana testing.

In your opinion, would it be fine for an exported consumer assertion to check the rendered Emotion className (e.g. a danger class) rather than the actual rendered CSS, and have EUI's internal unit tests be the ones in charge of confirming that the rendered CSS outputs the expected color?

No preference here as the only implementation detail I would like to know when testing is that a danger styling has been applied to the text. If that is passing thru a style or class check would be the same for me.

Copy link

👋 Hi there - this issue hasn't had any activity in 6 months. If the EUI team has not explicitly expressed that this is something on our roadmap, it's unlikely that we'll pick this issue up. We would sincerely appreciate a PR/community contribution if this is something that matters to you! If not, and there is no further activity on this issue for another 6 months (i.e. it's stale for over a year), the issue will be auto-closed.

@JasonStoltz
Copy link
Member

@tkajtoch FYI, we've been talking about test helpers.

@tkajtoch tkajtoch added the task A task associated with a larger Meta issue label Mar 27, 2024
@github-actions github-actions bot added the Stale label Oct 4, 2024
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Oct 11, 2024
@cee-chen cee-chen reopened this Oct 14, 2024
@cee-chen cee-chen removed the Stale label Oct 14, 2024
@JasonStoltz JasonStoltz added task A task associated with a larger Meta issue and removed task A task associated with a larger Meta issue labels Jan 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request task A task associated with a larger Meta issue testing Issues or PRs that only affect tests - will not need changelog entries
Projects
None yet
Development

No branches or pull requests

4 participants