Add a simple test runner in GDScript by ttencate · Pull Request #103 · godot-rust/gdext (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation33 Commits1 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
I made the output look like cargo test to reduce the cognitive burden while switching between the two languages:
Nothing has been done on the Rust side yet, so there is one big test IntegrationTests::test_all() (renamed from run()).
bors bot added a commit that referenced this pull request
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice to see some more automation and organization here!
I also like the rich printing with ok and colors, although I think it could be made slightly less verbose:
- the word
testat the beginning of each line is just repetitive, the--is less noisy - It might even be possible to print the class name once and then just list each method on a new line (like a tree).
100% same formatting as cargo test is not a requirement, it might even confuse some to think this is invoked by cargo test 😉
- It might even be possible to print the class name once and then just list each method on a new line (like a tree).
Repetition here facilitates test filtering, if/when we add that. Any substring of the full test name can then be copy/pasted as a filter.
CI is still failing. Apparently you need to open the project in the editor first, to cache class_name entries. Should I just add this file to Git?
$ cat itest/godot/.godot/global_script_class_cache.cfg
list=[{
"base": &"RefCounted",
"class": &"TestStats",
"icon": "",
"language": &"GDScript",
"path": "res://TestStats.gd"
}, {
"base": &"Node",
"class": &"TestSuite",
"icon": "",
"language": &"GDScript",
"path": "res://TestSuite.gd"
}]
Repetition here facilitates test filtering, if/when we add that. Any substring of the full test name can then be copy/pasted as a filter.
Ah, that's a nice consideration. I'm not sure if a potential future feature is worthy of this trade-off, as the test suite is mostly for CI; but I can see that filtering can be nice during development. I wonder if fast copy-pasting is a requirement though. With the -- at the front, things probably already look nicer, so we can leave it for now 🙂
Another question: should we use . instead of ::, as that's the GDScript separator?
Should I just add this file to Git?
Yes, that would be nice. You would also need to add an exclusion in .gitignore.
bors try
bors bot added a commit that referenced this pull request
Comment on lines 11 to 14
| .godot |
|---|
| godot.log |
| !/itest/.godot/extension_list.cfg |
| /itest/godot/.godot/** |
| !/itest/godot/.godot/extension_list.cfg |
| !/itest/godot/.godot/global_script_class_cache.cfg |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A general .godot should stay, we also have other project directories inside examples.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once a dir is ignored, you can't un-ignore specific files inside it. You'd want .godot/** but then it only matches relative to the current dir.
Solved it in a .gitignore per dir since dodge-the-creeps already had one, and it's more maintainable that way (see how the old ! line was actually wrong).
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once a dir is ignored, you can't un-ignore specific files inside it.
You're right, I just checked the documentation and indeed:
An optional prefix "!" which negates the pattern; any matching file excluded by a previous pattern will become included again. It is not possible to re-include a file if a parent directory of that file is excluded.
Solved it in a
.gitignoreper dir since dodge-the-creeps already had one, and it's more maintainable that way
I have mixed feelings about this:
- I have to keep N duplicated
.gitignorefiles instead of 1 in sync, where N is the number of Godot projects in the repo (itest + examples). This sounds less maintainable to me. - Example directories contain one more file which is not part of the example, just there for our own repo organization.
A not-so-nice option is to version files despite being ignored.
I guess **/.godot/** as a pattern does not work either?
Or examples/*/godot/.godot?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that works! 👍
Another question: should we use
.instead of::, as that's the GDScript separator?
There will be Rust tests soon as well. And making it language-dependent would be kinda weird I think, since it's not actually a fully qualified identifier in either language.
Since this works as-is, we might as well do the Rust side in a subsequent PR.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool to see we're also progressing on the tooling/testing side! 👍
I saw tiny nitpicks like leftover or, but I'll fix them in my follow-up #[itest] PR.
Thanks!
bors r+
2 participants
