Closed Bug 1121603 Opened 10 years ago Closed 9 years ago

fix issues causing failures in github/fetch polyfill tests

Categories

(Core :: DOM: Core & HTML, defect)

Product:
Core
Component:
DOM: Core & HTML
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: bkelly, Unassigned)

References

Details

The github fetch polyfill has a nice test suite.  We should investigate what is causing these to fail in latest nightly.

To run the tests:

  git clone git@github.com:github/fetch.git

The open navigate in your browser to the test.html file in the repo.  On my machine:

  git@github.com:github/fetch.git

On rev 3121f461b090 we pass 12 of 26 tests.
A lot of these tests use .json() to test the result.

Looks like we are failing to set the response status code correctly in many cases, though.
Depends on: 1072144
Ben, you need to first |npm install .| in the fetch/ dir, then go into test and run |node server.js|, then visit localhost:3000/test/test.html, so that the custom server can return the right responses. That way, I'm getting pass 32, fail 6. Looking into them.
Thats much better!  It looks like there are 6 tests that use .json.
Sorry, above comment was with some extra patches applied locally.
Just checking, are we observing CSP and Mixed Content? (Since it seems we don't have the Fetch layering quite yet as we don't get a lot for free...)
I just tried with my freshly updated nightly and it did indeed crash when running the tests.  I wonder if we have a bad non-DEBUG path.

I think we should leave this one open until we can verify with the nightly release build.
Keywords: leave-open
(In reply to Anne (:annevk) from comment #5)
> Just checking, are we observing CSP and Mixed Content? (Since it seems we
> don't have the Fetch layering quite yet as we don't get a lot for free...)

CSP support for the "fetch" context is implemented in Bug 1082924.
(In reply to Ben Kelly [:bkelly] from comment #6)
> I just tried with my freshly updated nightly and it did indeed crash when
> running the tests.  I wonder if we have a bad non-DEBUG path.
> 
> I think we should leave this one open until we can verify with the nightly
> release build.

What crashed? Was it the .json() call?
Once formData() lands, and with this pull request - https://github.com/github/fetch/pull/86, all the tests pass.
Depends on: 1109751
Keywords: leave-open
formData() not being implemented is a feature and not a bug. We have patches ready for it depending on a few other FormData bugs.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.