Web Platform Tests part 2 #90
Conversation
olegbespalov
requested review from
mstoykov and
joanlopez
and removed request for
a team
|
Also @mstoykov huge thanks for the suggestion about the approach, and @joanlopez, for the implementation that I used as the base, I really love how now everything is structured 😍 |
olegbespalov
force-pushed
the
feat/wpt-part-2
branch
2 times, most recently
from
3b7a584 to
ee44ef6
Compare
webcrypto/tests/util/testharness.js
Outdated
| function assert_unreached(description) { | ||
| throw `reached unreachable code, reason: ${description}` | ||
| } |
There was a problem hiding this comment.
Do we need this "adaptation" ? can we have a diff for it as well?
There was a problem hiding this comment.
Good question, it's something from the original implementation which probably should cover this, let me check quickly if it can be dropped
There was a problem hiding this comment.
Well, quickly checking, I think we need this 😢 It's the only place where it's defining, keeping in mind that we don't fully rely on the original testharness.js and helpers.js
There was a problem hiding this comment.
Yeah, but the questions is more "can we just really on the original ones?"
Or can we have diff patch against the original ones (like we have for the tests) and run them patched instead.
There was a problem hiding this comment.
Probably, yes. Not sure, though, if it's just a simple replacement. Perhaps if it was possible, we started to use them at the moment of beginning the webcrypto. How about opening a separate issue? Ideally, we should use the original test framework as much as possible.
There was a problem hiding this comment.
Okay, after re-looking to streams, it seems like it's not a big deal, so I applied the fixed. It removes the need to any overload (even if test time increased a bit 🤔 )
olegbespalov
force-pushed
the
feat/wpt-part-2
branch
from
ee44ef6 to
aed5017
Compare
olegbespalov
force-pushed
the
feat/wpt-part-2
branch
from
aed5017 to
49d0c6c
Compare
What?
This primarily applies suggestions that @mstoykov made in #87.
It also moves tests around, and the reason is to have the chance to test suites separately in CI (before the PR, we run the regular tests twice)
Why?
This simplifies the maintenance of the tests and improves the developer's UX.