Textualize / pytest-textual-snapshot

Snapshot testing for Textual applications
MIT License
21 stars 4 forks source link

environment changes require regenerating snapshots #10

Closed juftin closed 9 months ago

juftin commented 9 months ago

When making environment changes, such as upgrading textual, snapshots need to be regenerated due to a "terminal id" field changing in the SVG body

I've noticed this with the app.save_screenshot method too

-      .terminal-916827204-matrix {
+      .terminal-3256088574-matrix {
           font-family: Fira Code, monospace;
           font-size: 20px;
           line-height: 24.4px;
           font-variant-east-asian: full-width;
       }

-      .terminal-916827204-title {
+      .terminal-3256088574-title {
           font-size: 18px;
           font-weight: bold;
           font-family: arial;
       }

The screenshot itself doesn't have any visual differences though

image

See this error from a project of mine and the commit that resolved it.

willmcgugan commented 9 months ago

Rich generates the SVGs. I suspect what happened is that you upgraded Rich which had a change in the SVG code. This happens relatively infrequently. You should find that future upgrades of Rich or Textual won’t require you to regenerate the snapshots.

juftin commented 9 months ago

Hey @willmcgugan, in this case rich wasn't actually updated. I use lockfiles to manage these environments and can confirm that textual changed from 0.39.0 from 0.46.0 while rich stayed stable at 13.5.3.

Looking closer I think the issue might be that unique_id is not being set on the export_svg here in textual.

I'm not sure how deterministic the code is here in rich to create the unique_id when not set but I seem to be getting a different output as I make changes.

willmcgugan commented 9 months ago

I'm guessing it was the addition of the title. That hash should be completely deterministic.

We've been using snapshot tests for a year, and it is very rare to have to regenerate all the snapshots. I doubt it will be an issue for you in future updates.

juftin commented 9 months ago

It' happened few times this year which makes me think something in there is not deterministic.

It looks like your screenshots are affected by it too if you look at the diff on tests/snapshot_tests/__snapshots__/test_snapshots.ambr in https://github.com/Textualize/textual/pull/3571/commits/57b7895e84b98f0863fc5b55b8b94063e60e7fb6. There are a number of tests which had their unique_id change with no code changes.

It looks like exactly what's happening on my screenshots: https://github.com/juftin/browsr/pull/33/commits/5bc6dcea77513783fb6cdffe0a35ad49cdebfa6e

willmcgugan commented 9 months ago

Some snapshots will break when we make updates to the render code in Textual.

The raw data may change and that will cause the hash to change, but the content in the SVG might not actually change. The snapshots let us decide if there is any real visual change.

When that happens it will cause your app to fail snapshots, but you can review and accept the changes.

I really think thats the explanation. If the hashes were not deterministic I would expect the snapshots to fail constantly, and not now and again.

juftin commented 9 months ago

Gotcha. Fair enough, thanks for going down the rabbit hole with me. Happy Holidays Will!