Add BellAssertion for terminal bell events
#109
Conversation
WalkthroughA new Changes
Sequence DiagramsequenceDiagram
participant TestCase as Test Case
participant Shell as Shell
participant Asserter as Asserter
participant BellAssertion as Bell Assertion
TestCase->>Shell: Get VTBellChannel
TestCase->>Asserter: Add BellAssertion
Asserter->>BellAssertion: Run()
BellAssertion->>Shell: Check Bell Channel
alt Bell Received
BellAssertion-->>Asserter: Return Success
else Timeout
BellAssertion-->>Asserter: Return AssertionError
end
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🪧 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 (
|
ryan-gang
changed the title
BellAssertion for terminal bell events
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
internal/assertions/bell_assertion.go (3)
7-11: Complete the documentation comment.The documentation comment for
BellAssertionis incomplete. Please complete it to describe the purpose and behavior of this assertion type.-// BellAssertion asserts that ... +// BellAssertion asserts that a bell signal is received within a specified timeout.Also, consider removing the commented out
Vtfield if it's not needed.
17-27: Simplify the conditional logic.The current implementation uses an if-else block that can be simplified.
func (a BellAssertion) Run(screenState [][]string, startRowIndex int) (processedRowCount int, err *AssertionError) { - if !checkIfBellReceived(a.BellChannel) { - return 0, &AssertionError{ - StartRowIndex: startRowIndex, - ErrorRowIndex: startRowIndex, - Message: "Expected bell to ring, but it didn't", - } - } else { - return 0, nil - } + if checkIfBellReceived(a.BellChannel) { + return 0, nil + } + return 0, &AssertionError{ + StartRowIndex: startRowIndex, + ErrorRowIndex: startRowIndex, + Message: "Expected bell to ring, but it didn't", + } }
29-36: Make timeout configurable and add documentation.The function lacks documentation and uses a hardcoded timeout value.
- Add documentation:
+// checkIfBellReceived waits for a bell signal on the provided channel with a timeout. +// Returns true if a signal is received, false if the timeout is reached. func checkIfBellReceived(bellChannel chan bool) bool {
- Make timeout configurable:
+// defaultBellTimeout is the default duration to wait for a bell signal. +const defaultBellTimeout = 100 * time.Millisecond + func checkIfBellReceived(bellChannel chan bool) bool { select { case <-bellChannel: return true - case <-time.After(100 * time.Millisecond): // Add reasonable timeout + case <-time.After(defaultBellTimeout): return false } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
internal/assertions/bell_assertion.go(1 hunks)internal/test_cases/command_autocomplete_test_case.go(1 hunks)internal/test_cases/command_multiple_completions_test_case.go(1 hunks)
🔇 Additional comments (3)
internal/assertions/bell_assertion.go (1)
13-15: LGTM!The
Inspectmethod provides a clear string representation of the assertion type.internal/test_cases/command_autocomplete_test_case.go (1)
84-100: LGTM! Clean integration of BellAssertion.The integration of
BellAssertionis well-structured:
- Retrieves bell channel from shell
- Creates and adds assertion
- Runs assertion
- Logs success
- Cleans up by popping assertion
The implementation maintains a clear flow and proper error handling.
internal/test_cases/command_multiple_completions_test_case.go (1)
76-87: LGTM! Consistent implementation of BellAssertion.The bell assertion integration follows the same pattern as in
command_autocomplete_test_case.go, maintaining consistency across test cases.
Introduce a new
BellAssertionto verify terminal bell events and refactor existing test cases to utilize this assertion, simplifying the handling of bell checks in command autocomplete and multiple completions scenarios.Summary by CodeRabbit
New Features
BellAssertionpackage for handling bell signal assertions in test cases.Refactor
Bug Fixes