Web-stories-wp: inlineSelection.karma.js tests are flakey

[diegovar]

The following tests seem to be flakey and fail often when run before PRs:

Chrome Headless 85.0.4182.0 (Linux x86_64) CUJ: Editor Can Style Text Action: Inline selection when there is a mix of font weights should make black+bold+regular selection black when toggling FAILED
    Error: Expected '<span style="font-weight: 900">Fil</span>l in some text' to be '<span style="font-weight: 900">Fi</span><span style="font-weight: 900">l</span>l in some text'.
        at <Jasmine>
        at UserContext.<anonymous> (webpack:///assets/src/edit-story/components/richText/karma/inlineSelection.karma.js:326:24 <- assets/src/edit-story/karma-tests.js:301413:24)
Chrome Headless 85.0.4182.0 (Linux x86_64): Executed 16 of 139 (1 FAILED) (0 secs / 25.289 secs)
Chrome Headless 85.0.4182.0 (Linux x86_64) CUJ: Editor Can Style Text Action: Inline selection when there is a mix of font weights should make black+bold+regular selection black when toggling FAILED
    Error: Expected '<span style="font-weight: 900">Fil</span>l in some text' to be '<span style="font-weight: 900">Fi</span><span style="font-weight: 900">l</span>l in some text'.
        at <Jasmine>
        at UserContext.<anonymous> (webpack:///assets/src/edit-story/components/richText/karma/inlineSelection.karma.js:326:24 <- assets/src/edit-story/karma-tests.js:301413:24)
Chrome Headless 85.0.4182.0 (Linux x86_64) CUJ: Editor Can Style Text Action: Inline selection when there is a mix of font weights should make bold+regular selection bold when toggling FAILED
    Error: Expected '<span style="font-weight: 900">F</span><span style="font-weight: 700">il</span>l in some text' to be '<span style="font-weight: 900">F</span><span style="font-weight: 700">i</span><span style="font-weight: 700">l</span>l in some text'.
        at <Jasmine>
        at UserContext.<anonymous> (webpack:///assets/src/edit-story/components/richText/karma/inlineSelection.karma.js:297:24 <- assets/src/edit-story/karma-tests.js:301391:24)
Chrome Headless 85.0.4182.0 (Linux x86_64): Executed 17 of 139 (2 FAILED) (0 secs / 28.554 secs)
Chrome Headless 85.0.4182.0 (Linux x86_64) CUJ: Editor Can Style Text Action: Inline selection when there is a mix of font weights should make bold+regular selection bold when toggling FAILED
    Error: Expected '<span style="font-weight: 900">F</span><span style="font-weight: 700">il</span>l in some text' to be '<span style="font-weight: 900">F</span><span style="font-weight: 700">i</span><span style="font-weight: 700">l</span>l in some text'.
        at <Jasmine>
        at UserContext.<anonymous> (webpack:///assets/src/edit-story/components/richText/karma/inlineSelection.karma.js:297:24 <- assets/src/edit-story/karma-tests.js:301391:24)

See https://github.com/google/web-stories-wp/pull/3464/checks?check_run_id=913287582

[barklund]

Hm, interesting error. I actually created the tests as the (here failed) result, because it makes sense – I assumed draft-js would convert '123' to <b>12</b>3. But I was surprised to see, that the test failed with draft-js returning <b>1</b><b>2</b>3. The above indicates, that under some circumstances (maybe new library version somewhere?), draft-js does return the short-hand result.

I guess we should just pass it through some cleanup function to combine similar tags when directly adjacent - though for some cases there are no single "true" result (e.g. both <b>1<i>2</i></b><i>3</i> and <b>1</b><i><b>2</b>3</i> are valid and minimal representations of '123'. Let's see if that is a problem though.

[barklund]

@bmattb, I think we should prioritize this tech debt ticket for next sprint (I just ran in to the bug again). It should be fairly easy.

[barklund]

Thanks to a suggestion from @arthurandrade-dev elsewhere, I think we can actually just remove those tests completely and rely on the Percy snapshot tests right next to them only. If the rendered output appears the same, who cares if it's annotated one way or the other!

[swissspidy]

As long as we don't approve snapshot changes by accident without noticing any changes...
Keeping the tests would also help with local development, as otherwise you need to run Percy locally (which means requiring a percy token).

[barklund]

As long as we don't approve snapshot changes by accident without noticing any changes...

That's always a worry, but I think we'd notice. Or hope so!

Keeping the tests would also help with local development, as otherwise you need to run Percy locally (which means requiring a percy token).

I just don't see a clean way to do it, though?

An alternative way would be to do:

// Assume text content to be correctly formatted
// NOTE: Sometimes returned HTML has two single-letter bold spans, sometimes they're joined into one:
const actual = getTextContent().replace('(<span([^>]*)>\w+)</span><span\2>', '$1');
const expected = `<span style="font-weight: 900">F</span><span style="font-weight: 700">il</span>l in some text`;
expect(actual).toBe(expected);

That's a pretty nasty regexp - with backreferencing and everything - and as we all know, HTML is not regular (literally and figuratively)!

Would you be more comfortable with that?

[swissspidy]

@barklund What about using toBeOneOf?

```js
// NOTE: Sometimes returned HTML has two single-letter bold spans, sometimes they're joined into one:
expect(actual).toBeOneOf([expected1, expected2]);

[barklund]

@barklund What about using toBeOneOf?

If only we had those matchers... But I've added it and it seems to work now :)

If we could find a way to implement the "real" jest-extended and jest-dom matchers, that would be a lot better. Or a karma equivalent of course.