-
Notifications
You must be signed in to change notification settings - Fork 4
test: Add test to validate wrap around in select list works. #22
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
📝 WalkthroughWalkthroughNew test cases have been added for both the multiselect and select prompts. The tests simulate user navigation inputs—pressing down arrow keys to traverse options and wrapping around from the last to the first option. One test checks that the multiselect prompt correctly selects the first option after wrapping, while the other verifies similar behavior for the single select prompt. Existing tests remain unchanged, with these additions enhancing coverage for prompt wrap-around behavior. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant T as Test Harness
participant MS as MultiSelectPrompt
U->>T: Send down arrow key presses
T->>MS: Forward key inputs
MS->>MS: Execute wrap-around logic
MS-->>T: Return first option selected
T->>U: Present the multiselect result
sequenceDiagram
participant U as User
participant T as Test Harness
participant S as SelectPrompt
U->>T: Send down arrow key presses
T->>S: Forward key inputs
S->>S: Execute wrap-around logic
S-->>T: Return first option selected
T->>U: Present the select prompt result
Poem
🪧 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 (
|
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
example/main.dart(1 hunks)lib/src/analytics/analytics.dart(4 hunks)lib/src/better_command_runner/better_command.dart(1 hunks)lib/src/better_command_runner/better_command_runner.dart(5 hunks)lib/src/logger/helpers/progress.dart(3 hunks)lib/src/logger/logger.dart(3 hunks)lib/src/logger/loggers/std_out_logger.dart(7 hunks)lib/src/package_version/package_version.dart(3 hunks)lib/src/package_version/pub_api_client.dart(1 hunks)lib/src/prompts/confirm.dart(1 hunks)lib/src/prompts/select.dart(5 hunks)test/better_command_runner/analytics_test.dart(4 hunks)test/better_command_runner/command_test.dart(2 hunks)test/better_command_runner/default_flags_test.dart(3 hunks)test/better_command_runner/exit_exceptions_test.dart(1 hunks)test/better_command_runner/logging_test.dart(1 hunks)test/better_command_runner/parse_log_level_test.dart(1 hunks)test/better_command_test.dart(1 hunks)test/documentation_generator/generate_markdown_test.dart(1 hunks)test/local_storage_manager/local_storage_manager_test.dart(11 hunks)test/package_version_test.dart(3 hunks)test/prompts/confirm_test.dart(2 hunks)test/prompts/input_test.dart(1 hunks)test/prompts/multiple_select_test.dart(9 hunks)test/prompts/select_test.dart(5 hunks)test/pub_api_client_test.dart(1 hunks)test/test_utils/io_helper.dart(1 hunks)test/test_utils/mock_stdin.dart(9 hunks)test/test_utils/prompts/key_code_sequence.dart(1 hunks)test/test_utils/prompts/option_matcher.dart(1 hunks)
✅ Files skipped from review due to trivial changes (22)
- test/test_utils/prompts/key_code_sequence.dart
- test/better_command_runner/analytics_test.dart
- lib/src/prompts/select.dart
- lib/src/better_command_runner/better_command.dart
- test/better_command_runner/command_test.dart
- test/local_storage_manager/local_storage_manager_test.dart
- test/better_command_runner/logging_test.dart
- test/test_utils/io_helper.dart
- test/prompts/input_test.dart
- lib/src/logger/loggers/std_out_logger.dart
- test/better_command_runner/exit_exceptions_test.dart
- test/better_command_runner/default_flags_test.dart
- lib/src/package_version/pub_api_client.dart
- lib/src/logger/helpers/progress.dart
- test/better_command_test.dart
- test/prompts/confirm_test.dart
- lib/src/analytics/analytics.dart
- lib/src/logger/logger.dart
- example/main.dart
- lib/src/better_command_runner/better_command_runner.dart
- lib/src/package_version/package_version.dart
- test/test_utils/mock_stdin.dart
🚧 Files skipped from review as they are similar to previous changes (2)
- test/prompts/select_test.dart
- test/prompts/multiple_select_test.dart
🔇 Additional comments (12)
test/better_command_runner/parse_log_level_test.dart (1)
107-115: LGTM! The formatting changes improve readability.The test case has been reformatted with better line breaks and indentation while preserving the original functionality.
However, this change appears unrelated to the PR's objective of adding wrap-around tests for select lists.
Likely an incorrect or invalid review comment.
test/test_utils/prompts/option_matcher.dart (1)
55-57: LGTM! Clean formatting change.The multi-line formatting improves readability while maintaining the same functionality.
test/pub_api_client_test.dart (2)
1-164: Verify the relevance of changes to PR objectives.The changes in this file appear to be purely formatting-related, while the PR objectives mention adding tests for wrap-around behavior in select lists. This file tests pub API client functionality and doesn't contain any select list related tests.
Could you please confirm if the changes to this file were intentional? The actual tests for wrap-around behavior in select lists seem to be in
test/prompts/multiple_select_test.dartandtest/prompts/select_test.dart.
29-48: LGTM! The formatting changes improve readability.The reformatting of test case definitions enhances readability by:
- Using consistent indentation
- Breaking long lines appropriately
- Adding descriptive test names
- Maintaining clear separation between test setup, execution, and assertions
Also applies to: 51-70, 73-94, 97-118, 120-140, 143-163
test/documentation_generator/generate_markdown_test.dart (2)
103-122: LGTM! Well-structured test assertion for command documentation.The expected markdown output is properly formatted with clear sections for usage, description, and subcommands.
129-159: LGTM! Comprehensive test coverage for subcommand documentation.The test properly verifies the markdown generation for subcommands, including their descriptions, usage syntax, and available options.
test/package_version_test.dart (5)
1-149: Inconsistency between PR objectives and file changes.The PR title and description mention adding tests for wrap-around behavior in select lists. However, this file contains package version tests. Please verify if:
- The correct files are included in the review
- The PR description accurately reflects the intended changes
Likely an incorrect or invalid review comment.
9-26: LGTM! Test case validates cached version usage.The test correctly verifies that when a cached version is still valid (within its "valid until" time), it is returned without attempting to fetch a new version.
28-64: LGTM! Comprehensive test for expired cache scenario.The test thoroughly validates the behavior when cached version has expired:
- Fetches new version from pub.dev
- Stores the updated version with correct validity time
- Returns the new version
93-120: LGTM! Test case validates first-time version fetch.The test thoroughly verifies the behavior when fetching version for the first time:
- Successfully fetches version
- Stores it with correct validity time
- Returns the fetched version
122-147: LGTM! Test case validates error handling.The test properly verifies the behavior when version fetch fails:
- Stores appropriate retry timeout
- Returns null to indicate failure
lib/src/prompts/confirm.dart (1)
13-18: LGTM! Clear and intuitive prompt formatting.The changes improve user experience by clearly indicating the default choice through case sensitivity ([Y/n] or [y/N]). The implementation is clean and handles all cases correctly.
christerswahn
left a comment
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.
Looks good, just one comment!
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)
test/prompts/multiple_select_test.dart (1)
235-260: LGTM! Comprehensive test coverage for wrap-around functionality.The test effectively validates that navigating past the first option wraps around to the last option. Together with the previous test, this provides complete coverage of wrap-around behavior.
Consider adding a test case that combines both up and down navigation to ensure smooth transitions in both directions. For example:
test('Given multiple select prompt with multiple options ' 'when navigating up and down past boundaries ' 'then wraps around correctly in both directions', () async { late Future<List<Option>> result; var options = [Option('Option 1'), Option('Option 2'), Option('Option 3')]; await collectOutput( keyInputs: [ ...arrowUpSequence, // Move to Option 3 ...arrowDownSequence, // Move to Option 1 ...arrowDownSequence, // Move to Option 2 KeyCodes.space, // Select Option 2 KeyCodes.enterCR, ], () { result = multiselect( 'Choose multiple options:', options: options, logger: logger, ); }, ); await expectLater( result, completion(containsAllOptions([Option('Option 2')])), ); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
test/prompts/multiple_select_test.dart(9 hunks)test/prompts/select_test.dart(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- test/prompts/select_test.dart
🔇 Additional comments (2)
test/prompts/multiple_select_test.dart (2)
12-175: LGTM! Clean formatting improvements.The formatting changes improve readability while preserving the original test functionality.
206-233: LGTM! Well-structured test for wrap-around functionality.The test effectively validates that navigating past the last option wraps around to the first option, with clear steps and appropriate assertions.
christerswahn
left a comment
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.
LGTM! 🚀
Add tests to validate wrap around in select list works as expected.
Bonus
Summary by CodeRabbit
Summary by CodeRabbit
CommandDocumentationGeneratorto ensure correct markdown formatting.