chore(tests) fixate time format

[Dominik-Petrik]

What: Closes #8690

[patternfly-build]

Preview: https://patternfly-react-pr-8691.surge.sh

A11y report: https://patternfly-react-pr-8691-a11y.surge.sh

[adamviktora]

Solves the issue. Good job

[gitdallas]

looking good in the US

[adamviktora]

I ran the test again (previously I tried from different machine and it worked fine) and I still have the same issue in tests:

[wise-king-sullyman]

Is it just me or is the diff in that snapshot showing the same thing on each line?

[gitdallas]

yea.. i can't see a difference.. maybe a different type of space character or something? i'd be curious in you copying the line in question and converting the text to code representation (https://www.online-toolz.com/tools/text-unicode-entities-convertor.php).. then updating the snapshot and doing the same, see if something changed.

[adamviktora]

Yes, you are both right. It is a problem with the space just before "AM" (or PM in the next test).

This is the snapshot: 1/1/2022%2C%2012%3A00%3A00%20AM (Space)

And this is actually received: 1/1/2022%2C%2012%3A00%3A00%u202FAM (Narrow no-break space)

[adamviktora]

It seems, that new Date().toLocaleString() creates 'NARROW NO-BREAK SPACE' character before AM / PM in my localization.

Here is a suggestion, how it could be repaired: timestamp test repair commit

I have edited the tests, so they replace the 'NARROW NO-BREAK SPACE' with classic space before calling the screen.getByText(...) method.

I did not find an easy way to modify asFragment so it uses classic space, because it consists of a whole component using the .toLocaleString() internally. So I have modified the snapshot to use the 'NARROW NO-BREAK SPACE' -- but this is probably not a good way, because now it will break these localizations, which are using classic space. I don't know how to extend this expect(asFragment()).toMatchSnapshot(); condition, so we can compare fragment and snapshot as strings -- so the conversion of the narrow nbsp to space could be appliable.

Anyway, overall I don't think this is a great solution, it seems to me like only "hiding a problem" by this conversion of nbsp to space character -- and it could create some other troubles in the future.

[adamviktora]

It actually might be a Node version issue.

To reproduce the bug: I am using node v18.14.0, yarn 1.22.19

[Dominik-Petrik]

Slightly better solution than explicitly replacing spaces in the tested string might be to use normalization. Tested strings are getting normalized, which seems to mess up the spaces. We can configure the normalization like this:

I am not sure how to handle the snapshot failing, because there seems to be no way to configure normalization on snapshot.

[gitdallas]

screenshot is based on what the app produces, so if you normalize it in the app (timestamp component?) it will update the screenshot that way.

[Dominik-Petrik]

screenshot is based on what the app produces, so if you normalize it in the app (timestamp component?) it will update the screenshot that way.

Yeah, but the matchers are getting normalized and not what gets rendered. And since snapshots are not getting matched, we can't use normalization on them. And the code snapshot produces can't be easily modified because we would have to modify the Timestamp component itself. See Adam's comment

[wise-king-sullyman]

If it's a Node version issue would it potentially be resolved as part of #8303?