Add Beacon tests http://www.w3.org/TR/beacon/ #4024
Conversation
| <title>W3C Beacon Basic Blob Test</title> | ||
| <meta name="timeout" content="long"> | ||
| <script src="../resources/testharness.js"></script> | ||
| <script src="../resources/testharnessreport.js"></script> |
There was a problem hiding this comment.
In all these files, the resources URLs in those script elements need to be changed to an absolute path:
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
| var smallPayloadSize = 10; | ||
| var mediumPayloadSize = 10000; | ||
| var largePayloadSize = 50000; | ||
| var maxPayloadSize = 65536; // The maximum payload size allowed for a beacon request. |
There was a problem hiding this comment.
Technically, we did not spec a max limit in the spec.. should we be testing it here?
One solution is that we spec 64kb. Another is that we either remove this test, or (better), pump up the value to some "obviously bad" value and test that it fails? E.g. 1MB or some such.
|
|
||
| var success = navigator.sendBeacon("http://doesnotmatter", exceedPayload); | ||
| assert_false(success, "calling 'navigator.sendBeacon()' with payload size exceeding the maximum size must fail"); | ||
| }, "Verify calling 'navigator.sendBeacon()' with a large payload returns 'false'."); |
There was a problem hiding this comment.
Same comment as above on maxsize.
| test(function() { | ||
| var invalidUrl = "http://invalid:url"; | ||
| // http://osgvsowi/7877590 - Fetch consistently throws TypeMismatchError instead of TypeError | ||
| assert_throws("TypeMismatchError", function() { navigator.sendBeacon(invalidUrl, smallPayload); }, |
There was a problem hiding this comment.
Per spec...
Set parsedUrl to the result of the URL parser steps with url and base. If the algorithm returns an error, or if parsedUrl's scheme is not "http" or "https", throw a "SyntaxError" exception and terminate these steps.
- Chrome throws
SyntaxError - Firefox throws
URLMismatchError - Edge throws a
TypeMismatchError
Not sure what to do about this one. Wondering if we should simply relax the spec and indicate that an error should be throw, without declaring a type. Similarly, update the test to check that an error is thrown, without checking its type.
There was a problem hiding this comment.
@travisleithead @annevk @esprehn any thoughts or recommendations on how to proceed with this one?
There was a problem hiding this comment.
The specification has to be precise and the testcase has to be precise, matching the specification. DOMException "SyntaxError" seems okayish. Note that Fetch throws JavaScript's TypeError, not DOMException "TypeMismatchError".
There was a problem hiding this comment.
Fetch will throw a TypeError if it fails to parse the URL, correct? Given that all three implementations are different.. does it make sense to align with Fetch and spec that?
There was a problem hiding this comment.
Yeah, a JS TypeError. I think that would make sense, but I'm biased.
There was a problem hiding this comment.
I'm leaning towards TypeError because we'll also be telling folks to use Fetch for customized cases (e.g. non-POST method) of beacon..
@toddreifsteck wdyt?
There was a problem hiding this comment.
@igrigorik Agreed! I'll update the test to TypeError. You got the spec update or want me to make it?
There was a problem hiding this comment.
Done: w3c/beacon@ff26009
Filed: crbug.com/660241 and https://bugzilla.mozilla.org/show_bug.cgi?id=1313533.
|
Just realized this is still sitting here open.. Brandon will submit his updates to our branch then I'll rebase this on Master so we can bring it back to life. |
|
friendly bump :-) |
|
As some high-level feedback, you should use the .any.js format described here: http://web-platform-tests.org/writing-tests/testharness.html. That way you only need a single JavaScript resource to test windows and workers and no longer need the boilerplate HTML. See #5129 for an equivalent PR for Fetch. Simplifies things dramatically. |
|
@toddreifsteck any updates on this one? :) |
|
There are no owners for this pull request. Please reach out on W3C's irc server (irc.w3.org, port 6665) on channel #testing (web client) to get help with this. Thank you! |
|
This PR is being abandoned. We have another fork that removes Workers, updates quotas, etc. I just reviewed our internal code and it is being updated. Closing this PR to avoid noise. |
Note that only Microsoft Edge implements sendBeacon on worker. Also... we need to have someone from Firefox analyze their failures.