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.

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

Conversation

F1r3w477
Copy link

Problem

The Skript project currently has very low test coverage for its existing syntax, making it difficult to ensure changes don't introduce regressions. Specifically, the core length of expression was untested.

Solution

This PR adds a new syntax test file, ExprLength.sk, to provide unit test coverage for the length of expressions.

The tests cover behavior for:

String literals and variables.

Edge cases like empty strings, whitespace, and Unicode characters.

Automatic type conversion from numbers.

The list-looping behavior when applied to a list of strings.

String interpolation.

Testing Completed

Added the ExprLength.sk test. The full test suite was run via .\gradlew skriptTest and passed successfully.

Supporting Information

None


Completes: none
Related: #6158

@F1r3w477 F1r3w477 requested a review from a team as a code owner July 18, 2025 03:09
@F1r3w477 F1r3w477 requested review from cheeezburga and Burbulinis and removed request for a team July 18, 2025 03:09
@skriptlang-automation skriptlang-automation bot added the needs reviews A PR that needs additional reviews label Jul 18, 2025
Copy link
Contributor

@Absolutionism Absolutionism left a comment

Choose a reason for hiding this comment

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

Indentation needs to be in tabs, not spaces.
Maybe some tests using substring and joining.

Copy link
Member

@sovdeeth sovdeeth left a comment

Choose a reason for hiding this comment

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

Always nice to have more tests!

@skriptlang-automation skriptlang-automation bot removed the needs reviews A PR that needs additional reviews label Jul 18, 2025
@F1r3w477
Copy link
Author

F1r3w477 commented Jul 18, 2025

[...] Maybe some tests using substring and joining.

I think substring and joining should be tested elsewhere. ie: ExprSubstring.sk should get created to test the substring. This file tests the length, and at the end of the day, we are checking the final length of a var/str/etc which is already covered in this test.

In ExprSubstring.sk we'd want to check that, when using substring, the length is as expected, but an issue here would probably be with ExprSubstring not ExprLength

@F1r3w477 F1r3w477 requested a review from Absolutionism July 18, 2025 03:49
@Efnilite Efnilite added the enhancement Feature request, an issue about something that could be improved, or a PR improving something. label Jul 18, 2025
@skriptlang-automation skriptlang-automation bot added the feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version. label Jul 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature request, an issue about something that could be improved, or a PR improving something. feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants