proxy: add response API tests in proxyunits.t #1035
Conversation
feihu-stripe
force-pushed
the
feihu/proxyunits3
branch
from
fe22f6f
to
0bbf5e2
Compare
vendor/mcmc/mcmc.c
Outdated
| @@ -251,6 +252,8 @@ static int _mcmc_parse_response(mcmc_ctx_t *ctx, mcmc_resp_t *r) { | |||
| } else { | |||
| r->vlen_read = buffer_remain; | |||
| } | |||
| // remove vsize and any leading space from response length. | |||
| l -= (n - cur) / sizeof(char); | |||
There was a problem hiding this comment.
sizeof(char) is defined as 1 in the C spec. can skip the division.
|
So, mcmc lives here: https://github.com/dormando/mcmc - with its own unit tests. I can port this over and add the tests but if you want to try go ahead. when I make changes to mcmc I do them in order:
it lets me see historically when vendor stuff changes specifically. |
|
Sure I can give it a try. |
|
might be nice in the meantime if you separate out the working tests so I can upstream those. you can leave the broken test in with a skip marker and repair it afterward. |
|
will do |
feihu-stripe
force-pushed
the
feihu/proxyunits3
branch
2 times, most recently
from
eccb164
to
dae5bf4
Compare
feihu-stripe
changed the title
|
mcmc fix moved to dormando/mcmc#3 PR should now be mergable. |
t/proxyunits.t
Outdated
| subtest 'response:hit()' => sub { | ||
| # ps_recv must not receive an error | ||
| proxy_test( | ||
| ps_send => "mg /response/ok\r\n", |
There was a problem hiding this comment.
double testing ok() instead of hit here.
should also test cases where ok/hit are false work properly.
|
I think the rest looks good but kinda groggy and need to think more. for the |
|
Added more cases for ok() and hit() |
|
looks okay I think. I'm going to need to come back after a break for a final review though; if you think you're done feel free to squash it. |
feihu-stripe
force-pushed
the
feihu/proxyunits3
branch
from
79306a4
to
81a7dfc
Compare
Did what I can think of, but I guess it is never 100%. Squahsed. |
|
Thanks. I think it's more important to round this out by ensuring we have |
sure will take a look |
|
there're a bunch of examples commented in t/startfile.lua fwiw. |
| local key = r:key() | ||
| if key == "/response/elapsed" then | ||
| local elapsed = res:elapsed() | ||
| if elapsed > 100000 then |
There was a problem hiding this comment.
should probably just check that elapsed is nonzero to avoid flaky test behavior.
|
merged it. didn't feel like the comment was a blocker. |
|
Released in 1.6.21 |
No description provided.