Conversation
|
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![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 (4)
docs/guide.md (1)
138-139: Document behavior details for assert.contains (types, case-sensitivity, defaults).The new bullet is good. Please add concise notes so users know exactly how it behaves: supported container types (Text, Collection), case-sensitivity for Text, value coercion to Text in the Text branch, and default failure messages when message is omitted. Also add a short usage snippet.
Apply this inline clarification under the new bullet:
- - `$t.assert.contains($t; $container; $value; $message)` - Text or collection must contain value + - `$t.assert.contains($t; $container; $value; $message)` - Text or collection must contain value + - Supported containers: Text, Collection + - Text containment is case-sensitive; `$value` is coerced with `String($value)` + - Unsupported container types fail the assertion + - When `$message` is omitted, a default failure message is usedAnd add a compact example to the Usage Example section:
@@ $t.assert.isNotNull($t; $result; "Function should return a result") + + // Contains examples + $t.assert.contains($t; "hello world"; "world"; "Substring should be present") + var $letters : Collection + $letters:=["a"; "b"; "c"] + $t.assert.contains($t; $letters; "b"; "Collection should contain element")testing/Project/Sources/Classes/Assert.4dm (1)
56-83: Confirm intended semantics (case-sensitivity, empty Text, and default messages) and consider small polish.Current implementation is correct: Text uses Position(String($value); $container)>0 and Collection uses indexOf($value)#-1, with sensible defaults/messages for unsupported types and non-membership.
Two follow-ups to lock behavior down and improve debuggability:
- Decide and document explicit behavior for empty substrings (String(Null) -> "" and String("") cases). Many frameworks treat empty substring as contained; if you prefer fail, guard for it and test it.
- Consider richer default failure messages that include the searched value to aid debugging.
Optional polish to include the searched value in the default non-membership message:
- This.fail($t; "Assertion failed: container does not contain value") + This.fail($t; "Assertion failed: container does not contain value '" + String:C10($value) + "'")If you want empty-text to explicitly fail (rather than whatever Position does for ""), add this guard in the Text branch:
: ($type=Is text:K8:3) - $found:=(Position:C15(String:C10($value); $container)>0) + If (String:C10($value)="") + $found:=False // decide: treat empty needle as not contained + Else + $found:=(Position:C15(String:C10($value); $container)>0) + End iftesting/Project/Sources/Classes/_AssertTest.4dm (2)
172-184: Good coverage for Text containment; add edge-case tests (case and default message).These tests validate pass/fail paths. Please add:
- Case-sensitivity check ("Hello" vs "hello").
- Default message path (omit
$message) to lock contract.Suggested additions (can be placed after this block):
Function test_contains_text_case_sensitivity($t : cs:C1710.Testing) var $mockTest : cs:C1710.Testing $mockTest:=cs:C1710.Testing.new() // Expect fail if comparison is case-sensitive $t.assert.contains($mockTest; "Hello"; "hello") $t.assert.isTrue($t; $mockTest.failed; "Case-sensitive mismatch should fail") Function test_contains_text_default_message($t : cs:C1710.Testing) var $mockTest : cs:C1710.Testing $mockTest:=cs:C1710.Testing.new() // Omit message to exercise default message $t.assert.contains($mockTest; "abc"; "z") $t.assert.isTrue($t; $mockTest.failed; "Missing value should fail") $t.assert.areEqual($t; "Assertion failed: container does not contain value"; $mockTest.logMessages[0]; "Should use default message")
185-199: Solid Collection tests; add unsupported-type and default-message coverage.Please add a test ensuring unsupported containers fail with the expected default message.
Suggested addition:
Function test_contains_unsupported_type_defaults($t : cs:C1710.Testing) var $mockTest : cs:C1710.Testing $mockTest:=cs:C1710.Testing.new() var $obj : Object $obj:=New object() // Unsupported container type should fail with default message $t.assert.contains($mockTest; $obj; "x") $t.assert.isTrue($t; $mockTest.failed; "Unsupported type should fail") $t.assert.areEqual($t; "Assertion failed: unsupported type for contains"; $mockTest.logMessages[0]; "Should use default unsupported-type message")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
docs/guide.md(1 hunks)testing/Project/Sources/Classes/Assert.4dm(1 hunks)testing/Project/Sources/Classes/_AssertTest.4dm(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
testing/Project/Sources/Classes/*Test.4dm
📄 CodeRabbit inference engine (CLAUDE.md)
testing/Project/Sources/Classes/*Test.4dm: Name test classes with the suffix "Test" so they are auto-discovered
Define test methods with the prefix "test_" within test classes to be executed by the runner
Use comment-based tagging in tests with the exact format: // #tags: tag1, tag2
Disable automatic transactions for a specific test by adding the comment: // #transaction: false; omit the comment to keep the default rollback behavior
When manual transaction control is needed (with automatic transactions disabled), use the Testing context methods: startTransaction, inTransaction, validateTransaction, cancelTransaction, or wrappers withTransaction/withTransactionValidate
Files:
testing/Project/Sources/Classes/_AssertTest.4dm
🔇 Additional comments (2)
testing/Project/Sources/Classes/Assert.4dm (1)
48-55: Indent-only tweak in isNotNull — OK.No behavioral change; reads fine.
testing/Project/Sources/Classes/_AssertTest.4dm (1)
160-171: Indentation-only changes — OK.No logic change; test intent preserved.
1 participant
Summary
assert.containsto validate substring or collection membershipTesting
make testhttps://chatgpt.com/codex/tasks/task_e_68aa359daad88324bd505a329da70ae8
Summary by CodeRabbit