remove some Jasmine specific code from tests #1809
Conversation
dynst
force-pushed
the
jasmine-removal-prep
branch
from
c3383b8 to
286373f
Compare
it's present in all supported environments.
dynst
force-pushed
the
jasmine-removal-prep
branch
3 times, most recently
from
422bb8e to
b15a071
Compare
dynst
force-pushed
the
jasmine-removal-prep
branch
from
6e34a71 to
b7efa10
Compare
| return; | ||
| } | ||
| } | ||
| const enabled = !!(globalThis.process?.env.SIMAPP47_ENABLED || globalThis.process?.env.SIMAPP50_ENABLED); |
There was a problem hiding this comment.
Can we use this simappEnabled() like below for consistency and readability?
There was a problem hiding this comment.
No, this is faucet, that function only exists in the test-only code of a completely separate package.
| const ret = new Promise<void>((resolve, reject) => { | ||
| done = resolve as typeof done; | ||
| done.fail = reject; | ||
| }); |
There was a problem hiding this comment.
This looks great. Could we deduplicate the typeof done for all of the tests? And what about calling this via done.success()/done.fail() to avoid done being a function and an object at the same time? This way we could also avoid the casts from typeof resolve to typeof done.
There was a problem hiding this comment.
No code calls done.success() (which doesn't exist in Jasmine or isn't documented?), lots of code calls done(), so this was all just written as a purely mechanical change with an absolute minimum of code changes.
It doesn't really seem possible to share testing-only code across packages... or seem worth it to add a way to do it. So any potential deduplicating would be very limited. Maybe a followup PR?
|
Thanks! |
The
fail()andpending()globals exist in no other testing suite and are completely non-portable idioms.There's good reason others didn't copy them, dynamically deciding whether or not to skip every individual test only after each one has already started executing is a bit...barbaric.