-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Add XHR responseType
tests
#1006
Conversation
Ah,
Gotta love unit tests. Amended the commit, wrapping the |
Is there a reason you are using multiple tests, rather than using a bool (like the audio tests)? You'd be able to reduce the code considerably, since its basically the same test with a few different strings. |
Personally (and @ryanseddon has said this in the past too), I prefer separate tests to using subproperties – they're easier to work with, fit better when using classes etc. But maybe this is a valid application for subproperties. Even if sticking with multiple tests, we could reduce the duplication:
|
Thanks for the feedback! @stucox I went with separate tests that don’t depend on each other because these tests are very specific. If you ever need them, chances are you only need one of them, but not more than that. Let’s say you use TL;DR -1 to introducing an AMD dependency on |
Amended the commit. Please review. |
Looks good. OCD Stu would like to see balanced gaps above and below the text in the |
Closes #1006.
@stucox Done. I had copied the DOC block from |
Top stuff. Thanks Mr Bynens! |
Add XHR `responseType` tests
I’ve used the same tests here: http://mathiasbynens.be/demo/xhr-responsetype