pages snapshot POC

[kalinkrustev]

Here is an idea: as the pages folder already contains a lot of code that renders components using different combination of properties, it seems snapshot tests can be relatively easy to activate for the majority of the components.

I tried this only for the Button component, but it seems many already follow the pattern, where they are rendered in a div with the classes content-section implementation, so the snapshot can only capture this div:

expect(container.getElementsByClassName('content-section implementation')).toMatchSnapshot();

The change in components/doc/common/codehighlight.js is required, because in snapshot tests there is no window.Prism.

As I do not know how next.js works, I am not sure that the .spec.js files can be put in the page/xxx folders. This also causes the respective snapshots folder to appear there.

Anyway, I think this is an interesting idea, that might help very easily to achieve a lot of test coverage.
Note that the file indes.spec.js can be just copied to many of the page/xxx folders and it is likely to work.

[github-actions]

Thanks a lot for your contribution! But, PR does not seem to be linked to any issues. Please manually link to an issue or mention it in the description using #<issue_id>.

[melloware]

Interesting but not sure about this. I like where you are thinking but I prefer to keep unit tests as testing functionality and not pollute it in the demos as they change so often.???

[kalinkrustev]

When changing the demo, it is easy to change the snapshot too. The tricky part is if you change the demo and the component together.

[melloware]

I would rather Cypress or PlayWright integration test the working showcase demo with real browser than do SNAPSHOT testing against it. I really think the SNAPSHOTs should be constrained to unit tests of the component. This feels like it will add to much overhead on every demo change

[melloware]

actually for integration tests I think we should use Playwright: https://playwright.dev/

I got a demo from a friend and its really slick and the tests look like normal Jest tests

[kalinkrustev]

Yes, Playwright is good, I like it and I use it, but it has its own set of requirements for both developer and CI. Also using it is not a reason to not use Jest. I do not think the case of testing the pages can be defined as either pure unit or pure integration test and be a reason to choose the tool (or constrain to a single tool). Using Playwright will not result in significantly less effort for maintaining the tests. I think the full benefit of tests is when they test both the visual and markup result. I think Jest is easier to use for markup and Playwright for visual. This PR was more about discussing if testing via pages is something the team behind Primereact considers useful. The proposed approach is easy to start with. Maybe it is not the perfect one or the only one, but it will give a very good code coverage without a lot of effort. Therefore, it is not a lot of investment to use it and can be easily abandoned if it proves to be unsustainable. Updating the snapshots is easy to do and it is not mandatory to catch all the bugs always. If it is too much effort to analyze all the differences in all the snapshots, it is OK to not do it fully and just accept the new snapshots. It is better to have some tests for more cases, than to have none.

Anyway, let me share what are the difficulties I saw around Playwright, assuming it is used for visual tests:

1. It is officially supported only on specific Linux distributions
2. Snapshots taken under one OS will not match snapshots taken another OS
3. Snapshots taken in headless mode will not match snapshots taken in headed

I have observed other edge cases too, where testing in parallel resulted in confusing trace files.
Still, I think Playwright is a very good tool, worth using. Knowing the above difficulties, teams can plan how to address them in their development and CI processes.

If there is interest, I can extend this PR with some simple Playwright test too.

[melloware]

I definitely see what you are saying.

I come from a Java background so for me Unit Tests just test the component does what it says where Integration tests test the component is working in a real browser with real mouse/keyboard events etc. React Test Library does a really good job of simulating and maybe its good enough! But on the PrimeFaces side I know our quality went way up after we added real Integration Tests (using Selenium) it helped find problems we would not have found otherwise. Just my thoughts...

[melloware]

But that being said @kalinkrustev i do think you are right and we should increase our Unit test Jest coverage before we dive into Integration tests?

[kalinkrustev]

Yes, I agree integration tests should happen later, as they will be harder to maintain in case unit tests are missing too many cases.

…

On Tue, Dec 6, 2022 at 5:36 PM Melloware ***@***.***> wrote: But that being said @kalinkrustev <https://github.com/kalinkrustev> i do think you are right and we should increase our Unit test Jest coverage before we dive into Integration tests? — Reply to this email directly, view it on GitHub <#3598 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAXJVW2YID5D5LA7DSMQGFLWL5MQLANCNFSM6AAAAAAR3HNSVM> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[melloware]

Closing for now.