Asserting hits followed with a numerical number

[JonasHaouzi]

Hi guys.

First of all, a big thank you for the amazing work you did with this library and the Symfony bundle.

Here is a little suggestion related to my Varnish configuration, and I think many others developers are sharing it.

➜ What the library is currently doing
When writing a test and asserting if a request has been "HIT", we assert the header X-Cache is equal to HIT.

➜ Why is it not working in some cases?
Because some Varnish instances are set-up to return "HIT 1", "HIT 2", etc.. the assertions is failing.

➜ What's the proposal?
In this proposal, I modified the code to stop checking that the header is exactly identical to HIT but match a given regular expression. This regular expression allows the word HIT followed by an optional numerical number.

Jonas

[ddeboer]

First of all, a big thank you for the amazing work you did with this library and the Symfony bundle.

You’re welcome! And thank you for taking the time to contribute to this library.

Just to make sure this is a necessary change: in our docs we recommend configuring Varnish to set a simple HIT/MISS value. Can you point us to docs or config that suggest including a counter?

[dbu]

maybe we could just do false !== strpos to be more flexible for custom setups?

[JonasHaouzi]

Yes that'd work perfectly for me. Thanks.

[JonasHaouzi]

@ddeboer In our case, our VCL was already written and we haven't reuse the ones you're providing. So I can't provide it, sorry.

I found as well the site number 82 in France, and they're using the same:

$ curl --silent -I http://www.leparisien.fr/ | grep HIT
X-VarnCache: HIT 120

If you prefer not to include that, it's ok, we'll create our own assertions methods even if I think the cost is quite low and can benefit to others uses in the same case.

[dbu]

thanks, this makes sense to me, the cost indeed is low and it adds flexibility.

[dbu]

note that you did this PR against master which is the development version of 2.0. if you use 2.0 in production and it works, that is nice but i would recommend to at least lock to a specific commit id in your composer.json, as we might do some BC breaks without warning... see for example #285

[ddeboer]

Thanks @JonasHaouzi!