Open TomJGooding opened 1 month ago
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.
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!
There is a very curious line here... https://github.com/Textualize/textual/blob/main/src/textual/command.py#L389
Seems relevant
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!
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 🤷.
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 😆)
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.
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
Also works if we get rid of the emoji, which is always a good solution 😉
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.
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.
The @lru_cache is a sound theory. Easy to test. We could remove all the lru_caches.
Tried but no luck. Although there are also some inside Rich that I didn't remove.
@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
.
If I run make test
now with my laptop I'm afraid it might take off! :wink:
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?