search

web-platform-tests / interop

web-platform-tests Interop project
https://wpt.fyi/interop
278 stars 28 forks source link

Exclude changes to gradient-interpolation-method-computed.html and color-computed-color-mix-function.html #600

Closed nairnandu closed 9 months ago

nairnandu commented 10 months ago

Test List

Rationale

There were changes made to these tests, without an associated test-change-proposal:

nairnandu commented 9 months ago

@nt1m / @gsnedders could you please review for WebKit? @jgraham / @zcorpan could you please review for Gecko?

nt1m commented 9 months ago

As mentioned in the meeting, these tests were invalid in the first place, the commits fix them.

They both test core functionality, so I would be against removing them.

nairnandu commented 9 months ago

adding @mysteryDate as well for review

mysteryDate commented 9 months ago

For https://github.com/web-platform-tests/wpt/commit/892583e4adb98da9b63c6ceaf1e0896f2cfb71e7 :

I know that hwb/hsl serialize as legacy srgb, which never serializes "none" and simply swaps it for zero at parse time. According to the spec "During serialization, any missing values are converted to 0." and the tests agree. But in this instance, the colors are being serialized as non-legacy color(srgb ... ) format, so the "nones" should remain in my opinion. Regardless, I think we should have had a test change proposal.

Could you also explain your reasoning here: https://github.com/web-platform-tests/wpt/commit/79c26e1b48072705e936333a5443aa2aaf829809

I don't understand what this change is meant to be about.

I would ideally like this to just go through a normal test change proposal so we can have an obvious documentation of the reasoning for the test changes.

mysteryDate commented 9 months ago

Ah, I now see this

https://github.com/web-platform-tests/wpt/issues/42677

I'll respond there. I have no further objections here.

nt1m commented 9 months ago

The commit message from https://github.com/web-platform-tests/wpt/commit/79c26e1b48072705e936333a5443aa2aaf829809 says it all fwiw, computed value serialization should resolve the positions. Which is something neither Blink or WebKit are doing correctly in this case atm.

mysteryDate commented 9 months ago

Gotcha, I didn't see the spec link in the commit message, my apologies.