-
Notifications
You must be signed in to change notification settings - Fork 107
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
fix #1481: исправлено Вычислить() с именами параметров процедуры +тест #1484
Conversation
WalkthroughThe pull request introduces a subtle modification to the Changes
Sequence DiagramsequenceDiagram
participant Procedure as РаботаСПараметрамиПроцедуры
participant Evaluator as Вычислить
participant Test as ТестДолжен_ПроверитьРаботоспособностьСПараметрамиПроцедуры
Test->>Procedure: Call with structured parameters
Procedure->>Evaluator: Dynamically access parameters
Procedure-->>Test: Return concatenated result
Poem
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 (
|
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)
tests/eval.os (1)
346-352
: Validate dynamic parameter usage.
Concatenating the values ofЭлемент1
andЭлемент2
viaВычислить("Элемент" + Сч)
in a loop tests the dynamic parameter handling effectively. Consider adding a negative or boundary test case where parameters might be empty or of different types, to confirm robust error handling.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/OneScript.Core/Contexts/Variable.cs
(1 hunks)tests/eval.os
(2 hunks)
🔇 Additional comments (4)
src/OneScript.Core/Contexts/Variable.cs (2)
35-37
: Clarify exception handling for invalid indexes.
Accessingiv.Value
forces evaluation of the indexed property. If the index is invalid (e.g., out of bounds), an exception might be thrown here. Ensure that this behavior is intended and that exceptions are properly handled or surfaced to the caller.
39-39
: Confirm no unintended side effects.
After the conditional block, a newVariableReference
is always returned. Make sure that, ifiv.Value
raised an exception, the code’s desired fallback or error handling behavior is properly implemented.tests/eval.os (2)
28-28
: Well-documented addition to the test suite.
AddingТестДолжен_ПроверитьРаботоспособностьСПараметрамиПроцедуры
to the test list expands coverage forВычислить
with dynamically accessed parameters. This is a good practice to ensure the fix is validated.
354-356
: Effective test procedure.
The structure passed intoРаботаСПараметрамиПроцедуры
ensures reliable coverage of the new functionality. Good job verifying the result is"Поле1Поле2"
. If future logic changes, consider adding checks for alternative input scenarios.
Спасибо! |
Было сломано в PR#1439
Summary by CodeRabbit
New Features
Tests