Le 09/01/2019 à 10:43, Frederic Lecaille a écrit :
On 1/8/19 11:25 PM, PiBa-NL wrote:
Hi Frederic,

Hi Pieter,

Op 7-1-2019 om 10:13 schreef Frederic Lecaille:
On 12/23/18 11:38 PM, PiBa-NL wrote:
As requested hereby the regtest send for inclusion into the git
repository.
It is OK like that.

Note that you patch do not add reg-test/filters/common.pem which could
be a symlink to ../ssl/common.pem.
Also note that since 8f16148Christopher's commit, we add such a line
where possible:
     ${no-htx} option http-use-htx
We should also rename your test files to reg-test/filters/h00000.*
Thank you.
Fred.

Together with these changes you have supplied me already off-list, i've
also added a " --max-time 15" for the curl request, that should be
sufficient for most systems to complete the 3 second testcase, and
allows the shell command to complete without varnishtest killing it
after a timeout and not showing any of the curl output..

One last question, currently its being added to a new folder:
reg-test/filters/ , perhaps it should be in reg-test/compression/ ?
If you agree that needs changing i guess that can be done upon
committing it?

I have modified your patch to move your new files to reg-test/compression.

I have also applied this to it ;) :   's/\r$//'


Note that the test fails on my FreeBSD system when using HTX when using
'2.0-dev0-251a6b7 2019/01/08', i'm not aware it ever worked (i didn't
test it with HTX before..).
**** top  15.2 shell_out|curl: (28) Operation timed out after 15036
milliseconds with 187718 bytes received

Ok, I will take some time to have a look at this BSD specific issue.
Note that we can easily use the CLI at the end of the script to
troubleshooting anything.

Log attached.. Would it help to log it with the complete "filter trace
name BEFORE / filter compression / filter trace name AFTER" ? Or are
there other details i could try and gather?

I do not fill at ease enough on compression/filter topics to reply
to your question Pieter ;)

Nevertheless I think your test deserve to be merged.

*The patch to be merged is attached to this mail*.

Thank a lot Pieter.


Thanks Fred and Pieter, now merged. I've just updated the patch to add the list of required options in the VTC file.

--
Christopher Faulet

Reply via email to