search

Automattic / php-push

Work-in-progress on Push Notifications v2
5 stars 1 forks source link

Should random_string and random_uuid be seeded? #6

Open mokagio opened 4 years ago

mokagio commented 4 years ago

This might be an overkill, but would it be worth using a seed to generate random_string and random_uuid?

This would make the tests more repeatable.

jkmassel commented 4 years ago

I personally prefer to use random-ish garbage data to test most methods because it can turn up unexpected results. (ie – an unplanned test failure caused by random data can show bugs we might not otherwise catch).

If there's a failure, it'll show up in the logs with the expected and actual values, so we could use those as a specific test case if we wanted to repeat things?

That said, if there's a specific use-case, I'm not opposed to adding additional methods that do it?

mokagio commented 4 years ago

I personally prefer to use random-ish garbage data to test most methods because it can turn up unexpected results.

That's the beauty of randomness, isn't it? 😄

I should have been more specific. The idea of seed that I was proposing is along the lines of what RSpec does to run tests in random order. It randomizes the order with a seed and reports it as part of the test results. So if you get a failure, you can run the tests with the same seed to debug it.

jkmassel commented 4 years ago

Ah that's an interesting idea PHPUnit allows something similar to what I think you're suggesting:

➜  pushphp git:(add/disabling-ssl-verification) ✗ phpunit --order-by=random
PHPUnit 9.2.5 by Sebastian Bergmann and contributors.

Random seed:   1593102731

................................................................. 65 / 92 ( 70%)
...........................                                       92 / 92 (100%)

Time: 00:00.477, Memory: 10.00 MB

OK (92 tests, 128 assertions)

Generating code coverage report in HTML format ... done [00:00.131]

LMK if that looks like what you're after? It's an easy enough change to make

mokagio commented 4 years ago

Yeah, that's it.

Can the seed be read when the tests run and used for the generation of the random values?

Regardless, I think randomizing the tests is useful, too. Helps preventing being coupled to the state / execution order.