-
-
Notifications
You must be signed in to change notification settings - Fork 370
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
⭐ Automated tests for all syntax and changes. #6158
Comments
Hi! I'd like to help add tests for existing functions. I'm new to opensource and I'd like to ask a few questions regarding how these tests should be implemented. Thanks! |
See https://github.com/SkriptLang/Skript/blob/master/src/test/skript/README.md |
Sure! It's fairly simple but if you'd like somebody to walk you through the process please contact one of us in our Discord server :) |
@Moderocky I think it would be a good idea to establish stronger naming guidelines for tests. What do you think about requiring tests to start with (or just be) the name of the expression. For example, the name of the test for ExprBlockData would just be ExprBlockData. Some files have multiple tests, so in those cases we could use "ExprBlockData - " to differentiate the test names. We may also want to consider whether having multiple tests in one file is practical compared to just "sectioning" tests using comments. |
The base test is currently named after the syntax class (like
These are usually named I think this is fine since when the test fails that makes it fairly clear what's not working. What I'm more concerned about is the assertion messages - most testers have taken care to make all of the messages unique (so you can work out which one failed from the message) but ideally I'd like the testing system to be able to determine the line that was the cause of the failure so we don't have to rely on that. |
In the future, we want to set a standard of every contribution providing its own tests.
Why?
These automated tests can be run before a build, meaning we have a guarantee that anything in the test files will perform as expected.
On top of that, the tests are run for each pull request before being merged, so if a contribution accidentally breaks or changes the behaviour of other syntax in an unexpected way we will know about this before release.
For example, if I open a pull request that changes parsing behaviour and it accidentally breaks the parsing of an obscure expression, the tests will fail and catch my mistake.
Where do we start?
To begin with, I would like all existing syntax to have tests (where applicable), even if they are just very basic.
Test coverage is incredibly poor currently.
The existing test coverage is displayed below, for anybody who would like to help out by filling in some of these missing gaps.
Expressions
Conditions
Effects
Sections
What can we test?
We can test almost anything that can be done on a server without players. This means anything involving items, maths, blocks, most entities, world operations and various and sundry bits and pieces.
We can't (comprehensively) test things involving players, things that have to be tested over a server restart or require something external to be changed. We also can't test things like performance or capacity very easily. These tests need to run automatically on any computer building Skript.
How can I do this?
In practice, this is fairly simple. If my pull request adds or changes the functionality of syntax, I just need to create (or update) the corresponding Skript test file.
A lot of pull requests won't need any extra tests. You'll only need to add/change a test file if you're adding/changing new functionality.
That said, if you think you can improve the existing test for a syntax element you are welcome to.
For example, if I change how
ExprDurability
works, I need to make sure that theExprDurability.sk
file is updated to test my changes.(If you add a new syntax you will need to create a new test file for it.)
The text was updated successfully, but these errors were encountered: