Failing snapshot tests with pytest tests/snapshot_tests/test_snapshots.py?

[TomJGooding]

This is weird issue that has been bugging me for a couple of weeks. Despite everything passing when I run all tests with make test, when I try running only the snapshot tests this results in failing tests relating to the command palette.

Can anyone else reproduce this?

------------------------- snapshot report summary --------------------------
2 snapshots failed. 293 snapshots passed.
========================= short test summary info ==========================
FAILED tests/snapshot_tests/test_snapshots.py::test_command_palette - AssertionError: assert False
FAILED tests/snapshot_tests/test_snapshots.py::test_command_palette_discovery - AssertionError: assert False
=========== 2 failed, 293 passed, 1 warning in 88.05s (0:01:28) ===========

[darrenburns]

Yes - it seems to be an issue where if you run the command palette snapshot tests and unit tests it produces different output vs running the command palette snapshots in isolation.

It's been driving me nuts the past couple of days because I have to run every single test and then run them again to generate new snapshots.

[TomJGooding]

Thanks Darren for confirming it isn't just me!

Initially I didn't spot any visual difference until I looked a bit closer!

I'm struggling to understand why the SVG fill would change depending on how you run the tests though!

[willmcgugan]

There is a very curious line here...
https://github.com/Textualize/textual/blob/main/src/textual/command.py#L389

Seems relevant

[darrenburns]

Yeah, I remember this issue was happening in the past, and adding that line seemed to workaround it. The issue seems to have returned though - the workaround is no longer working!

[darrenburns]

If I recall correctly, the original issue was the the segments for the emoji had a different "color" attribute in the output SVG. Hardcoding the color in the CSS for SearchIcon resulted in a constant color.

I'm guessing the recent change to SVGs to normalise them has broken the workaround here 🤷.

[darrenburns]

Do we also need to hardcode the background color of the SearchIcon? 🤠

(Even if this does happen to fix it I'd still love to know what is going on 😆)

[TomJGooding]

I'm guessing the recent change to SVGs to normalise them has broken the workaround here 🤷.

I think more likely the recent changes to the OptionList and/or CommandPalette in #4667, maybe where the background changed to transparent?

From a quick git bisect this issue started with the snapshot update in 793fcbd.

[darrenburns]

Confirmed that hardcoding a non-transparent background in the SearchIcon DEFAULT_CSS resolves the snapshot test issue (e.g. background: red;).

With that change in place, I can run the following commands and they all succeed (this currently fails on main):

Update the snapshots:

pytest tests/input/test_input_clear.py tests/snapshot_tests/ -k test_command_palette --snapshot-update

Check the command palette tests pass in isolation:

PASS:

pytest tests/snapshot_tests/test_snapshots.py::test_command_palette

PASS:

pytest tests/snapshot_tests/test_snapshots.py::test_command_palette_discovery

[darrenburns]

Also works if we get rid of the emoji, which is always a good solution 😉

[willmcgugan]

Can you print out the list of segments for both paths, and compare?

I can think of no earthy reason why they might differ, tbh.

[darrenburns]

I was thinking this is an issue around global caches - for example an @lru_cache is global and will persist across tests.

Perhaps some background color is being computed from transparent in the first test, being stored in the cache, then the same cache is being hit in the second test?

@willmcgugan I checked the SVG in the past and it was just the background color which differs - you can also see the background color differing on the snapshot test output if you look closely.

[willmcgugan]

The @lru_cache is a sound theory. Easy to test. We could remove all the lru_caches.

[darrenburns]

Tried but no luck. Although there are also some inside Rich that I didn't remove.

[darrenburns]

@TomJGooding This is still an issue, but if you run the tests with make test now, it'll distribute the tests across processes and is MUCH faster, so it makes this issue a little less painful. There's also make test-snapshot-update.

[TomJGooding]

If I run make test now with my laptop I'm afraid it might take off! 😉

[TomJGooding]

It looks like these snapshots were updated recently in dd13211, now running only the snapshot tests doesn't have this issue!

I'll leave it up to you whether you want to close this issue or try to investigate why this was happening.