spatie/phpunit-snapshot-assertions

Allow passing options as env var

mathieutu opened this issue · 6 comments

mathieutu commented

Hi, I would like to use the package with paratest, which is used under the hood by Laravel for parallel testing.

I don't succeed to pass any parameters like --without-creating-snapshots, and raised an issue: paratestphp/paratest#607

The answer is:

In Paratest the command is passed to PHPUnit directly, not through var:

https://github.com/paratestphp/paratest/blob/82d50dadbc357bc6a24c0a69710b6cae004017d5/bin/phpunit-wrapper.php#L41-L42

I see you are leveraging a non orthodox way to get an option to the PHPUnit extension: the should be reserved for settings.

Yes it would be possible to have working on Paratest too, but I suggest you to ask/PR https://github.com/spatie/phpunit-snapshot-assertions to move away from and rely on environment variables: they are widely used for this purpose (custom flags) and already supported both into configuration and one-time runs.

And actually I'm quite agreeing with this statement: using env var for those parameters doesn't seem to be a bad idea (in addition of current way).

Would you be open to a PR for that?

Thanks.

freekmurze commented

I like that users of our package can now simply add something to the command to change certain behaviour. What would the required steps be for a user to run that tests without creating snapshots?

mathieutu commented

If you want to keep it in cli it can be something like

  • SNAPSHOTS_CREATION=false phpunit for avoiding snapshot creation (or WITHOUT_CREATING_SNAPSHOT=true if you prefer)
  • SNAPSHOTS_UPDATE=true phpunit to allow snapshot update if already existing.

But with that, we also can set it in .env, in CI env var, in phpunit.xml file, etc. so the users can do whatever they want.

  • 👍1
mfn commented

I did something mildly related in our project:

I learned that when you're developing locally, you really always just want to update the snapshots. You'll see the diff anyway in your favorite VCS. But with a lots-of-devs it's easy to have someone not add this setting into their favourite IDE integration etc. So I extended the trait from this package and implemented my own shouldUpdateSnapshots with some env checking.

And in fact, if the env vars would have been supported already, I could have added this to the shared phpunit.xm.dist for example, not needing to change anything with the package.

I like the idea and too think that env vars are more flexible; the package could just keep the existing behaviour (e.g. command line overrides env var?) and add the new ones.

  • 👀1
freekmurze commented

I would be open for a PR that allows listening for env vars and while still keeping support for the current command line parameters.

mathieutu commented

Ok I'm doing it.

Do you have a preference for the env var names ?
CREATE_SNAPSHOTS=false and UPDATE_SNAPSHOTS=true are ok for you?

freekmurze commented

We'll continue the conversation in #126