tests(driver): type check #10123
tests(driver): type check #10123
Conversation
| inspectablePromise.getDebugValues = () => ({resolvedValue, rejectionError}); | ||
|
|
||
| return inspectablePromise; | ||
| return Object.assign(inspectablePromise, { |
There was a problem hiding this comment.
This pattern allows type script to infer the type correctly, while not changing the prototype of the return value (it is still a Promise).
| /** | ||
| * @template {keyof LH.CrdpCommands} C | ||
| * @param {C} command | ||
| * @param {RecursivePartial<LH.CrdpCommands[C]['returnType']>=} response |
There was a problem hiding this comment.
Test code often doesn't provide a full protocol response, so this makes everything optional (yet well-typed for the properties that are given).
| @@ -28,8 +28,9 @@ declare global { | |||
|
|
|||
| /** Make optional all properties on T and any properties on object properties of T. */ | |||
| type RecursivePartial<T> = { | |||
| [P in keyof T]+?: T[P] extends object ? | |||
| RecursivePartial<T[P]> : | |||
| [P in keyof T]+?: | |||
There was a problem hiding this comment.
This change adds array support.
| connect() { | ||
| return Promise.resolve(); | ||
| } | ||
| disconnect() { | ||
| return Promise.resolve(); | ||
| } | ||
| replayLog() { | ||
| redirectDevtoolsLog.forEach(msg => this.emit('protocolevent', msg)); | ||
| // @ts-ignore |
There was a problem hiding this comment.
why do we need this one isn't redirectDevtoolsLog any? ts-ignore explanation would be nice :)
There was a problem hiding this comment.
it was given some ungodly mega inferred type. Fixed at the import level.
| @@ -587,6 +604,7 @@ describe('.gotoURL', () => { | |||
| driver._waitForNetworkIdle = createMockWaitForFn(); | |||
| driver._waitForCPUIdle = createMockWaitForFn(); | |||
|
|
|||
| // @ts-ignore | |||
There was a problem hiding this comment.
is this so we don't need to create an entire passcontext?
| expect(connectionStub.sendCommand).toHaveBeenCalledTimes(1); | ||
| // Network still has one enable. | ||
|
|
||
| // @ts-ignore: Fetch is not in protocol typings yet. |
There was a problem hiding this comment.
can/should we update? or it's not even typed at all yet?
There was a problem hiding this comment.
was gonna defer to #9459 but we can just update now
Co-Authored-By: Patrick Hulce <[email protected]>
| connectionStub.sendCommand = jest.fn() | ||
| .mockImplementationOnce(() => new Promise(r => setTimeout(r), 5000)); | ||
| .mockImplementationOnce(() => new Promise(r => setTimeout(r, 5000))); |
There was a problem hiding this comment.
this is why types are important
There was a problem hiding this comment.
I'm now very concerned how this was working before, wouldn't this have resolved immediately and we shouldn't have gotten protocol_timeout? 🤔
There was a problem hiding this comment.
#10135
tldr: microtasks
connorjclark
requested review from
brendankenny and
paulirish
as code owners
|
@patrickhulce anything else you'd like to see changed here? |
| return mockFn.mock.calls.find(call => call[0] === event)[1]; | ||
| }; | ||
| const mockFn = Object.assign(mockFnImpl, { | ||
| ...mockFnImpl, |
| return mockFn.mock.calls.find(call => call[0] === event)[1]; | ||
| }; | ||
| const mockFn = Object.assign(mockFnImpl, { | ||
| ...mockFnImpl, |
There was a problem hiding this comment.
why are we assigning its spread to itself?
Previous work: #6874