Skip to content
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

Rework integration tests: self-registering #[itest], Rust runner #142

Merged
merged 8 commits into from
Mar 5, 2023

Conversation

Bromeon
Copy link
Member

@Bromeon Bromeon commented Mar 5, 2023

Main changes:

  • #[itest] is now self-registering, just like Rust's #[test].
    • No more ok &= test_xy(); shenanigans. When declared, it runs.
  • A large part of the runner logic is moved from GDScript to Rust.
    • Unifies printing, statistics and time measurement.
    • Significantly speeds up test execution, from around 0.28s to 0.13s on my machine. Not that this matters much at the moment, but it may once we have more tests, or tests are generated at larger scale.
  • No longer prints the panic and stack trace during expect_panic, reducing the stdout spam during integration tests.

And unrelated to testing, but needed for this PR:

  • New Variant::call() method
  • .gitignore fixes

@Bromeon Bromeon added quality-of-life No new functionality, but improves ergonomics/internals c: tooling CI, automation, tools labels Mar 5, 2023
bors bot added a commit that referenced this pull request Mar 5, 2023
@bors
Copy link
Contributor

bors bot commented Mar 5, 2023

try

Build succeeded:

# Conflicts:
#	itest/rust/src/lib.rs
#	itest/rust/src/variant_test.rs
@Bromeon Bromeon force-pushed the feature/test-framework branch from 1c6a830 to 5d6218f Compare March 5, 2023 16:22
@Bromeon
Copy link
Member Author

Bromeon commented Mar 5, 2023

bors r+

@bors
Copy link
Contributor

bors bot commented Mar 5, 2023

Build succeeded:

@bors bors bot merged commit 55c4d3a into master Mar 5, 2023
@bors bors bot deleted the feature/test-framework branch March 5, 2023 16:45
@@ -148,6 +149,32 @@ pub fn default_call_error() -> GDExtensionCallError {
}
}

#[doc(hidden)]
#[inline]
pub fn panic_on_call_error(err: &GDExtensionCallError) {
Copy link
Contributor

@hydrolarus hydrolarus Mar 5, 2023

Choose a reason for hiding this comment

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

a little late, but this seems like a great candidate for using #[track_caller], doesn't it?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is great advice, thanks! I was actually considering to make it a macro for this very reason, but totally forgot about this attribute 👍

No worries about "late", 2nd round of test improvements already on the way 😎

Copy link
Member Author

Choose a reason for hiding this comment

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

I just tested it btw, indeed it works and shows the occurrence of the panic (with RUST_BACKTRACE=0) on the call site.

bors bot added a commit that referenced this pull request Mar 6, 2023
146: `#[itest(skip)]` + `#[itest(focus)]` r=Bromeon a=Bromeon

Follow-up to #142.

Two new test features:
* `#[itest(skip)]` ignores the current test, and shows the number of skipped tests in a statistic at the end of the run.
   * This is better than empty tests with `// TODO` or commented-out tests, as it reminds you of the technical debt and gives an impression of its extents.
* If at least one test is annotated with `#[itest(focus)]`, then _only_ "focused" tests are run. 
   * Extremely helpful during debugging sessions, or when one is working on a very particular feature.
   * If Godot is invoked with `-- --disallow-focus`, focus runs will always fail (for CI, to avoid accidental disabling of tests).

This is not yet implemented for GDScript. Might be worth it, or might not.

Apart from that, this PR massively simplifies the internal APIs to parse `#[attribute(key, key2="value")]` attributes.

Co-authored-by: Jan Haller <bromeon@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: tooling CI, automation, tools quality-of-life No new functionality, but improves ergonomics/internals
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants