-
Notifications
You must be signed in to change notification settings - Fork 199
test: add tests to GraphView
#808
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
Conversation
Add tests for `getEdgeStyle` and `getPerimeterFunction`.
WalkthroughA new test suite has been introduced for the Changes
Sequence Diagram(s)sequenceDiagram
participant TestSuite
participant GraphView
participant StyleRegistry
participant PerimeterRegistry
TestSuite->>GraphView: getEdgeStyle(cellState, ...args)
alt isLoopStyleEnabled returns true
GraphView-->>TestSuite: Return loop style (default or from cell state)
else isLoopStyleEnabled returns false
alt style is string and in StyleRegistry
GraphView->>StyleRegistry: get(styleKey)
StyleRegistry-->>GraphView: styleFunction
GraphView-->>TestSuite: Return styleFunction
else style is function
GraphView-->>TestSuite: Return style function directly
else
GraphView-->>TestSuite: Return null
end
end
TestSuite->>GraphView: getPerimeterFunction(cellState)
alt perimeter is string and in PerimeterRegistry
GraphView->>PerimeterRegistry: get(perimeterKey)
PerimeterRegistry-->>GraphView: perimeterFunction
GraphView-->>TestSuite: Return perimeterFunction
else perimeter is function
GraphView-->>TestSuite: Return perimeter function directly
else
GraphView-->>TestSuite: Return null
end
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
|
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 (1)
packages/core/__tests__/view/GraphView.test.ts (1)
1-154: Consider adding tests for potential edge cases.While the current tests provide excellent coverage of normal usage patterns, consider adding tests for edge cases such as:
- What happens when an invalid value type is provided for edge style or perimeter?
- What happens with empty string values?
- Are there any performance considerations with registry lookups that should be tested?
These additional tests would strengthen the test suite and help prepare for the future refactoring mentioned in the PR objectives.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/core/__tests__/view/GraphView.test.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/core/__tests__/view/GraphView.test.ts (1)
packages/core/src/view/style/register.ts (1)
unregisterAllEdgeStylesAndPerimeters(94-98)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: build
- GitHub Check: build (windows-2022)
- GitHub Check: build (ubuntu-22.04)
- GitHub Check: build (macos-14)
🔇 Additional comments (7)
packages/core/__tests__/view/GraphView.test.ts (7)
1-27: Proper setup with comprehensive imports and license header.The test file correctly includes the necessary imports for testing and the required components from the source code. The Apache 2.0 license header is appropriately included.
28-53: Well-structured tests forgetEdgeStylewithisLoopStyleEnabledreturning true.The tests effectively cover both scenarios: default loop style behavior and custom loop style from cell state. Using a subclass to control
isLoopStyleEnabledis a clean approach for this specific use case.
55-62: Good test isolation practices.Using
beforeEachandafterAllhooks withunregisterAllEdgeStylesAndPerimeters()ensures proper test isolation by cleaning up the registry before each test and after all tests are complete, preventing side effects between test cases.
64-73: Clear approach for controlling test conditions.The comment explains the rationale for using a manual double instead of Jest mocks, which is appropriate for this simple case. The helper function for creating graphs with the test double improves code readability.
74-114: Comprehensive test coverage forgetEdgeStylewithisLoopStyleEnabledreturning false.These tests cover all key scenarios:
- No edge style in cell state
- String edge style with no registry match
- String edge style with registry match
- Edge style with
noEdgeStyleflag- Function edge style directly
Each test verifies a specific behavior of the method, providing excellent coverage.
117-124: Good test isolation for perimeter function tests.Similar to the edge style tests, proper test isolation is ensured by using
beforeEachandafterAllhooks withunregisterAllEdgeStylesAndPerimeters().
126-153: Thorough test coverage forgetPerimeterFunction.These tests effectively cover all key scenarios for the perimeter function:
- No perimeter specified
- String perimeter with no registry match
- String perimeter with registry match
- Function perimeter directly
The tests are clear, concise, and verify the expected behavior for each scenario.



Add tests for
getEdgeStyleandgetPerimeterFunction.This change prepares refactoring that will be later done on this 2 methods.
Notes
Covers #758
Covers #767
Summary by CodeRabbit