Added some tests for existing syntax as well as an up to date test coverage list. #8264
Conversation
TFSMads
requested review from
TheMug06 and
cheeezburga
and removed request for
a team
skriptlang-automation
bot
added
the
needs reviews
src/test/java/org/skriptlang/skript/test/tests/syntaxes/effects/EffElytraBoostConsumeTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/skriptlang/skript/test/tests/syntaxes/effects/EffMakeEggHatchTest.java
Outdated
Show resolved
Hide resolved
| To run individual test files, use <code>/sk test \<file\></code>. To run last | ||
| used file again, just use <code>/sk test</code>. | ||
|
|
||
| ## Test Coverage |
There was a problem hiding this comment.
this is bound to become outdated, should we keep it?
There was a problem hiding this comment.
I agree that it could potentially become outdated, but I still think it’s valuable to keep. The test coverage list provides a clear overview of what’s currently tested and helps identify missing tests, especially for new contributors. Even if it requires occasional updates, it serves as useful documentation for anyone working on improving test coverage.
There was a problem hiding this comment.
It won't become outdated if PR's that add new tests are required to update the list.
There was a problem hiding this comment.
I'm ok with adding it if you, @TFSMads, or someone else is willing to take responsibility in occasionally checking to see if it's up to date (e.g. monthly, after every release). It's not something I have the time or will to ensure is up to date so I'd like someone who's able to keep track of it. If it's not updated, it won't be useful.
It's not currently up to date though so that needs fixing first, regardless.
There was a problem hiding this comment.
I'm willing to take responsibility in occasionally updating the list to ensure it stays up to date.
…s/EffElytraBoostConsumeTest.java Co-authored-by: Efnilite <[email protected]>
sovdeeth
left a comment
sovdeeth
left a comment
There was a problem hiding this comment.
sorry for such a long delay in reviewing, it slipped off my queue and only just now saw it again
src/test/java/org/skriptlang/skript/test/tests/syntaxes/effects/EffElytraBoostConsumeTest.java
Outdated
Show resolved
Hide resolved
| function returnTest() returns number: | ||
| return 42 | ||
|
|
||
| function returnTest2() returns numbers: | ||
| return 1,2,3 and 4 |
There was a problem hiding this comment.
it'd be nice to see some tests that require conversions
There was a problem hiding this comment.
What do you mean by tests that require conversion?
| To run individual test files, use <code>/sk test \<file\></code>. To run last | ||
| used file again, just use <code>/sk test</code>. | ||
|
|
||
| ## Test Coverage |
There was a problem hiding this comment.
I'm ok with adding it if you, @TFSMads, or someone else is willing to take responsibility in occasionally checking to see if it's up to date (e.g. monthly, after every release). It's not something I have the time or will to ensure is up to date so I'd like someone who's able to keep track of it. If it's not updated, it won't be useful.
It's not currently up to date though so that needs fixing first, regardless.
skriptlang-automation
bot
removed
the
needs reviews
skriptlang-automation
bot
added
the
needs reviews
4 participants
Problem
The Skript project currently has very low test coverage for its existing syntax and no recent list of test coverage.
Solution
This PR adds tests for the following syntax:
This PR also adds an up to date list of test coverage.
Testing Completed
Tests have been tested on versions 1.20.6, 1.21.3, 1.21.4, 1.21.5, 1.21.8 and 1.21.10.
Supporting Information
Completes: none
Related: #6158