-
Notifications
You must be signed in to change notification settings - Fork 750
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 useQuizResources and update functionality for annotation and loading states #13080
base: develop
Are you sure you want to change the base?
Add tests for useQuizResources and update functionality for annotation and loading states #13080
Conversation
…n and loading states
Build Artifacts
|
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.
Thank you for the test suite - the tests look okay to me on my first read.
I wonder if the tests would pass without the changes made to useQuizResources
- I'm not seeing the purpose for the changes to the implementation there so clarification there would be appreciated.
Why did you remove all of the JSDoc definitions from useQuizResources
? To get this ready for merge, please undo the diff on useQuizResources
altogether and see if the tests pass - if they don't, then please revert all changes to comments in the file and only commit the necessary implementation changes to pass the test suite.
Then we can look for some manual testing of the underlying components.
const api = { | ||
setResources, | ||
resources: computed(() => get(_resources)), | ||
loading: computed(() => get(_loading) || get(treeLoading)), | ||
loadingMore: computed(() => get(_loadingMore)), | ||
hasMore, | ||
topic, | ||
annotateTopicsWithDescendantCounts, | ||
fetchQuizResources: undefined, | ||
fetchMoreQuizResources: undefined, | ||
}; |
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 purpose of defining the api in a separate object here?
Hi @nucleogenesis , did the suggested changes . You can have a look and confirm if it meets all requirements or needs further improvements. Thanks! |
Summary
This PR adds unit tests for the new useQuizResources module. The module extends the already-tested useFetchTree module, so in these tests the useFetchTree functionality is mocked to return a suitable data structure. The tests verify that:
Manual testing was performed by running the full test suite locally, and all tests passed.
References
Addresses #12033
Reviewer guidance