-
Notifications
You must be signed in to change notification settings - Fork 220
Enable/fix test typing #2034
Enable/fix test typing #2034
Changes from 12 commits
cdc7878
6af8c2d
6d61996
684eeee
7d73af7
8b1b535
d66ebdc
36c03de
61b4a94
3bfccdc
b21f65c
dc005ab
1f0a82b
7ced21b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -797,7 +797,7 @@ describe('useBaseList', () => { | |
</> | ||
))} | ||
|
||
<button type="button" onClick={onNewDefault} /> | ||
<button type="button" onClick={onNewDefault as any} /> | ||
</> | ||
); | ||
} | ||
|
@@ -847,7 +847,7 @@ describe('useBaseList', () => { | |
optionName: newDefaultOption, | ||
optionValue: newDefaultOptionValue, | ||
}, | ||
]); | ||
] as any); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Having to cast this as any seems odd. How come it's needed? Is there a way we can change the signature of newDefaultPrice/newDefaultOption/newDefaultOptionValue to make it compliant? |
||
|
||
expect(wrapper).toContainReactText(`Default: ${newDefaultPrice}`); | ||
expect(wrapper).toContainReactText(`Default: ${newDefaultOption}`); | ||
|
@@ -881,7 +881,7 @@ describe('useBaseList', () => { | |
optionName: newDefaultOption, | ||
optionValue: newDefaultOptionValue, | ||
}, | ||
]); | ||
] as any); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Having to cast this as any seems odd. How come it's needed? Is there a way we can change the signature of newDefaultPrice/newDefaultOption/newDefaultOptionValue to make it compliant? |
||
|
||
expect(wrapper).toContainReactText( | ||
`Default: ${newDefaultPrice}Default: ${newDefaultOption}Default: ${newDefaultOptionValue}`, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -73,7 +73,7 @@ describe('useDynamicList', () => { | |
|
||
const sort1 = wrapper | ||
.findAll('li') | ||
.map((i) => i.find(TextField).props.value); | ||
.map((i) => i.find(TextField)!.props.value); | ||
|
||
expect(sort1).toStrictEqual(['A', 'C', 'B']); | ||
}); | ||
|
@@ -235,7 +235,9 @@ describe('useDynamicList', () => { | |
}); | ||
|
||
it('handles dirty state when adding a field and resetting it', () => { | ||
const wrapper = mount(<DynamicListComponent list={randomVariants()} />); | ||
const wrapper = mount( | ||
<DynamicListComponent list={randomVariants(1)} />, | ||
); | ||
|
||
expect(wrapper).toContainReactText('Dirty: false'); | ||
|
||
|
@@ -372,7 +374,7 @@ describe('useDynamicList', () => { | |
</> | ||
))} | ||
|
||
<button type="button" onClick={onNewDefault}> | ||
<button type="button" onClick={onNewDefault as any}> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Having to cast this as any seems odd. How come it's needed? Is there a way we can change the signature of onNewDefault to make it compliant? |
||
Default | ||
</button> | ||
<button type="button" onClick={() => addItem()}> | ||
|
@@ -430,7 +432,7 @@ describe('useDynamicList', () => { | |
optionName: newDefaultOption, | ||
optionValue: newDefaultOptionValue, | ||
}, | ||
]); | ||
] as any); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Having to cast this as any seems odd. How come it's needed? Is there a way we can change the signature of newDefaultPrice/newDefaultOption/newDefaultOptionValue to make it compliant? |
||
|
||
expect(wrapper).toContainReactText(`Default: ${newDefaultPrice}`); | ||
expect(wrapper).toContainReactText(`Default: ${newDefaultOption}`); | ||
|
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.
Having to cast this as any seems odd. How come it's needed? Is there a way we can change the signature of
onNewDefault
to make it compliant?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 chose the brute force cast because the test doesn't deal with MouseEvents and I wanted to minimize changes to the original test. I followed the example of clickEvent() and changeEvent() used throughout react-forms tests. I agree it seems wonky, but I'm not sure how to avoid it without replacing
Variant[]
values with fakeReact.MouseEvent
objects.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.
Uuugh yeah, it seems that this is doing weird things - it's triggering a button onClick with an argument that is nothing to do with a MouseEvent I'm kinda surprised that this works at all. This shouldn't be using a button - is should be some custom Event type that gets triggered and then we can control the type rather than using something weird and not fit for purpose .
I guess this is fine for now - but at some point we should work out how to rework this to use a custom event rather than repurposing the onClick event