Skip to content
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

Need to add tests for sendBeacon #29

Closed
toddreifsteck opened this issue Apr 21, 2016 · 4 comments
Closed

Need to add tests for sendBeacon #29

toddreifsteck opened this issue Apr 21, 2016 · 4 comments
Assignees
Milestone

Comments

@toddreifsteck
Copy link
Member

No description provided.

@toddreifsteck toddreifsteck self-assigned this Apr 21, 2016
@toddreifsteck
Copy link
Member Author

One of our devs is locking down and cleaning up some tests for sendBeacon. I expect I'll be able to submit the client portion of this by end of May. It will likely take a bit of work by @plehegar at that point to get the server portion of the code wired up as internal framework is ASP.Net.

@igrigorik igrigorik added this to the Level 1 milestone Jun 29, 2016
@igrigorik igrigorik mentioned this issue Oct 11, 2016
9 tasks
@toddreifsteck
Copy link
Member Author

@igrigorik
Copy link
Member

igrigorik commented Oct 24, 2016

@toddreifsteck awesome, thanks Todd! Took a pass over the tests, and left some questions and comments on the PR: web-platform-tests/wpt#4024 (review)

Points we need to address:

  1. We did not spec a hard limit on max size of payload. I'm not sure if/how we want to test that.

    Related.. Chrome fails a subset of the submitted tests due to how it implements max size. Specifically, unlike Edge/FF we currently set a 64KB overall quota across for the lifetime of the context; each time a beacon is sent, we decrement its size from the quota and eventually drain the full quota in the tests and hence fail delivery of "max payload" tests... Technically, I do believe that this behavior is acceptable within current spec language. In practice, however, I think it's a bit restrictive and we might want to relax it... That said, as an interesting data point, so far this has not come up as a reported user problem, and it does serve to demonstrate that different UAs can adopt different strategies -- as intended.

  2. As noted in Add Beacon tests http://www.w3.org/TR/beacon/ web-platform-tests/wpt#4024 (comment), we have three different implementations throwing three different errors on invalid URL. We need to sort out how we want to handle that.

  3. Neither FF nor Chrome current expose sendBeacon on Worker: Beacon is only exposed to Worker in Microsoft Edge #37

@igrigorik
Copy link
Member

(3) was resolved in #37.
(2) was addressed in web-platform-tests/wpt#4024 (comment).
(1) we're working on in #38.

Closing this, let's continue in (1).. which will also define the test update(s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants