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

Don't use assert_own_property in CSS tests. #16892

Merged
merged 1 commit into from May 17, 2019

Conversation

@emilio
Copy link
Contributor

emilio commented May 16, 2019

They are in fact not supposed to be own properties.

This caused all CSS tests to fail on Firefox for no good reason.

See https://bugs.chromium.org/p/chromium/issues/detail?id=700338 for the bug
that this tests are relying on.

This fixes e8b0b32. CC @lilles @ewilligers

@wpt-pr-bot wpt-pr-bot added the support label May 16, 2019
@wpt-pr-bot wpt-pr-bot requested review from frivoal and gsnedders May 16, 2019
@emilio emilio requested a review from ewilligers May 16, 2019
@emilio
Copy link
Contributor Author

emilio commented May 16, 2019

The cc's were supposed to be @lilles @ewilligers

@emilio emilio requested a review from lilles May 16, 2019
@emilio
Copy link
Contributor Author

emilio commented May 16, 2019

Per spec they're defined to be WebIDL attributes, so they should behave the same as document.body.hasOwnProperty("style").

Copy link
Contributor

gsnedders left a comment

LGTM, but maybe add an assertion message so it's clear why they fail (a second arg, something like "property not in getComputedStyle object")?

They are in fact not supposed to be own properties.

This caused all CSS tests to fail on Firefox for no good reason.

See https://bugs.chromium.org/p/chromium/issues/detail?id=700338 for the bug
that this tests are relying on.
@emilio emilio force-pushed the emilio:style-own-property branch from 5f5bb93 to 553c93b May 16, 2019
@emilio
Copy link
Contributor Author

emilio commented May 16, 2019

Sure, done. Will wait for CI just to sanity-check, but feel free to merge whenever if you get here before me :)

@emilio emilio merged commit 52cf6b3 into web-platform-tests:master May 17, 2019
10 checks passed
10 checks passed
wpt.fyi - firefox[experimental] Firefox results
Details
Azure Pipelines Build #20190516.222 succeeded
Details
Azure Pipelines (./wpt test-jobs) ./wpt test-jobs succeeded
Details
Azure Pipelines (affected tests (Safari Technology Preview)) affected tests (Safari Technology Preview) succeeded
Details
Azure Pipelines (affected tests without changes (Safari Technology Preview)) affected tests without changes (Safari Technology Preview) succeeded
Details
Azure Pipelines (wpt.fyi hook: safari-preview-affected-tests) wpt.fyi hook: safari-preview-affected-tests succeeded
Details
Azure Pipelines (wpt.fyi hook: safari-preview-affected-tests-without-changes) wpt.fyi hook: safari-preview-affected-tests-without-changes succeeded
Details
Taskcluster (pull_request) TaskGroup: success
Details
wpt.fyi - chrome[experimental] Chrome results
Details
wpt.fyi - safari[experimental] Safari results
Details
@emilio emilio deleted the emilio:style-own-property branch May 17, 2019
@annevk
Copy link
Member

annevk commented May 17, 2019

We should probably add a check for the inverse, that they're not own properties? Otherwise this passes in Chrome while they're not compliant.

@emilio
Copy link
Contributor Author

emilio commented May 17, 2019

Yeah I intentionally didn't do it since this is used in a lot of parsing and serialization tests that are important for interop. Testing it once per property is not useful, I suspect there's a test that chrome fails already because of that bug, but I'll file one otherwise.

BorisChiou added a commit to BorisChiou/wpt that referenced this pull request Jul 10, 2019
These `assert_own_property` tests cause these CSS tests to fail on Firefox
only. I think these properties are not supposted to be own properties,
and use a similar way as web-platform-tests#16892,
to test the existence of these properties.
BorisChiou added a commit to BorisChiou/wpt that referenced this pull request Jul 10, 2019
These `assert_own_property` tests cause these CSS tests to fail on Firefox
only, and I think these properties are not supposted to be own properties,
so use a similar way as web-platform-tests#16892,
to test the existence of these properties.
emilio added a commit that referenced this pull request Jul 11, 2019
…17778)

These `assert_own_property` tests cause these CSS tests to fail on Firefox
only, and I think these properties are not supposted to be own properties,
so use a similar way as #16892,
to test the existence of these properties.
marcoscaceres added a commit that referenced this pull request Jul 23, 2019
They are in fact not supposed to be own properties.

This caused all CSS tests to fail on Firefox for no good reason.

See https://bugs.chromium.org/p/chromium/issues/detail?id=700338 for the bug
that this tests are relying on.
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Jul 24, 2019
…property on testing style properties., a=testonly

Automatic update from web-platform-tests
[css-transforms] Avoid using assert_own_property on style property. (#17778)

These `assert_own_property` tests cause these CSS tests to fail on Firefox
only, and I think these properties are not supposted to be own properties,
so use a similar way as web-platform-tests/wpt#16892,
to test the existence of these properties.
--

wpt-commits: 47543ee66eb535b6910292f199f3b789bec263f0
wpt-pr: 17778
xeonchen pushed a commit to xeonchen/gecko that referenced this pull request Jul 25, 2019
…property on testing style properties., a=testonly

Automatic update from web-platform-tests
[css-transforms] Avoid using assert_own_property on style property. (#17778)

These `assert_own_property` tests cause these CSS tests to fail on Firefox
only, and I think these properties are not supposted to be own properties,
so use a similar way as web-platform-tests/wpt#16892,
to test the existence of these properties.
--

wpt-commits: 47543ee66eb535b6910292f199f3b789bec263f0
wpt-pr: 17778
natechapin added a commit to natechapin/wpt that referenced this pull request Aug 23, 2019
…eb-platform-tests#17778)

These `assert_own_property` tests cause these CSS tests to fail on Firefox
only, and I think these properties are not supposted to be own properties,
so use a similar way as web-platform-tests#16892,
to test the existence of these properties.
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 4, 2019
…property on testing style properties., a=testonly

Automatic update from web-platform-tests
[css-transforms] Avoid using assert_own_property on style property. (#17778)

These `assert_own_property` tests cause these CSS tests to fail on Firefox
only, and I think these properties are not supposted to be own properties,
so use a similar way as web-platform-tests/wpt#16892,
to test the existence of these properties.
--

wpt-commits: 47543ee66eb535b6910292f199f3b789bec263f0
wpt-pr: 17778

UltraBlame original commit: 3f420b25532cbc1cc2bb3b2b51430ec9c83b205b
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 4, 2019
…property on testing style properties., a=testonly

Automatic update from web-platform-tests
[css-transforms] Avoid using assert_own_property on style property. (#17778)

These `assert_own_property` tests cause these CSS tests to fail on Firefox
only, and I think these properties are not supposted to be own properties,
so use a similar way as web-platform-tests/wpt#16892,
to test the existence of these properties.
--

wpt-commits: 47543ee66eb535b6910292f199f3b789bec263f0
wpt-pr: 17778

UltraBlame original commit: 3f420b25532cbc1cc2bb3b2b51430ec9c83b205b
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 4, 2019
…property on testing style properties., a=testonly

Automatic update from web-platform-tests
[css-transforms] Avoid using assert_own_property on style property. (#17778)

These `assert_own_property` tests cause these CSS tests to fail on Firefox
only, and I think these properties are not supposted to be own properties,
so use a similar way as web-platform-tests/wpt#16892,
to test the existence of these properties.
--

wpt-commits: 47543ee66eb535b6910292f199f3b789bec263f0
wpt-pr: 17778

UltraBlame original commit: 3f420b25532cbc1cc2bb3b2b51430ec9c83b205b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

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