-
Notifications
You must be signed in to change notification settings - Fork 76
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: Simplify app content library tests #14656
Conversation
📝 WalkthroughWalkthroughThis pull request revises the tests for the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
2f62256
to
a9ec8a3
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #14656 +/- ##
=======================================
Coverage 95.76% 95.76%
=======================================
Files 1914 1914
Lines 24947 24947
Branches 2855 2855
=======================================
Hits 23890 23890
Misses 799 799
Partials 258 258 ☔ View full report in Codecov by Sentry. |
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.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/app-development/features/appContentLibrary/AppContentLibrary.test.tsx
(4 hunks)frontend/libs/studio-content-library/src/index.ts
(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
frontend/app-development/features/appContentLibrary/AppContentLibrary.test.tsx
[error] 34-34: Do not shadow the global "constructor" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🔇 Additional comments (3)
frontend/libs/studio-content-library/src/index.ts (1)
11-11
: LGTM!The new type export follows the established pattern and supports the test refactoring objectives.
frontend/app-development/features/appContentLibrary/AppContentLibrary.test.tsx (2)
37-45
: LGTM!The test data setup is well-structured, properly typed, and follows best practices.
49-141
: LGTM! Test cases are well-structured and align with PR objectives.The test cases have been effectively simplified by:
- Directly invoking prop methods instead of simulating user events
- Having clear and descriptive test names
- Following single assertion principle
- Properly handling error cases
frontend/app-development/features/appContentLibrary/AppContentLibrary.test.tsx
Outdated
Show resolved
Hide resolved
frontend/app-development/features/appContentLibrary/AppContentLibrary.test.tsx
Outdated
Show resolved
Hide resolved
…Library.test.tsx Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…Library.test.tsx Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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.
Actionable comments posted: 0
🧹 Nitpick comments (4)
frontend/app-development/features/appContentLibrary/AppContentLibrary.test.tsx (4)
27-32
: Add return type annotation to the mock function.The mock function is well-structured but could benefit from explicit return type annotation for better type safety and documentation.
function mockContentLibrary( ...args: ConstructorParameters<typeof ResourceContentLibraryImpl> -): Partial<ResourceContentLibraryImpl> { +): Partial<ResourceContentLibraryImpl> & { getContentResourceLibrary: jest.Mock } { mockConstructor(...args); return { getContentResourceLibrary }; }
37-44
: Consider moving test data to a separate file.The test data is well-structured but could be moved to a separate file (e.g.,
AppContentLibrary.test.data.ts
) to improve maintainability and reusability.
68-73
: Add assertions for code list properties.While the test verifies the code list data, it could benefit from additional assertions to verify other properties of the code list component.
it('Renders with the given code lists', () => { renderAppContentLibraryWithOptionLists(); const codeListConfig = retrieveConfig().codeList; const codeListDataList = codeListConfig.props.codeListsData; const expectedData: CodeListData[] = [{ title: codeListName, data: codeList }]; expect(codeListDataList).toEqual(expectedData); // Additional assertions expect(codeListConfig.props.isLoading).toBeFalsy(); expect(codeListConfig.props.error).toBeUndefined(); });
75-141
: Group related test cases usingdescribe
blocks.The test cases for code list operations (upload, update, delete) could be grouped using a
describe
block for better organization and readability.describe('code list operations', () => { it('calls uploadOptionList with correct data when onUploadCodeList is triggered', ...); it('renders success toast when onUploadOptionList is called successfully', ...); it('renders error toast when onUploadOptionList is rejected with unknown error code', ...); it('calls updateOptionList with correct data when onUpdateCodeList is triggered', ...); it('calls updateOptionListId with correct data when onUpdateCodeListId is triggered', ...); it('calls deleteOptionList with correct data when onDeleteCodeList is triggered', ...); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/app-development/features/appContentLibrary/AppContentLibrary.test.tsx
(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build environment and run e2e test
Description
Streamlined the content library mock in the
AppContentLibrary
component tests. Since the real package isn’t rendered, triggering functions with user events is unnecessary.Related Issue
Verification
Summary by CodeRabbit
Tests
New Features