Add tests for body property handling in Request constructor #4612
Conversation
|
Notifying @jdm and @youennf. (Learn how reviewing works.) |
|
These new tests look good to me. I'm not going to merge them until someone else says that they're testing the cases we care about for the fetch PRs mentioned in the original comment. |
|
cc: @annevk |
| assert_true(bodyRequest.bodyUsed , "bodyUsed is true when request is disturbed"); | ||
| assert_equals(bodyRequest.body, originalBody, "body should not change"); | ||
| assert_not_equals(originalBody, undefined, "body should not be undefined"); | ||
| assert_not_equals(originalBody, null, "body should not be null"); |
There was a problem hiding this comment.
Can we add a test here about what it should be? Either instanceof/typeof or some such?
And perhaps we should also have a test where the initial request does not have a body?
There was a problem hiding this comment.
Oops, I found that I forgot to expose body property on Request. I'll make another fetch spec PR...
There was a problem hiding this comment.
Anyway, I added some assertions.
There was a problem hiding this comment.
Maybe just add that to the existing PR? Then we can use these tests for both.
There was a problem hiding this comment.
Never mind, what you did is fine.
| assert_equals(bodyRequest.body, originalBody, "body should not change"); | ||
| assert_not_equals(originalBody, undefined, "body should not be undefined"); | ||
| assert_not_equals(originalBody, null, "body should not be null"); | ||
| }, "Input request used for creating new request became disturbed"); |
There was a problem hiding this comment.
This test is basically duplicated no as a promise_test, no? Maybe it's better to just remove it? Or am I missing something subtle?
There was a problem hiding this comment.
Oh wait, the difference is that the promise_test passes a body. Disregard the above comment please.
4 participants
This change is for whatwg/fetch#425 and whatwg/fetch#458.