-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add tests for remaining actions #18
Conversation
Test output:
|
this is beautiful!! am interested in adding github CI/CD on this next. will take a quick look |
added here 65beaa1 |
@@ -3,5 +3,12 @@ import * as jsdom from 'jsdom'; | |||
const window = new jsdom.JSDOM('<main></main>').window; | |||
global.document = window.document; | |||
global.window = window; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
type error:
Error: src/test.ts(5,1): error TS2322: Type 'DOMWindow' is not assignable to type 'Window & typeof globalThis'.
Type 'DOMWindow' is not assignable to type 'Window'.
Types of property 'self' are incompatible.
Type 'DOMWindow' is not assignable to type 'Window & typeof globalThis'.
Type 'DOMWindow' is missing the following properties from type 'typeof globalThis': RTCStatsReport, AnalyserNode, Animation, AnimationEffect, and 489 more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll open a separate PR to fix these. Didn't realize they were build breaking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
window.dispatchEvent(event); | ||
|
||
assert.ok(preventDefaultSpy.calledOnce); | ||
assert.ok(returnValSpy.set.calledWith('')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
type error
Argument of type '""' is not assignable to parameter of type 'boolean | SinonMatcher | undefined'.ts(2345)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in #20
Resolves #5
This adds tests for the remaining actions:
It also adds cleanup to the existing clickOutside tests -- the nodes created for the test were not being removed from the DOM and the action wasn't being destroyed.
Writing these tests discovered a bug where the shortcut action was not updating params. This PR fixes that bug as well.