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

WPT test for Blob constructor has unexpected behaviour #11075

Closed
izgzhen opened this issue May 8, 2016 · 6 comments
Closed

WPT test for Blob constructor has unexpected behaviour #11075

izgzhen opened this issue May 8, 2016 · 6 comments
Labels

Comments

@izgzhen
Copy link
Contributor

@izgzhen izgzhen commented May 8, 2016

In tests/wpt/metadata/FileAPI/blob/Blob-constructor.html.ini, the second test:

 [Passing an platform object that supports indexed properties as the blobParts array should work (window).]
expected: FAIL

you can delete this line, modify it to PASS or whatever else, the test will still pass.

Look at the tests/wpt/web-platform-tests/FileAPI/blob/Blob-constructor.html and we found:

test_blob(function() {
  var select = document.createElement("select");
  select.appendChild(document.createElement("option"));
  return new Blob(select);
}, {
  expected: "[object HTMLOptionElement]",
  type: "",
  desc: "Passing an platform object that supports indexed properties as the blobParts array should work (select)."
});

First, it is select but not window, but even if I changed the select to window, the test is still not working.

@izgzhen
Copy link
Contributor Author

@izgzhen izgzhen commented May 8, 2016

Ok ... I found another one testing select ... I will fix the test script later

@izgzhen
Copy link
Contributor Author

@izgzhen izgzhen commented May 8, 2016

@Ms2ger [Passing an platform object that supports indexed properties as the blobParts array should work (window with custom toString).] and [Passing an platform object that supports indexed properties as the blobParts array should work (window).] seem not existing in the Blob-constructor.html anymore. Should I delete the corresponding testing entry?

(BTW, looks like the ./mach test doesn't complain even if some test in ini doesn't exist)

@cbrewster cbrewster added the A-testing label May 8, 2016
@jdm
Copy link
Member

@jdm jdm commented May 8, 2016

Yes, INI entries that don't correspond to actual tests are ignored by the test harness. It can lead to confusion, but I'm not sure if it's a big deal.

@izgzhen
Copy link
Contributor Author

@izgzhen izgzhen commented May 8, 2016

@jdm Ok, if "nothing happens" == "everything is good" then then it looks sensible to ignore it. I will fix this manually later.

izgzhen added a commit to izgzhen/servo that referenced this issue May 8, 2016
bors-servo added a commit that referenced this issue May 8, 2016
Delete outdated test cases ini, addressing issue #11075

As discussed in issue #11075 , these two are no longer found in current `tests/wpt/web-platform-tests/FileAPI/blob/Blob-constructor.html`

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11078)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue May 8, 2016
Delete outdated test cases ini, addressing issue #11075

As discussed in issue #11075 , these two are no longer found in current `tests/wpt/web-platform-tests/FileAPI/blob/Blob-constructor.html`

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11078)
<!-- Reviewable:end -->
@timvandermeij
Copy link
Contributor

@timvandermeij timvandermeij commented May 9, 2016

Fixed by b93409b ?

@cbrewster cbrewster closed this May 9, 2016
@cbrewster
Copy link
Member

@cbrewster cbrewster commented May 9, 2016

Fixed by #11078

zakorgy added a commit to zakorgy/servo that referenced this issue May 26, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.