Snapshot testing

[jiripudil]

• new feature
• BC break? no
• doc PR: todo

Snapshot testing is a technique which allows users to instruct the testing framework to generate and store a snapshot of a tested value and in the subsequent runs assert that the value matches the snapshot. It presumably gained popularity among web developers because it was introduced in Jest to facilitate testing of the user interface. On backend, it is particularly useful e.g. for testing API responses.

I'd like to explain in short some of the design decisions – first and foremost, why this can't be done in an external code similarly to PHPUnit plugins: Tester runs tests differently. Where in_array('--update', $argv) is sufficient in PHPUnit and works even in third-party code, Tester requires changes in the test runner to pass the information to the child processes.

I added the --update-snapshots flag and an environment variable. In the updating mode, tests with new or updated snapshots fail. I initially skipped them, but then I changed my mind: PHPUnit plugins mark such tests as incomplete, which is a PHPUnit-specific concept and is somewhat different from skipped tests. I think that in Tester, failing the test is semantically more correct than skipping.

As for the assertion API, I started with a simple Assert::matchSnapshot() method, but the code eventually grew with the reading and updating logic, so it started to make sense to extract it to a separate class: Snapshot. I am considering adding the method back to Assert as an alias so that the Assert class remains to be the primary public API, but I don't have a strong opinion on this.

The Snapshot::match() method requires a snapshot name. I played with generating it from test arguments, but I couldn't figure out a way to make it deterministic in all cases, especially accounting for the fact that @testCases can be executed as a single test as well as each method in a separate job. In the end it was much easier to leave it to the developer. The only problem is that the snapshot name is used as a part of the snapshot file name, so I guess it should be validated against a limited set of characters – or at least it should be well documented that it has to be a safe string. What do you think?

The last thing to point out is that var_export and eval are used for storing and reading the snapshot value. I tried other solutions such as JSON or serialize and while var_export also has its trade-offs, it still seems to be the best one: it doesn't change x.0 floats to x integers like JSON does; it produces PHP code and as such is instantly readable, as opposed to serialized values. The var_export function is bad at serializing objects, but objects do not typically need to be stored in snapshots. I guess that as long as this limitation is documented, it should be good enough. It's also what the aforementioned PHPUnit plugins use.

[milo]

I really like this idea. It's about year and half I created something similar in my local repo. But as a Tester's plugin and on high layer that this. Actually, I didn't hear about snapshot testing till today. So thanks :)

To issues you mentioned...

It's quite long in my mind that the Tester could support "user's arguments" from CLI. Some UNIX tools (based on getopt, AFAIK) use -- (end of args) separator: my/bin --args -- anything --else. The -- stops arguments processing and the rest of line takes as ordinary values. The Tester could pass it to a test. This solves some cases when users want to somehow control tests behaviour and they are using env variables for it now. But with snaphost testing in the Tester core, the global argumnet is more suitable.

The var_export() is OK. Value can be stored as <?php return ...; in a snaphost-xy.php so evil() is not needed.

I have to think about the rest. But definitely 👍

[jiripudil]

It's quite long in my mind that the Tester could support "user's arguments" from CLI.

👍 that would greatly improve extensibility. It's basically the only reason why snapshot testing can't be made as an external plugin without resorting to env variables.

so evil() is not needed.

I believe eval is used because it is more reliable than include or require. With include, there is no way to distinguish whether false means a failure, or a genuine false is stored in the snapshot, and require needs more complicated error handling, while eval just returns the expression (or throws ParseError) and the IO error handling is left to file_get_contents where it is easier to do. But perhaps when preceded by is_readable check, require should be equally suitable.

Looking forward to what you have to say to the other points!

[dg]

Failure can be distingushed by triggered error.

[milo]

Thinking about API, just for discussion.

Not sure, there should be class Snapshot in user space. Looks like Assert::snapshot() is sufficient. IMHO the method should assert always as Assert::same(), not match().

Signature of method could be Assert::snapshot(string $name, mixed $actual) (in manner of ($expected, $actual)). So, user have to pass snapshot name explicitly, no need for automatic naming. Every snapshot can be saved only once to prevent rewriting. That means, one snapshot can be asserted only once. Following is not allowed:

Assert::snapshot('name', $foo);
Assert::snapshot('name', $bar);

What do you think?

[jiripudil]

Not sure, there should be class Snapshot in user space. Looks like Assert::snapshot() is sufficient.

@milo that was something I was considering too. I've tried the suggested approach and it looks really good.

Does anybody have any idea why the test fails on appveyor?

[jkuchar]

Does anybody have any idea why the test fails on appveyor?

E_WARNING: file_put_contents(C:\projects\tester\tests\Framework/snapshots\Assert.snapshot.update.updatedSnapshot.phps): failed to open stream: Permission denied

Try to have path to phps file shorther then 255 chars.

[jiripudil]

Try to have path to phps file shorther then 255 chars.

This one is 88 characters long. I've even made it use platform-specific directory separator, but it doesn't seem to help. Unfortunately I don't have any experience with appveyor and not much with Windows either, so it's a mystery to me 😟

[jiripudil]

Ha, I've accidentally found why appveyor failed. It's not a feature, it's a bug, apparently not fixed until PHP 7.2.18 via this commit.

I'm now skipping the test in environments where this bug is manifested and everything is finally green 🍏