-
Notifications
You must be signed in to change notification settings - Fork 210
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
(test) O3-3853: Add Unit Tests for Datepicker Component #1126
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.
Hi @Samstar10!
Can you please connect with Sachit and combine #1130 together.
Thanks!
CC: @NethmiRodrigo
beforeAll(() => { | ||
window.i18next = { | ||
language: 'en', | ||
changeLanguage: jest.fn(), | ||
t: jest.fn((key) => key), // Simple mock for translation function | ||
// Other properties and methods that might be required | ||
init: jest.fn(), | ||
getFixedT: jest.fn(), | ||
exists: jest.fn(), | ||
loadNamespaces: jest.fn(), | ||
loadLanguages: jest.fn(), | ||
use: jest.fn(), | ||
on: jest.fn(), | ||
off: jest.fn(), | ||
dir: jest.fn(), | ||
cloneInstance: jest.fn(), | ||
} as unknown as i18nType; // Casting the mock to the correct i18n type | ||
}); |
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.
beforeAll(() => { | |
window.i18next = { | |
language: 'en', | |
changeLanguage: jest.fn(), | |
t: jest.fn((key) => key), // Simple mock for translation function | |
// Other properties and methods that might be required | |
init: jest.fn(), | |
getFixedT: jest.fn(), | |
exists: jest.fn(), | |
loadNamespaces: jest.fn(), | |
loadLanguages: jest.fn(), | |
use: jest.fn(), | |
on: jest.fn(), | |
off: jest.fn(), | |
dir: jest.fn(), | |
cloneInstance: jest.fn(), | |
} as unknown as i18nType; // Casting the mock to the correct i18n type | |
}); | |
Object.defineProperty(window, 'i18next', { | |
value: { | |
language: 'en', | |
changeLanguage: jest.fn(), | |
t: jest.fn((key) => key), // Simple mock for translation function | |
// Other properties and methods that might be required | |
init: jest.fn(), | |
getFixedT: jest.fn(), | |
exists: jest.fn(), | |
loadNamespaces: jest.fn(), | |
loadLanguages: jest.fn(), | |
use: jest.fn(), | |
on: jest.fn(), | |
off: jest.fn(), | |
dir: jest.fn(), | |
cloneInstance: jest.fn(), | |
} | |
}) |
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.
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.
Thanks for picking this up @Samstar10! Please see the suggestions and the build failures:
@@ -70,6 +70,7 @@ | |||
"@openmrs/esm-translations": "workspace:*", | |||
"@types/geopattern": "^1.2.9", | |||
"autoprefixer": "^9.8.8", | |||
"cross-env": "^7.0.3", |
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 required?
// Test to ensure that the component renders without crashing | ||
it('renders without crashing', () => { | ||
render(<OpenmrsDatePicker />); | ||
expect(screen.getByRole('group')).toBeInTheDocument(); |
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.
See the comment in #1130
const { container } = render(<OpenmrsDatePicker defaultValue={new Date(2024, 7, 27)} />); | ||
expect(container.querySelector('input')).toHaveValue('2024-08-27'); | ||
}); | ||
|
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.
Can we also add a test case to ensure that the current date is highlighted in the calendar grid? We should test this for different locales
if (input?.hasAttribute('aria-invalid')) { | ||
expect(input).toHaveAttribute('aria-invalid', 'false'); | ||
} else { | ||
expect(input).not.toHaveAttribute('aria-invalid'); | ||
} |
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 don't think this is the right way to do this. We should check that dates outside the min and max range are disabled.
}); | ||
|
||
// Test to ensure the component handles edge cases like `minDate` and `maxDate` correctly | ||
it('handles edge cases gracefully', () => { |
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.
it('handles edge cases gracefully', () => { | |
it('disables dates that are outside the given min and max range', () => { |
@Samstar10 are you still working on this? Is it clear to you how to move forward with it? |
Requirements
feat
,fix
, orchore
, among others). See existing PR titles for inspiration.For changes to apps
If applicable
Summary
This PR includes unit tests for the DatePicker component as mentioned in this ticket (https://openmrs.atlassian.net/issues/O3-3853).
Screenshots
Related Issue
Other