tester
tester copied to clipboard
Snapshot testing
- 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.
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 👍
It's quite long in my mind that the Tester could support "user's arguments" from CLI.
:+1: 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!
Failure can be distingushed by triggered error.
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?
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?
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.
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 😟
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 🍏
Hello, I've once again found a use for snapshot testing, and it'd be nice if it could be part of Tester. What's the status on this? Can I help in any way to make this happen?
I am joining to @jiripudil.
We can wake this up. What about this summary:
- if some snapshot is missing, assertion fails
- calling
tester --snapshot testsgenerates fresh new snapshots for all run tests - API will be:
Assert::snapshot('name', $actual)
And now problems I have in my mind.
Q: Where to store snapshots?
A: In a folder snapshots similarly like output? Maybe configurable.
Q: Snapshot file naming.
A: I would prefer single file per test file. For examle: even for twenty Assert::snapshot() calls in a single test MyTest.php only one snapshots/MyTest.php will be created. Such snapshot file may contain deeper structure like:
<?php return [
'one' => ... data of snapshot 'one',
'two' => ... data of snapshot 'two',
];
Q: How to handle @testCase, @multiple, @phpIni?
A: Prohibite them? Or save snapshot for every combination. The inner snapshot file structure can handle it.
Q: What about repeated snapshot call like: Assert::snapshot('one', $one); Assert::snapshot('two', $one);?
A: A would fail the test.
Q: What about missing snapshot assert after editing test, like: there is stored snapshot 'one' but Assert::snapshot('one', $one); never called?
A: I would fail the test.
Probably there are some next issues, I didn't try to implement it yet.
Opinions?
- if some snapshot is missing, assertion fails
✔️
- calling
tester --snapshot testsgenerates fresh new snapshots for all run tests
✔️
The flag is currently --update-snapshots, but that's the least of the issues at the moment
- API will be:
Assert::snapshot('name', $actual)
✔️
Q: Where to store snapshots? A: In a folder
snapshotssimilarly likeoutput? Maybe configurable.
✔️
Yep, that's the default, and it's configurable via Tester\Snapshot::$snapshotDir.
Q: Snapshot file naming. A: I would prefer single file per test file. For examle: even for twenty Assert::snapshot() calls in a single test MyTest.php only one snapshots/MyTest.php will be created. Such snapshot file may contain deeper structure like:
That's an interesting idea. My concerns are that:
- it might be more difficult to navigate if you have complex data in the snapshots (such as API responses);
- it might take more effort to see which snapshots have changed in the diff if you update some of them.
Q: How to handle @testCase, @multiple, @phpIni? A: Prohibite them? Or save snapshot for every combination. The inner snapshot file structure can handle it.
I recall we've discussed a similar issue before: I wanted to implement some sort of automatic naming for the snapshots, but figured I wasn't able to do that deterministically, especially with @testCase. So we agreed it'd be easiest to require developers to name their snapshots explicitly.
I still think that makes the most sense, because only the developer knows the nature of the snapshot data – with @multiple, you can test that the snapshot changes with every iteration because some counter is incremented somewhere, but you might as well want to assert that the snapshot value doesn't change within multiple runs.
So in my opinion, Tester could expose a convenient method to fetch the run number for @multiple (similar to Environment::loadData()) and just leave it up to the developer to include all the necessary keys in the snapshot name.
Q: What about repeated snapshot call like: Assert::snapshot('one', $one); Assert::snapshot('two', $one);? A: A would fail the test.
How would you detect if it is intentional or not? I think there might be legitimate use cases for this, such as asserting an API response, then making some changes, and asserting that the response stays the same (e.g. because the changes have not yet been published).
Q: What about missing snapshot assert after editing test, like: there is stored snapshot 'one' but Assert::snapshot('one', $one); never called? A: I would fail the test.
It sounds nice to point out to the developer that they forgot to delete data for a removed/renamed snapshot, but I wouldn't fail the test.
Besides, it might be difficult to detect – if you use @testCase and only assert one snapshot per test method, each process knows about one snapshot, even though all of them are used in the complete test suite.