test: Fix Cover block color picker test #52943
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JavaScript logic fails due to undefined references from mocked Sass files.
Synchronous queries of user-facing text is generally preferred over test IDs or asynchronous queries. Additionally, the `react-native-hsv-color-picker` must be mocked to trigger the required interaction events. Lastly, selecting the white color results in a non-custom color, which does not appear to display the HEX value in the picker footer label.
dcalhoun
added
[Type] Automated Testing
|
Size Change: 0 B Total Size: 1.44 MB ℹ️ View Unchanged
|
|
Flaky tests detected in be989b6. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5658415230
|
dcalhoun
commented
Jul 25, 2023
| const colorPalette = await screen.findByTestId( 'color-palette' ); | ||
| const colorButton = within( colorPalette ).getByTestId( | ||
| 'custom-color-picker' | ||
| const colorButton = screen.getByA11yHint( |
There was a problem hiding this comment.
When possible, queries relying upon synchronous, user-facing text are generally preferred over asynchronous or test ID queries as they are more stable and simpler to understand. I applied this principle here and a few other places. I also removed the now unused test IDs.
There was a problem hiding this comment.
When possible, queries relying upon synchronous, user-facing text are generally preferred over asynchronous or test ID queries as they are more stable and simpler to understand.
This is a good tip, thank you.
| const screen = await initializeEditor( { | ||
| initialHtml: COVER_BLOCK_PLACEHOLDER_HTML, | ||
| } ); | ||
|
|
||
| const block = await screen.findByLabelText( 'Cover block. Empty' ); |
There was a problem hiding this comment.
This query and a few others appeared (arguably) unnecessary as later queries implicitly assert the presence of this element as children. In general, I try to use as few queries/assertions as necessary to keep test logic simple, but I do understand there is sometimes value in numerous assertions to communicate subtleties.
Please feel free to reinstate this query if you find it valuable, though.
| // Assertion to check the hex value in bottomLabelText | ||
| expect( bottomLabelText ).toHaveTextContent( COLOR_WHITE ); | ||
| // Assert label hex value after tapping color picker | ||
| expect( screen.getByText( '#00FF00' ) ).toBeVisible(); |
There was a problem hiding this comment.
When selecting white as the "custom" color, the following logic results in absence of a custom color as white is an available preset color. Therefore, I modified this to be a "random" color.
I am unsure if the above logic needs to be modified to support white and other preset colors...
There was a problem hiding this comment.
I was just using white as a placeholder color here. I had yet to think through how we might actually select a color on the color picker within the test. I think using hsv-color-picker and onHuePickerPress is a good solution. 👍
Comment on lines
+205
to
+206
| picker: {}, | ||
| pickerPointer: {}, |
There was a problem hiding this comment.
Due to the way we rely upon Sass files, reference individual styles from them in JavaScript, and subsequently mock them in tests, we have to add individual style mocks in this file. It is quite cryptic and cumbersome.
An alternative is mocking the values within the test file itself, which is arguably less cryptic. The inline mock is a recent practice introduced by @geriux, one that is likely worth embracing IMO.
There was a problem hiding this comment.
Thank you for pointing this out. This is the part I was missing.
|
@dcalhoun Thank you for taking the time to create a PR! At a quick glance, this answers many of my questions about best practices for mocking styles within our tests. I'd like to take a deeper look at the individual comments later this week to fill in the gaps in my understanding, and then merge this PR to fix the tests. 👍 |
derekblank
approved these changes
Jul 27, 2023
derekblank
merged commit 81abcec
into
rnmobile/display-value-in-colorpicker
51 checks passed
derekblank
added a commit
that referenced
this pull request
Aug 3, 2023
…ker (#51414) * Display custom color value in mobile color picker * Update Cover block bottomLabelText to use proper hex color style * Update text color style * Update CHANGELOG * Update getBottomLabelText from a function to a const * Add test for Cover block custom color picker * Update Cover block color picker test * test: Fix Cover block color picker test (#52943) * test: Mock required styles for Cover block tests JavaScript logic fails due to undefined references from mocked Sass files. * test: Fix Cover block HEX test assertions Synchronous queries of user-facing text is generally preferred over test IDs or asynchronous queries. Additionally, the `react-native-hsv-color-picker` must be mocked to trigger the required interaction events. Lastly, selecting the white color results in a non-custom color, which does not appear to display the HEX value in the picker footer label. --------- Co-authored-by: David Calhoun <github@davidcalhoun.me>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Repair a failing Cover block test focused on the custom color picker.
Why?
Improve automated test coverage.
How?
test: Mock required styles for Cover block tests
JavaScript logic fails due to undefined references from mocked Sass
files.
test: Fix Cover block HEX test assertions
Synchronous queries of user-facing text is generally preferred over test
IDs or asynchronous queries. Additionally, the
react-native-hsv-color-pickermust be mocked to trigger the requiredinteraction events. Lastly, selecting the white color results in a
non-custom color, which does not appear to display the HEX value in the
picker footer label.
Testing Instructions
n/a
Testing Instructions for Keyboard
n/a
Screenshots or screencast
n/a