web-platform-tests / rfcs

web-platform-tests RFCs
75 stars 63 forks source link

RFC 65: Switching to "Py3-first" for WPT tests on CI #65

Closed ziransun closed 3 years ago

stephenmcgruer commented 3 years ago

Is there a proposal for the date when these changes will occur?

Not yet. I would suggest that we make it part of the RFC to say we aim to switchover shortly after merge, but will announce the switchover in [PLACES] with X days notice.

Before we get there, we need to determine what are the list of blocking issues versus which aren't blocking. The blocking issues I know of are:

Are there any other blocking issues folks are aware of?

Hexcles commented 3 years ago

The threading issue - this is our riskiest blocker.

With a tight-ish time frame in front us, I think we should take https://github.com/web-platform-tests/wpt/pull/26111 to work around this for now. We talked about experimenting with multiprocessing; I fully agree this is something we should try, but we probably don't want to block the migration on the experiment.

stephenmcgruer commented 3 years ago

With a tight-ish time frame in front us, I think we should take web-platform-tests/wpt#26111 to work around this for now. We talked about experimenting with multiprocessing; I fully agree this is something we should try, but we probably don't want to block the migration on the experiment.

Sorry, last I checked I thought the fix did not actually address the encoding problem. Did the locking added in https://github.com/web-platform-tests/wpt/pull/26111/commits/b7fa47294b8c4646b77a53287384b90554974fc2 definitely resolve the encoding import errors?

Hexcles commented 3 years ago

Sorry, should've made that clear in the PR. The answer is yes, but see https://github.com/web-platform-tests/wpt/pull/26111#pullrequestreview-508523896 for caveats...

stephenmcgruer commented 3 years ago

Sorry, should've made that clear in the PR. The answer is yes, but see web-platform-tests/wpt#26111 (review) for caveats...

I see. That changes my opinion on this; if we have a valid (albeit scary) workaround and are committed to finding a proper path forward that will remove the workaround (which we are; I am happy to own that myself), I don't think the threading issue should block the RFC then.

@jgraham , WDYT about the threading issue given this?

jgraham commented 3 years ago

Ideally I'd like us to try the absolute import thing first. If we can't make that work we could fall back to the existing patch, but I think we should be very wary of taking something that pokes at so many internals; if we drop the ball on fixing the underlying issue it seems almost certain we'll see breakage at some point.

stephenmcgruer commented 3 years ago

Ideally I'd like us to try the absolute import thing first. If we can't make that work we could fall back to the existing patch, but I think we should be very wary of taking something that pokes at so many internals; if we drop the ball on fixing the underlying issue it seems almost certain we'll see breakage at some point.

Ack, I can own experimenting with that over the next few (business) days.

stephenmcgruer commented 3 years ago

Ack, I can own experimenting with that over the next few (business) days.

To keep folks up-to-date on this, I am playing with it in https://github.com/web-platform-tests/wpt/pull/26328 . My next task is to diff a set of trigger runs that I did for Chrome Dev.

stephenmcgruer commented 3 years ago

An update:

EDIT: RFC filed for Python file handler change - https://github.com/web-platform-tests/rfcs/pull/68

stephenmcgruer commented 3 years ago

So I generally agree with everything here, but I'd like a Python 3 meta RFC formalizing the timeline for all the steps, rather than just having an RFC for one part of the process and the overall framework in an issue. I can write that.

I mean, no objection since you're willing to write it, but isn't that two lines?

jgraham commented 3 years ago

I've written #69 to detail the overall transition plan.

The one thing I realised while writing that that isn't covered here is the behaviour of wpt --py3; I think we need a --py2 option during the transition. I can make a PR for that.

jgraham commented 3 years ago

https://github.com/web-platform-tests/wpt/pull/26507