-
Notifications
You must be signed in to change notification settings - Fork 53
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test fails when more than five pixels are requested #7
Comments
Hey @gka, I can replicate that the test you've linked there fails. But i wanted to make sure we had a different external check, so i wrote up a little set of ruby scripts just as a sanity check, and things seem to be working as intended there: https://gist.github.com/knowtheory/e3c370ad28a6503a9cf6 |
I'm looking at this, and yes, it seems the last request in the test isn't going through, but this:
Seems like a different bug? Are you saying that the requests are being double counted? Cause that is definitely strange. |
Yes they are counted multiple times. There are exactly five requests (maybe six), but the resulting JSON data that is flushed to the test endpoint is |
I think we need to revisit the test, because all the external checks i've written in ruby have indicated that pixel-ping isn't double counting even across flush boundaries. I took the scripts from the gist i posted above, upped the sleep to 5 seconds, and did 100 count per threads and got the expected results https://gist.github.com/knowtheory/2accf4151d6cdd07fe7e |
While now the simple test with just 3 requests succeeds, a very similar test with six requests fails
❯ cake test 4: one 8: two 6: three --- flushed --- Test Failed { one: 4, two: 8, three: 6 }
Apparently the last requests is timed out, and the flush is only happening after the 60-seconds interval defined in test/config.json has passed. Also note that the counts are completely wrong, the expected result should be either
or, since the last request failed, at least:
I could reproduce this multiple counting of single pixel.gif requests in a real server setting, so this is a serious bug.
The text was updated successfully, but these errors were encountered: