Replies: 1 comment 2 replies
-
wow, lots to unpack there 🙂
Yes -- it is annoying, it seems it was for a reason though: #190 - TL;DR; it increases the overhead of the method call depending on the number arguments, but while that is negligible (it's constant at least) more importantly it coupled the before/after methods (which accept a Contrary to PHPUnit I think it's improtant to have access to the parameters in the before/after methods as you'll often want to f.e. create X number of things to test against.
You should do:
Yes, "assertions" is a missing concern. At the moment sanity checking with One option is to allow returning a value from the benchmark, and having a way to run assertions or whatever on the result.
I do quite like the idea 🙂 PHPBench is an old project, with lots of legacy and stuff I'd love to change at some point, it could be far better (and simpler) than it is and the current API introduces constraints which are not ideal.
You can use |
Beta Was this translation helpful? Give feedback.
-
I will link my PR as soon as it's ready but I have something along those lines at the moment:
My first impressions:
Warmup
requires a parameter, at least in this case I do not see the need of having more than 1 revolution and I wonder if there is really many cases where you would want more than one.*: I want to expend on this point in this discussion. As you can see above I have:
chdir()
call need to be done in the::bench()
instead of the::warmUp()
.::bench()
as I do not want to be benchmarked.Warmup()
. I guess I can do this in__destruct()
. I admit I don't like that much but might be more a me problem 😅So I am wondering if it would not be possible to do something like this:
The key idea being you would have a
Bencher
class which defines what is benchmarked exactly. You remove the need of many hooks like the warmup or other as the user has complete freedom of this. And the result could contain both the output of the benchmarked closure and also provide the benchmark results which you could do assertion againsts.WDYT?
Beta Was this translation helpful? Give feedback.
All reactions