updates bullet tests for expansion #2243
Conversation
| // Expect that the entire "pin" section is removed: | ||
| expect(newState).not.toContain('- =pin\n - true') |
There was a problem hiding this comment.
We already have reducer tests that make assertions about the Redux state, so we actually want to avoid accessing or exporting the state in the component tests.
The component tests should test what is rendered to the DOM.
| // Click the second bullet directly: | ||
| await userEvent.click(bulletElements[1]) // Index 1 for the second bullet |
There was a problem hiding this comment.
Selecting all bullets and then indexing into the array is not ideal, because it couples the test to the linear order of thoughts as they are rendered from top-to-bottom on the screen. If a thought is the 2nd or 4th or 10th from the top, that's not very meaningful to the user. While it's not much of a problem when only rendering a few thoughts, it's going to become a problem for tests that involve a larger number of thoughts, or deeply nested thoughts. Pretty soon you have developers counting lines of code in order to figure out which thoughtContainers[12] refers to. And adding one thought at the top changes the indexes all the way down.
Instead, we want to find a way to select a thought based on its text. You can use contextToPath(['a', 'b']) to get the path which can be used to select the exact testid of interest. Or you could select by the visible text itself, or embed the thought value in a data attribute. They each have their tradeoffs, but at least they are not dependent on the linear order.
Let me know what you think. Thanks!
Updates broken expansion tests
findSubthoughtsfunction, instead, utilizes the index of the payloads to be tested