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

Teach idlharness.js to avoid invoking iterable-related members #9790

Closed
wants to merge 1 commit into from

Conversation

@foolip
Copy link
Contributor

foolip commented Mar 2, 2018

This allows uncommenting iterable declarations and at least check for
the existence of the related members.

@wpt-pr-bot wpt-pr-bot requested review from jensl and yuki3 Mar 2, 2018

@foolip foolip referenced this pull request Mar 2, 2018

Merged

Update the dom IDL file #9779

@foolip

This comment has been minimized.

Copy link
Contributor Author

foolip commented Mar 2, 2018

Reviewers of this might be interested to see #9779 (comment) as well.

@foolip foolip force-pushed the idlharness-iterable branch from 254e10a to 0fb8011 Mar 2, 2018

@wpt-pr-bot wpt-pr-bot added the infra label Mar 2, 2018

@wpt-pr-bot wpt-pr-bot requested review from ayg, gsnedders and jgraham Mar 2, 2018

@foolip

This comment has been minimized.

Copy link
Contributor Author

foolip commented Mar 7, 2018

@youennf, I found you via eccb7d1, can you review?

I'm not so sure my changes make sense, since I see the same number of tests before and after, and no tests that have "entries()" in their name like I would expect.

@lukebjerring
Copy link
Contributor

lukebjerring left a comment

Looks reasonable. I have concerns that the subsequent issue (propagate the iterable type) will never be resolved, though.

@lukebjerring lukebjerring added this to idlharness.js in Auto-import IDL files Jun 18, 2018

@foolip foolip force-pushed the idlharness-iterable branch from 0fb8011 to cc9077b Jul 27, 2018

@foolip

This comment has been minimized.

Copy link
Contributor Author

foolip commented Jul 27, 2018

Rebased to resolve conflicts, and working on the "value iterator" vs. "pair iterator" problem.

@foolip foolip moved this from idlharness.js blockers/issues to In progress in Auto-import IDL files Aug 4, 2018

@gsnedders

This comment has been minimized.

Copy link
Contributor

gsnedders commented Sep 3, 2018

Is there any reason this hasn't landed? Waiting on further reviewers?

@gsnedders

This comment has been minimized.

Copy link
Contributor

gsnedders commented Sep 3, 2018

Is there an issue open on webidl2.js for the TODO?

@foolip foolip referenced this pull request Sep 9, 2018

Closed

Update interfaces/dom.idl #12913

@foolip

This comment has been minimized.

Copy link
Contributor Author

foolip commented Sep 9, 2018

@gsnedders it's not a particularly satisfactory fix, and I've had a half-finished nicer one sitting on a branch since July.

Teach idlharness.js to avoid invoking iterable-related members
This allows uncommenting iterable declarations and at least check for
the existence of the related members.

@foolip foolip force-pushed the idlharness-iterable branch from e96a08c to 59218cb Dec 20, 2018

@lukebjerring

This comment has been minimized.

Copy link
Contributor

lukebjerring commented Dec 20, 2018

@foolip - ready or not, can you create a PR with the other branch so we can see what you think is a nicer fix?

foolip added a commit that referenced this pull request Dec 20, 2018

Revamp how idlharness.js handles iterable declarations
Instead of adding IDL interface members in `add_iterable_members` in
the style of what webidl2.js would have added if the declaration were
expanded, instead test directly for what effect a single `iterable<T>`
or `iterable<T1,T2>` declaration should have.

This is more along the lines of `test_member_stringifier`, where no
`toString` is added as IDL members.

Alternative to #9790.
@foolip

This comment has been minimized.

Copy link
Contributor Author

foolip commented Dec 20, 2018

I dug up my old branch and dusted it off: #14629

I'll close this PR because I think that's better.

@foolip foolip closed this Dec 20, 2018

Auto-import IDL files automation moved this from In progress to Done Dec 20, 2018

@foolip foolip deleted the idlharness-iterable branch Dec 22, 2018

foolip added a commit that referenced this pull request Jan 15, 2019

Revamp how idlharness.js handles iterable declarations (#14629)
Instead of adding IDL interface members in `add_iterable_members` in
the style of what webidl2.js would have added if the declaration were
expanded, instead test directly for what effect a single `iterable<T>`
or `iterable<T1,T2>` declaration should have.

This is more along the lines of `test_member_stringifier`, where no
`toString` is added as IDL members.

Alternative to #9790.

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Feb 5, 2019

Bug 1523562 [wpt PR 14629] - Revamp how idlharness.js handles iterabl…
…e declarations, a=testonly

Automatic update from web-platform-tests
Revamp how idlharness.js handles iterable declarations (#14629)

Instead of adding IDL interface members in `add_iterable_members` in
the style of what webidl2.js would have added if the declaration were
expanded, instead test directly for what effect a single `iterable<T>`
or `iterable<T1,T2>` declaration should have.

This is more along the lines of `test_member_stringifier`, where no
`toString` is added as IDL members.

Alternative to web-platform-tests/wpt#9790.
--

wpt-commits: 76bbe5aeb4e8f8fd050af8d7d4933a4a507bba27
wpt-pr: 14629

mykmelez pushed a commit to mykmelez/gecko that referenced this pull request Feb 6, 2019

Bug 1523562 [wpt PR 14629] - Revamp how idlharness.js handles iterabl…
…e declarations, a=testonly

Automatic update from web-platform-tests
Revamp how idlharness.js handles iterable declarations (#14629)

Instead of adding IDL interface members in `add_iterable_members` in
the style of what webidl2.js would have added if the declaration were
expanded, instead test directly for what effect a single `iterable<T>`
or `iterable<T1,T2>` declaration should have.

This is more along the lines of `test_member_stringifier`, where no
`toString` is added as IDL members.

Alternative to web-platform-tests/wpt#9790.
--

wpt-commits: 76bbe5aeb4e8f8fd050af8d7d4933a4a507bba27
wpt-pr: 14629

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Feb 7, 2019

Bug 1523562 [wpt PR 14629] - Revamp how idlharness.js handles iterabl…
…e declarations, a=testonly

Automatic update from web-platform-tests
Revamp how idlharness.js handles iterable declarations (#14629)

Instead of adding IDL interface members in `add_iterable_members` in
the style of what webidl2.js would have added if the declaration were
expanded, instead test directly for what effect a single `iterable<T>`
or `iterable<T1,T2>` declaration should have.

This is more along the lines of `test_member_stringifier`, where no
`toString` is added as IDL members.

Alternative to web-platform-tests/wpt#9790.
--

wpt-commits: 76bbe5aeb4e8f8fd050af8d7d4933a4a507bba27
wpt-pr: 14629

mykmelez pushed a commit to mykmelez/gecko that referenced this pull request Feb 8, 2019

Bug 1523562 [wpt PR 14629] - Revamp how idlharness.js handles iterabl…
…e declarations, a=testonly

Automatic update from web-platform-tests
Revamp how idlharness.js handles iterable declarations (#14629)

Instead of adding IDL interface members in `add_iterable_members` in
the style of what webidl2.js would have added if the declaration were
expanded, instead test directly for what effect a single `iterable<T>`
or `iterable<T1,T2>` declaration should have.

This is more along the lines of `test_member_stringifier`, where no
`toString` is added as IDL members.

Alternative to web-platform-tests/wpt#9790.
--

wpt-commits: 76bbe5aeb4e8f8fd050af8d7d4933a4a507bba27
wpt-pr: 14629
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.