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

Merge progress-events into XMLHttpRequest #7754

Merged
merged 3 commits into from
Oct 13, 2017
Merged

Conversation

foolip
Copy link
Member

@foolip foolip commented Oct 13, 2017

These tests are for https://www.w3.org/TR/progress-events/, and the
Editor's Draft link there redirects to https://xhr.spec.whatwg.org/.

Status.html isn't a test and is deleted.

These tests are for https://www.w3.org/TR/progress-events/, and the
Editor's Draft link there redirects to https://xhr.spec.whatwg.org/.

Status.html isn't a test and is deleted.
@ghost
Copy link

ghost commented Oct 13, 2017

Build PASSED

Started: 2017-10-13 11:54:13
Finished: 2017-10-13 11:57:05

View more information about this build on:

@annevk
Copy link
Member

annevk commented Oct 13, 2017

One of the test was a submission from Samsung. Did you verify it was correct?

@annevk
Copy link
Member

annevk commented Oct 13, 2017

Also, I don't think we necessarily have to move these over to the XMLHttpRequest directory. If we cross-link as we do in other situations it's fine.

@foolip
Copy link
Member Author

foolip commented Oct 13, 2017

The Samsung tests passes everywhere: https://wpt.fyi/progress-events/tests/submissions/Samsung

And what it's testing seems in line with https://xhr.spec.whatwg.org/#firing-events-using-the-progressevent-interface, but I didn't go looking deeply for things that may go beyond the spec.

This actually did start out as a cross-linking change, but then I looked at the tests and there were so few, and some ProgressEvent in XMLHttpRequest already, so just merging them seems better.

@foolip
Copy link
Member Author

foolip commented Oct 13, 2017

Oh, and I should update the link in those tests, now that I found a better one.

@foolip
Copy link
Member Author

foolip commented Oct 13, 2017

Oh, I managed to break firing-events-http-content-length.html, good thing I took a look at https://pulls.web-platform-tests.org/job/19448.10. #7475 will be a big improvement :)

@foolip
Copy link
Member Author

foolip commented Oct 13, 2017

The trouble is that firing-events-http-content-length.html requests resources/img.jpg, which was previously 404, but now exists. And wptserve doesn't send a Content-Length header for img.jpg, but does for a 404. So I'll have to make sure the resource used has a Content-Length header...

@foolip
Copy link
Member Author

foolip commented Oct 13, 2017

OK, fixed by using trickle.py to control whether the header is sent, and also checking the exact value. @annevk, LGTY?

assert_true(pe.loaded >= 0, "loaded is initialize to the number of HTTP entity body bytes transferred.");
assert_true(pe.lengthComputable, "lengthComputable is true.");
assert_not_equals(pe.total, 0, "total is not zero.");
assert_greater_than_equal(pe.loaded, 0, "loaded");
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I simplified the descriptions because the failure messages actually make more sense when just the actual value is described, as opposed to what it should be.

@foolip foolip merged commit 63fc3cd into master Oct 13, 2017
@foolip foolip deleted the xhr-progress-events branch October 13, 2017 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants