Skip to content

Conversation

@devdinc
Copy link
Owner

@devdinc devdinc commented Jan 17, 2026

Summary by CodeRabbit

  • New Features

    • Introduced "single line section" expression syntax for executing multiple statements within test definitions.
  • Documentation

    • Updated test framework documentation to reflect simplified test identifiers (test names only, without script qualification).
    • Modified test event pattern and expression signatures.
  • Tests

    • Added comprehensive test coverage for the new single-line section functionality with multiple statement scenarios.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Jan 17, 2026

Warning

Rate limit exceeded

@devdinc has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 11 minutes and 59 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 1ed3cf8 and dc5af52.

📒 Files selected for processing (3)
  • scripts/libs/functionsv2.sk
  • scripts/utils/testframework.sk
  • tests/singlinesection.sk

Walkthrough

The PR simplifies test identifier semantics from script-qualified format to test-name-only identifiers across documentation and test framework declarations. It refactors the parser lifecycle in the single-line section handler, updates lambda argument passing in functions, and adds test coverage for new syntax patterns.

Changes

Cohort / File(s) Summary
Test Framework Documentation
docs/testframework.md
Removes per-script test name uniqueness requirement; shifts identifier presentation and internal storage from script-qualified ("<script>/") to test-name-only format; updates related behavior notes and examples throughout.
Test Framework DSL Declarations
scripts/utils/testframework.sk
Updates skriptTest event pattern to include [devdinc] prefix; modifies all tests plural expression signature to remove [([with]in|from) %-script%] parameter; adjusts test invocation references.
Function Lambda Argument Handling
scripts/libs/functionsv2.sk
Changes lambda argument passing from runtime args list to explicit string-quoted arguments for specific arities; adds script context initialization via {_functionsv2.sk::ctx::script} in inner lambda scenarios.
Single-Line Section Parser Refactor
scripts/libs/singlelinesection.sk
Adds java.lang.String and java.lang.Object imports; refactors parser lifecycle to capture original isActive state and manage script/event/structure setup; updates expression signature from compile line %string% with %object% to [new] [single] line section %strings%[ with [arg[ument]s [variables]] %-objects%][in %-script%]; changes dynamic test variable construction and code generation via newline joining.
Test Updates
tests/functionsv2.sk, tests/singlinesection.sk
Adds java.net.URL import and replaces HashMap test case with URL instance validation; adds two new test cases for multi-statement single-line sections with variable assignment and arithmetic operations.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • Stable #6 — Modifies test framework DSL patterns and all tests expression semantics alongside documentation updates in parallel with this PR's identifier simplification changes.
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title references three distinct changes: update to version 2.14.0, testframework changes, and singlelinesection updates. The pull request modifies files across all three areas, making the title accurate but overly broad and list-like.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch update-to-2.14.0&&testframework-fixes&&multiline-in-section-expression

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (2)
tests/functionsv2.sk (2)

104-106: Import statement placed mid-file.

The import for java.net.URL is added after test definitions. While this may work in Skript, consolidating imports at the top of the file (with the existing java.util.HashMap import on line 2) improves readability and follows conventional import organization.

Suggested refactor

Move the import to the top of the file:

 import:
     java.util.HashMap
+    java.net.URL

Then remove lines 104-106.


107-110: Assertion is missing an error message.

The assertion on line 110 lacks a with "<message>" clause, which is inconsistent with other assertions in this file. If the assertion fails, the error output will be less informative.

Suggested fix
-    assert {_val}.getClass().getSimpleName().equalsIgnoreCase("URL") is true 
+    assert {_val}.getClass().getSimpleName().equalsIgnoreCase("URL") is true with "java call type check failed"
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2c736ba and 1ed3cf8.

📒 Files selected for processing (6)
  • docs/testframework.md
  • scripts/libs/functionsv2.sk
  • scripts/libs/singlelinesection.sk
  • scripts/utils/testframework.sk
  • tests/functionsv2.sk
  • tests/singlinesection.sk
🔇 Additional comments (11)
tests/singlinesection.sk (2)

33-35: Variable {_b} is used but not set in this section.

The section sets {_a} to 2 and {_c} to 3, then returns {_a} + {_b} + {_c}. However, {_b} is not assigned within this section. The expected result of 5 implies {_b} carries over a value from the previous test (line 30 sets {_b} to 2).

If this is intentional to test variable context inheritance between section runs, consider adding a comment to clarify. If not, this test may produce unexpected results if run in isolation.


28-31: New multi-statement section test looks correct.

The test validates that statements separated by %nl% are executed sequentially, with {_a}=1, {_b}=2, and the return value being their sum (3). This aligns with the implementation changes in scripts/libs/singlelinesection.sk.

scripts/libs/functionsv2.sk (2)

73-74: Argument passing changed to string interpolation format.

The change from runtime arg references to string-quoted arguments ("%{proxy}%", "%{arg}%") alters how arguments are resolved. This appears intentional for the noreturn consumer/biconsumer paths.


106-108: Inner lambda script context fallback.

The fallback to current script when the script reference cannot be resolved from the origin node handles nested lambda scenarios where the config filename is unavailable.

scripts/utils/testframework.sk (2)

14-20: Test event pattern and registration updated for test-name-only semantics.

The event pattern now accepts an optional [devdinc] prefix, and test registration stores identifiers using only the test name. This aligns with the documented shift away from script-qualified identifiers.


242-326: Internal framework tests prefixed with devdinc.

All internal test declarations consistently use the new devdinc prefix. The test logic and assertions remain functionally equivalent.

docs/testframework.md (1)

77-78: Documentation updated to reflect test-name-only identifier semantics.

The documentation correctly reflects the implementation changes: test identifiers are now based solely on test names rather than script-qualified paths. Storage format, discovery expressions, and isolation guarantees are all updated consistently.

Also applies to: 92-93, 125-126, 135-136, 355-356

scripts/libs/singlelinesection.sk (4)

46-58: Parser lifecycle properly preserves and restores state.

The new implementation correctly captures the original isActive state, performs the parse operations, then restores both the backup and original active state. This is cleaner than the previous approach and ensures proper state management for nested parsing scenarios.


62-62: Expression signature updated for multi-statement support.

The pattern change from compile line %string% to [new] [single] line section %strings% supports the new multi-statement capability where expr-1 can be multiple strings joined with newlines.


72-83: String-based argument extraction path.

When expr-2 is a String instance (lines 72-79), the code extracts variable names by stripping curly braces and creating Variable instances. This handles the case where arguments are passed as string literals like "{_x}" rather than raw variable references.

The logic correctly differentiates between string-based and raw expression-based argument passing, routing to the appropriate extraction function.


84-86: Multi-statement code generation via newline join.

The change to join expr-1 with nl enables multi-statement sections where each string in expr-1 becomes a separate line. This aligns with the new test cases in tests/singlinesection.sk.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

@devdinc devdinc merged commit f753876 into dev Jan 17, 2026
1 of 2 checks passed
@devdinc devdinc deleted the update-to-2.14.0&&testframework-fixes&&multiline-in-section-expression branch January 17, 2026 11:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants