Removed test code

[Nyholm]

No description provided.

[dbu]

the tests fail. why do you want to remove this? any idea how to keep the tests happy?

[Nyholm]

When I copy the contents of the file to my varnish configuration, varnish starts to complain on that line.
I thought these files where just to make it easier to configure your varnish installation. I did not know they were used in integration testing.

[dbu]

okay. lets think about it again if there is a more elegant solution. i won't get to it before my holidays unfortunately, but this is the not-yet-released 2.0 version so there should be no harm done.

[ddeboer]

I suggest we move set req.url = "/_fos_user_context_hash"; to a separate VCL, in a user_context_hash_url sub . We can then:

• suggest users include that VCL if they want to use the default hash URL (/_fos_user_context_hash)
• with the option of implementing the sub themselves if they want a custom URL
• use our current user_context_hash_url only in our tests, which we then no longer include from fos_user_context.vcl (but in Functional/Fixtures/.../fox.vcl instead): I agree with @Nyholm that by default, our testing could should not be included.

[dbu]

sounds like a plan

[Nyholm]

Thank you both for input.
I've updated according to @ddeboer suggestions. I've aslo updated the docs.

[dbu]

i think the doc should mention that you can change the lookup url by including your own vcl file

[dbu]

apart from that 👍

[Nyholm]

Yes, It should be mentioned in the documentation. I'll add something in a minute.

I would like to share how I use these VCLs. I use them as reference configuration. I copied everything to one big VCL and made some minor adjustments. I keep them outside of the git repo. My point is: If I'm going to change the URL, I'm more likely to actually edit fos_user_context_url.vcl than create my own and make sure to include my new VCL instead of the provided one.
Some other developer using the VCLs might remove the call user_context_hash_url; and replace it with set req.url = "/_fos_user_context_hash"; because it simpler and they don't care about the tests.

[dbu]

lets rephrase this to say that fos_user_context.vcl needs the user_context_hash_url subroutine that sets the url to the request lookup url. The default URL is /_fos_user_context_hash and you can simply include ..._url.vcl in your configuration to provide this. If you need a different URL, include a custom file.

[dbu]

sorry, missed this update. i proposed an improvement on the documentation, otherwise i think this is ready to merge and indeed more clean than the previous solution.

re: your usage: our idea is that for version 2.0 you can use the vcl as-is and just add your own things. this would make maintenance easier. but of course people are free to follow other approaches. everything in one file seems hard on readability and updateability to me though - and performance wise should not matter as the configuration is compiled at startup anyways.

[dbu]

oh, and could you squash the commits please?

[Nyholm]

Sorry for not making a comment after updating last time. I've updated the PR now. What do you think?

[dbu]

thanks a lot!

the test failure is with hhvm, and noted in #245 and has nothing to do with your changes.

[Nyholm]

Thank you for reviewing and merging this.