Compare whole table for snapshot testing

[pawelru]

I have noticed that for some of the snapshot tests (example 1 2) we are comparing tibble object using default print method that results in only small subset of table being printed out - on default it is just 10 rows. I feel it's not necessarily correct as a discrepancy might occur in e.g. row 11 and this won't be caught. We need to set appropriate options in the testthat setup so as to compare all values. Alternatively we can use expect_snapshot_value() instead of expect_snapshot() if that's respect all values.

[Melkiades]

The two examples have only 10 rows. They are not sub-setted if not in pre-processing, as intended. The only post-processing is sorting.

Besides the examples, there are random subsettings happening at times, I agree, but they are principally used to avoid unreadable feedback and uselessly enormous PRs for minimal practical updates. Note that with the recent introduction of snapshots, we have already done a great enhancement as before the structure (e.g. indentation) was NOT tested at all. That is why using snapshot_value would be going back (see reference afterward).

Nonetheless, I agree that we can do better in snapshot testing by selecting smartly what to test instead of random subsetting, but I do not think the overhead of setting local options would do us any good. I think it would also be good to test smart selections of tables instead of entire tables as it would also help to have context-aware testing (which is also faster and feedback is more readable). Still, I argue that this should be introduced incrementally, also through the introduction of regression tests when new features or bugs are introduced/fixed.

expect_snapshot_value captures the result of a function, flexibly serializing it into a text representation that's stored in a snapshot file. See expect_snapshot() for more details on snapshot testing.

expect_snapshot() Snapshot tests (aka golden tests) are similar to unit tests except that the expected result is stored in a separate file that is managed by testthat. Snapshot tests are useful for when the expected value is large, or when the intent of the code is something that can only be verified by a human (e.g. this is a useful error message). Learn more in vignette("snapshotting").

expect_snapshot() runs code as if you had executed it at the console, and records the results, including output, messages, warnings, and errors. If you just want to compare the result, try expect_snapshot_value().

[pawelru]

Some clarifications from my side

The two examples have only 10 rows

No. Both has 600 rows (1 2). We are doing just print of 10 rows (that's the default value) which means that more than 98% of rows is not checked. Add to this hidden columns (11 printed out of 51) then we came to the conclusion that almost whole table is not checked.

we can do better in snapshot testing by selecting smartly what to test instead of random subsetting

Exactly! We need to decide whether we want to test it all...

Note that with the recent introduction of snapshots, we have already done a great enhancement as before the structure (e.g. indentation) was NOT tested at all. That is why using snapshot_value would be going back (see reference afterward).

Yes I am aware that expect_snapshot is the right function for testing output from print on rtables object because of identation and more. Here I am asking a question what function to use when comparing snapshot of tibble object.

[Melkiades]

my apologies I did not notice it was the printout only! It makes sense a lot then, also checking 600 lines is a bit of an overkill for that function tests probably. I will think about it a bit more, for the moment smart or random sampling seems the best way to me.