Review or Edit in CodeSandbox

Open the branch in Web Editor • VS Code • Insiders

Open Preview

Build Stats

I think this PR is fine.
There is something i want to discuss.
This is a single playwright test, as a single browser window that is going to run N test functions, so n tests.
That is OK let's say apart that at some point if we have 200 tests and the 197 fails, the recording for that test will be long and hard to manage.
Would it make sense to have this test divided in small lot of tests divided by topic?
SVG export could be its own test that does what the generic rendering test was doing, just is its own file.

There would be some duplicated scaffolding maybe but can be resolved with a test builder.

Ideas?

I think this PR is fine. There is something i want to discuss. This is a single playwright test, as a single browser window that is going to run N test functions, so n tests. That is OK let's say apart that at some point if we have 200 tests and the 197 fails, the recording for that test will be long and hard to manage. Would it make sense to have this test divided in small lot of tests divided by topic? SVG export could be its own test that does what the generic rendering test was doing, just is its own file.

There would be some duplicated scaffolding maybe but can be resolved with a test builder.

Ideas?

if you look at the change closely you can already see that I made each visual test its own test
reason I did that is for easier debuggability and better overview what failed
previously everything executed in one test in steps
now each test entry is really a separate test with two steps: browser and node
now you know everything that fails
and before it would stop at first entry that would fail

you can observe this in master build and this build
master has 24 tests and this pr build has like 47 tests I think

locally this is very fast
but I am not sure why you limited playwright to only 1 worker in CI
CI should be able to handle at least 4 workers, what I do in my own projects is that I put it at 70% then playwright can determine itself how much is available and how much it should spawn

What I wanted to do is also to add property to render test case interface like only
then in case some entries have that set to true we only run those tests, that is if you want locally to run single test or just a few of them

another remark I have is that this current playwright setup is too complex
and I think it doesn't have to be like that
I think in the future this would be a better option
vitest-dev/vitest#690
its possible to make vitest in browser mode to compare images
but when vitest implements native visual snapshot testing I think it will be the best option for what you want to do
because you write it as a simple unit test, import what you want directly from ts files and be done with it
and it supports watch mode natively, without having to write babel acrobatics as you have now
and all those shenanigans for serializeing code into the browser and having to have mutiple hardcoded index files

when vitest supports it natively I can bring it for discussion, prepare a POC and we can discuss that again

Got a break i ll be back to this PR either tomorrow or wednesday

Got a break i ll be back to this PR either tomorrow or wednesday

No worries Andrea, tests will not run away, take your time

I ll convert some other visual tests to be sure i m comfortable with the process