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

[math functions] remove incorrect tests #337

Closed
danielsakhapov opened this issue May 16, 2023 · 8 comments
Closed

[math functions] remove incorrect tests #337

danielsakhapov opened this issue May 16, 2023 · 8 comments
Labels
focus area: CSS Math Functions test-change-proposal Proposal to add or remove tests for an interop area

Comments

@danielsakhapov
Copy link

Test List

https://wpt.fyi/results/css/css-values/exp-log-serialize.html and https://wpt.fyi/results/css/css-values/round-mod-rem-serialize.html

Rationale

We can't expect -infinity and NaN as a computed value for opacity and transform, so I propose either to remove them or test only the specified value.

@danielsakhapov danielsakhapov added the test-change-proposal Proposal to add or remove tests for an interop area label May 16, 2023
@foolip
Copy link
Member

foolip commented May 16, 2023

@danielsakhapov can these tests be updated in some way, or is there no pass condition that makes sense?

@danielsakhapov
Copy link
Author

@foolip we can test only for the specified value and not the computed one.

@foolip
Copy link
Member

foolip commented May 17, 2023

Thanks @danielsakhapov!

@jgraham can you handle this proposal for Gecko?
@gsnedders @nt1m can you handle this proposal for WebKit?

@foolip
Copy link
Member

foolip commented May 25, 2023

To make it clear, the proposal would be to only keep the tests for specified value in Interop 2023. Tests for the computed value would be removed or split out.

@jgraham
Copy link
Contributor

jgraham commented May 25, 2023

@emilio does removing the computed value tests seem reasonable here? Should we be testing some other behaviour instead?

@emilio
Copy link

emilio commented May 25, 2023

We should probably test that those produce what the spec says, which IIRC is that NaN computes to 0, and that infinity / -infinity compute to a very large (or small, respectively) but finite number.

cc @CanadaHonk

@nmoucht
Copy link

nmoucht commented Jun 7, 2023

We should probably test that those produce what the spec says, which IIRC is that NaN computes to 0, and that infinity / -infinity compute to a very large (or small, respectively) but finite number.

cc @CanadaHonk

I agree that we should change the computed value tests based on the language in the spec here:

By definition, ±∞ are outside the allowed range for any property, and will clamp to the minimum/maximum value allowed. Even properties that can explicitly represent infinity as a keyword value, such as animation-iteration-count, will end up clamping ±∞, as math functions can’t resolve to keyword values; the numeric part of the property’s syntax still has a minimum/maximum value.

@danielsakhapov
Copy link
Author

@emilio Please have a look at my change web-platform-tests/wpt#40567

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jun 26, 2023
Change tests results to see that NaN goes to 0 on top-level as per:
https://drafts.csswg.org/css-values-4/#top-level-calculation

And correctly handle the infinity expectations. Remove -infinity for
matrix as it can't round-trip.

Reviewed: #40567
Discussed: web-platform-tests/interop#337

Change-Id: I4a0a7273c9e82a7f6e21b3bf8ed877b62076e3d3
danielsakhapov added a commit to web-platform-tests/wpt that referenced this issue Jun 27, 2023
Change tests results to see that NaN goes to 0 on top-level as per:
https://drafts.csswg.org/css-values-4/#top-level-calculation

And correctly handle the infinity expectations. Remove -infinity for
matrix as it can't round-trip.

Reviewed: #40567
Discussed: web-platform-tests/interop#337

Change-Id: I4a0a7273c9e82a7f6e21b3bf8ed877b62076e3d3

Co-authored-by: Daniil Sakhapov <sakhapov@chromium.org>
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Jul 12, 2023
…handle NaN and infinity, a=testonly

