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
Jest tests for Apex recipes #58
Conversation
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.
Nice! Some comments for you to review.
const buttonEl = element.shadowRoot.querySelector('lightning-button'); | ||
buttonEl.dispatchEvent(new CustomEvent('click')); | ||
|
||
jest.runAllTimers(); |
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.
Is this line really necessary? runAllTimers
will flush the macro-task queue but the only macro-task here is the one in flushPromises
that hasn't been called yet.
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.
Removed.
return new Promise(resolve => setImmediate(resolve)); | ||
} | ||
|
||
it('lists two contacts based on user input', () => { |
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.
For this case the two contacts aren't actually from the user input. They're hardcoded above in this test. Maybe something more like "renders contacts returned from imperative Apex method call".
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.
Changed.
return new Promise(resolve => setImmediate(resolve)); | ||
} | ||
|
||
it('lists two contacts based on user input', () => { |
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.
This only renders a single contact. It's also not actually based on user input.
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.
Changed.
const buttonEl = element.shadowRoot.querySelector('lightning-button'); | ||
buttonEl.dispatchEvent(new CustomEvent('click')); | ||
|
||
jest.runAllTimers(); |
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'm skeptical about the need for fake timers in this test as well.
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.
Removed.
|
||
const inputEl = element.shadowRoot.querySelector('lightning-input'); | ||
inputEl.value = 'Taylor'; | ||
inputEl.dispatchEvent(new CustomEvent('change')); |
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.
We're updating the input here to set the searchKey
, which will be passed as a param to findContacts
after the button is clicked, but we never use or verify the searchKey
value.
It'd be nice to have a test case for that. Since findContacts
is a Jest mock function we should be able to verify the parameters on the invocation.
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.
Added.
import { createElement } from 'lwc'; | ||
import ApexWireMethodToProperty from 'c/apexWireMethodToProperty'; | ||
import { registerLdsTestWireAdapter } from '@salesforce/wire-service-jest-util'; | ||
import getContactList from '@salesforce/apex/ContactController.getContactList'; |
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.
Same comment as apexStaticSchema.test.js above. After upgrading to lwc-jest, import from lwc-jest and use registerApexTestWireAdapter
.
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.
Changed.
import { createElement } from 'lwc'; | ||
import ApexWireMethodWithParams from 'c/apexWireMethodWithParams'; | ||
import { registerLdsTestWireAdapter } from '@salesforce/wire-service-jest-util'; | ||
import findContacts from '@salesforce/apex/ContactController.findContacts'; |
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.
Same comment as apexStaticSchema.test.js above. After upgrading to lwc-jest, import from lwc-jest and use registerApexTestWireAdapter
.
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.
Changed.
force-app/main/default/lwc/apexWireMethodWithParams/__tests__/apexWireMethodWithParams.test.js
Show resolved
Hide resolved
}); | ||
|
||
describe('findContacts @wire data', () => { | ||
it('with one record', () => { |
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.
What is the behavior you're verifying here? findContacts @wire data > with one record
explains the scenario you're executing but not what you're expecting to happen.
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.
Description changed
|
||
const inputEl = element.shadowRoot.querySelector('lightning-input'); | ||
inputEl.value = 'Amy'; | ||
inputEl.dispatchEvent(new CustomEvent('change')); |
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.
Similar to my comment in apexImperativeMethodWithParams.test.js, this user input does not affect the outcome of the test since we emit the same set of test data regardless of input.
It'd be great to have a test that verifies this input is passed along to the @wire
. Try the findContactsAdapter.getLastConfig()
API.
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.
Changed.
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.
Few more places where we use fake timers we don't need to.
|
||
describe('c-apex-imperative-method', () => { | ||
beforeAll(() => { | ||
jest.useFakeTimers(); |
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.
Remove
const buttonEl = element.shadowRoot.querySelector('lightning-button'); | ||
buttonEl.dispatchEvent(new CustomEvent('click')); | ||
|
||
jest.runAllTimers(); |
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.
Remove
const buttonEl = element.shadowRoot.querySelector('lightning-button'); | ||
buttonEl.dispatchEvent(new CustomEvent('click')); | ||
|
||
jest.runAllTimers(); |
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.
Remove
|
||
describe('c-apex-imperative-method-with-params', () => { | ||
beforeAll(() => { | ||
jest.useFakeTimers(); |
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.
Remove
Fixes #16