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

Test more CSSStyleSheet error cases #18892

Merged
merged 1 commit into from Sep 9, 2019
Merged

Test more CSSStyleSheet error cases #18892

merged 1 commit into from Sep 9, 2019

Conversation

foolip
Copy link
Member

@foolip foolip commented Sep 6, 2019

This is mainly to get coverage of calling deleteRule() on an empty
style, which was added and reverted with the wrong exception type:
#16342
#16353

The test is split into more subtests, but the order is kept the same
and only assert_throws tests were added in between tests to avoid
having to change anything. For this reason the order of subtests is a
bit haphazard.

This is mainly to get coverage of calling `deleteRule()` on an empty
style, which was added and reverted with the wrong exception type:
#16342
#16353

The test is split into more subtests, but the order is kept the same
and only `assert_throws` tests were added in between tests to avoid
having to change anything. For this reason the order of subtests is a
bit haphazard.
@foolip
Copy link
Member Author

foolip commented Sep 6, 2019

Alright, https://wpt.fyi/results/css/cssom/CSSStyleSheet.html?diff&filter=ADC&run_id=293320003&run_id=297180006 shows the benfit of splitting into subtests. It's now clear the problem is likely that rules returns a new object every time.

@foolip
Copy link
Member Author

foolip commented Sep 6, 2019

https://wpt.fyi/results/css/cssom/CSSStyleSheet.html?sha=78579a9fe9&label=pr_head shows Chrome and Firefox too, they pass all the tests.

@youennf FYI in case you're tracking Safari-specific failures.

@foolip foolip merged commit 8ed93b5 into master Sep 9, 2019
@foolip foolip deleted the foolip/CSSStyleSheet branch September 9, 2019 08:13
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.

None yet

3 participants