Skip to content

add expr length tests #8052

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.

Sign up for GitHub

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

Open
wants to merge 4 commits into
base: dev/feature
Choose a base branch
from
Open

add expr length tests #8052

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
21b04b3
add expr length tests
F1r3w477 Jul 18, 2025
27dbdfd
tab spacing, remove unneeded clear
F1r3w477 Jul 18, 2025
921e9d7
add new line
F1r3w477 Jul 20, 2025
c7b97b8
with "..." additions, formatting
F1r3w477 Jul 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions src/test/skript/tests/syntaxes/expressions/ExprLength.sk
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
test "basic string length":
assert length of "hello" is 5

set {_string} to "Skript"
assert length of {_string} is 6

test "edge case string lengths":
set {_empty_string} to ""
assert length of {_empty_string} is 0

assert length of " " is 3
assert length of "😂" is 2

test "type conversion for length":
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this test is necessary - it's not handled by ExprLength but rather internally (and should be tested elsewhere)

Copy link
Author

Choose a reason for hiding this comment

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

Good point about the test for the undefined variable. I'll remove that assertion.

Regarding the length of "12345" test, my intention was to test the expression with a string literal that contains numeric characters. Since ExprLength is registered for %strings%, this directly tests its intended functionality.

To avoid future confusion, I'll renamed the test block from "type conversion for length" to "numeric string length"

assert length of "12345" is 5

clear {_undefined}
assert length of {_undefined} is not set

test "list looping behavior of length":
set {_list::*} to "a", "bb", and "cherry"
set {_lengths::*} to length of {_list::*}

assert {_lengths::1} is 1
assert {_lengths::2} is 2
assert {_lengths::3} is 6

test "string interpolation":
Copy link
Member

Choose a reason for hiding this comment

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

I think this is also not really necessary and should be handled in a different test focused on VariableString behavior

Copy link
Author

Choose a reason for hiding this comment

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

I agree that this test would ideally live in a dedicated VariableString.sk file.

Given that overall test coverage is a known issue Skript is working to improve, would it be acceptable to keep this integration test here for now? This ensures this case is covered now, rather than waiting until a VariableString.sk file is created.

It can always be moved to a more appropriate file later on.

set {_string} to "Skript"
set {_empty_string} to ""

assert "%length of "hello"%" is "5"
assert "%length of {_string}%" is "6"
assert "%length of {_empty_string}%" is "0"