Automatic update from web-platform-tests
Fix wpt tests for css math functions to handle NaN and infinity (#40567)

Change tests results to see that NaN goes to 0 on top-level as per:
https://drafts.csswg.org/css-values-4/#top-level-calculation

And correctly handle the infinity expectations. Remove -infinity for
matrix as it can't round-trip.

Reviewed: web-platform-tests/wpt#40567
Discussed: web-platform-tests/interop#337

Change-Id: I4a0a7273c9e82a7f6e21b3bf8ed877b62076e3d3

Co-authored-by: Daniil Sakhapov <sakhapov@chromium.org>
--

wpt-commits: 4e1fc86b7e59919575c9832142e8d81217b3ff02
wpt-pr: 40567
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Jul 16, 2023
…handle NaN and infinity, a=testonly

Automatic update from web-platform-tests
Fix wpt tests for css math functions to handle NaN and infinity (#40567)

Change tests results to see that NaN goes to 0 on top-level as per:
https://drafts.csswg.org/css-values-4/#top-level-calculation

And correctly handle the infinity expectations. Remove -infinity for
matrix as it can't round-trip.

Reviewed: web-platform-tests/wpt#40567
Discussed: web-platform-tests/interop#337

Change-Id: I4a0a7273c9e82a7f6e21b3bf8ed877b62076e3d3

Co-authored-by: Daniil Sakhapov <sakhapovchromium.org>
--

wpt-commits: 4e1fc86b7e59919575c9832142e8d81217b3ff02
wpt-pr: 40567

UltraBlame original commit: be20ef0ed97bfd3bebfe2cf91cb8965e9cc88ec4
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Jul 16, 2023
…handle NaN and infinity, a=testonly

Automatic update from web-platform-tests
Fix wpt tests for css math functions to handle NaN and infinity (#40567)

Change tests results to see that NaN goes to 0 on top-level as per:
https://drafts.csswg.org/css-values-4/#top-level-calculation

And correctly handle the infinity expectations. Remove -infinity for
matrix as it can't round-trip.

Reviewed: web-platform-tests/wpt#40567
Discussed: web-platform-tests/interop#337

Change-Id: I4a0a7273c9e82a7f6e21b3bf8ed877b62076e3d3

Co-authored-by: Daniil Sakhapov <sakhapovchromium.org>
--

wpt-commits: 4e1fc86b7e59919575c9832142e8d81217b3ff02
wpt-pr: 40567

UltraBlame original commit: be20ef0ed97bfd3bebfe2cf91cb8965e9cc88ec4
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Jul 16, 2023
…handle NaN and infinity, a=testonly

Automatic update from web-platform-tests
Fix wpt tests for css math functions to handle NaN and infinity (#40567)

Change tests results to see that NaN goes to 0 on top-level as per:
https://drafts.csswg.org/css-values-4/#top-level-calculation

And correctly handle the infinity expectations. Remove -infinity for
matrix as it can't round-trip.

Reviewed: web-platform-tests/wpt#40567
Discussed: web-platform-tests/interop#337

Change-Id: I4a0a7273c9e82a7f6e21b3bf8ed877b62076e3d3

Co-authored-by: Daniil Sakhapov <sakhapovchromium.org>
--

wpt-commits: 4e1fc86b7e59919575c9832142e8d81217b3ff02
wpt-pr: 40567

UltraBlame original commit: be20ef0ed97bfd3bebfe2cf91cb8965e9cc88ec4
ErichDonGubler pushed a commit to erichdongubler-mozilla/firefox that referenced this issue Jul 16, 2023
…handle NaN and infinity, a=testonly

Automatic update from web-platform-tests
Fix wpt tests for css math functions to handle NaN and infinity (#40567)

Change tests results to see that NaN goes to 0 on top-level as per:
https://drafts.csswg.org/css-values-4/#top-level-calculation

And correctly handle the infinity expectations. Remove -infinity for
matrix as it can't round-trip.

Reviewed: web-platform-tests/wpt#40567
Discussed: web-platform-tests/interop#337

Change-Id: I4a0a7273c9e82a7f6e21b3bf8ed877b62076e3d3

Co-authored-by: Daniil Sakhapov <sakhapov@chromium.org>
--

wpt-commits: 4e1fc86b7e59919575c9832142e8d81217b3ff02
wpt-pr: 40567
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
focus area: CSS Math Functions test-change-proposal Proposal to add or remove tests for an interop area
Projects
None yet
Development

No branches or pull requests

6 participants