chore(#368): enforce unit tests typing#367
Conversation
jonathanbataire
changed the title
jonathanbataire
changed the title
| decodeTokenStub.returns(session); | ||
|
|
||
| await MoveLib.scheduleJob( | ||
| await (MoveLib as any).scheduleJob( |
There was a problem hiding this comment.
not sure about this one chief, do we really need this when calling static funcs?
There was a problem hiding this comment.
im mainly just adding types to whats existing without fiddling with existing logic
the any changes nothing compared to what was there
how can we handle this better?
There was a problem hiding this comment.
in this case i find it odd that a call needs some type, i assumed await MoveLib.scheduleJob(... would just work
There was a problem hiding this comment.
movelib.schedulejob takes only 2 args this test is passing 5
hence the casting
There was a problem hiding this comment.
yeah that seems wrong and i don't think these integration tests run at all. I think it can be handled in another PR. Could you create another issue for this?
| it('invalid session cannot be decoded for cookie', () => { | ||
| const session = mockChtSession(); | ||
| delete session.username; | ||
| delete (session as any).username; |
There was a problem hiding this comment.
should this be ChtSession instead?
There was a problem hiding this comment.
the properties username and password of CHTSession are readonly
meaning they cant be deleted
this test is deleting them
in order for that to happen we need to cast it to any
There was a problem hiding this comment.
makes sense, what about initializing an invalid cht session to avoid the deletion?
There was a problem hiding this comment.
i have improved this
| place: { _id: PLACE_ID } | ||
| }]); | ||
| const actual = await DisableUsers.disableUsersAt([PLACE_ID], cht); | ||
| const actual = await DisableUsers.disableUsersAt([PLACE_ID], cht as unknown as ChtApi); |
There was a problem hiding this comment.
cht as unknown as ChtApi is this some ts quirk you're working around?
There was a problem hiding this comment.
the problem is mainly how these tests were written originally
no types meaning you could write anything that doesn't fit the the argument type and things work
cht is not type CHTApi the double type casting as unkown as ChtApi is a way to force cht be type ChtApi even though it doesnt fit the original type
these mistakes were caused by lack of type enforcing
There was a problem hiding this comment.
I have made improvements to CHTApi mock logic to reduce this double casting
| const attackerCall = stub.getCalls().find(c => new URL(c.args[0].url as string).host === 'idp.example.com.attacker.com'); | ||
| expect(attackerCall, 'attacker host was visited').to.exist; | ||
| expect(attackerCall!.args[0].headers.Authorization).to.be.undefined; | ||
| expect((attackerCall!.args[0].headers as any)?.Authorization).to.be.undefined; |
There was a problem hiding this comment.
is there some http.Headers type you can use here?
freddieptf
changed the title
2 participants
fixes #368