diff --git a/.github/composite/godot-install/action.yml b/.github/composite/godot-install/action.yml index 68998d4bd..c30a63de8 100644 --- a/.github/composite/godot-install/action.yml +++ b/.github/composite/godot-install/action.yml @@ -10,7 +10,7 @@ inputs: required: true description: "Name of the compiled Godot artifact to download" - binary-filename: + godot-binary: required: true description: "Filename of the Godot executable" @@ -25,7 +25,7 @@ runs: run: | runnerDir=$(echo "${{ runner.temp }}" | sed "s!\\\\!/!") echo "RUNNER_DIR=$runnerDir" >> $GITHUB_ENV - echo "GODOT4_BIN=$runnerDir/godot_bin/${{ inputs.binary-filename }}" >> $GITHUB_ENV + echo "GODOT4_BIN=$runnerDir/godot_bin/${{ inputs.godot-binary }}" >> $GITHUB_ENV shell: bash # - name: "Check cache for installed Godot version" diff --git a/.github/composite/godot-itest/action.yml b/.github/composite/godot-itest/action.yml index 1b1466353..b900176a4 100644 --- a/.github/composite/godot-itest/action.yml +++ b/.github/composite/godot-itest/action.yml @@ -10,10 +10,15 @@ inputs: required: true description: "Name of the compiled Godot artifact to download" - binary-filename: + godot-binary: required: true description: "Filename of the Godot executable" + godot-args: + required: false + default: '' + description: "Command-line arguments passed to Godot" + rust-toolchain: required: false default: 'stable' @@ -26,8 +31,8 @@ inputs: with-llvm: required: false - description: "Set to 'true' if LLVM should be installed" default: '' + description: "Set to 'true' if LLVM should be installed" runs: @@ -39,7 +44,7 @@ runs: uses: ./.github/composite/godot-install with: artifact-name: ${{ inputs.artifact-name }} - binary-filename: ${{ inputs.binary-filename }} + godot-binary: ${{ inputs.godot-binary }} # The chmod seems still necessary, although applied before uploading artifact. Possibly modes are not preserved. # The `| xargs` pattern trims the output, since printed version may contain extra newline, which causes problems in env vars. @@ -94,7 +99,7 @@ runs: run: | cd itest/godot echo "OUTCOME=itest" >> $GITHUB_ENV - $GODOT4_BIN --headless 2>&1 | tee "${{ runner.temp }}/log.txt" | tee >(grep "SCRIPT ERROR:" -q && { + $GODOT4_BIN --headless ${{ inputs.godot-args }} 2>&1 | tee "${{ runner.temp }}/log.txt" | tee >(grep "SCRIPT ERROR:" -q && { printf "\n -- Godot engine encountered error, abort...\n"; pkill godot echo "OUTCOME=godot-runtime" >> $GITHUB_ENV diff --git a/.github/workflows/full-ci.yml b/.github/workflows/full-ci.yml index 04285b415..82a383b11 100644 --- a/.github/workflows/full-ci.yml +++ b/.github/workflows/full-ci.yml @@ -57,7 +57,7 @@ jobs: uses: ./.github/composite/godot-install with: artifact-name: godot-linux - binary-filename: godot.linuxbsd.editor.dev.x86_64 + godot-binary: godot.linuxbsd.editor.dev.x86_64 - name: "Check clippy" run: | @@ -126,7 +126,7 @@ jobs: uses: ./.github/composite/godot-install with: artifact-name: godot-${{ matrix.name }} - binary-filename: ${{ matrix.godot-binary }} + godot-binary: ${{ matrix.godot-binary }} - name: "Compile tests" run: cargo test $GDEXT_FEATURES --no-run @@ -168,15 +168,19 @@ jobs: # Additionally, the Godot source is patched to make dlclose() a no-op, as unloading dynamic libraries loses stacktrace and # cause false positives like println!. See https://github.com/google/sanitizers/issues/89. # The gcc version can possibly be removed later, as it is slower and needs a larger artifact than the clang one. + + # --disallow-focus: fail if #[itest(focus)] is encountered, to prevent running only a few tests for full CI - name: linux-memcheck-gcc os: ubuntu-20.04 rust-toolchain: stable godot-binary: godot.linuxbsd.editor.dev.x86_64.san + godot-args: -- --disallow-focus - name: linux-memcheck-clang os: ubuntu-20.04 rust-toolchain: stable godot-binary: godot.linuxbsd.editor.dev.x86_64.llvm.san + godot-args: -- --disallow-focus steps: - uses: actions/checkout@v3 @@ -185,7 +189,8 @@ jobs: uses: ./.github/composite/godot-itest with: artifact-name: godot-${{ matrix.name }} - binary-filename: ${{ matrix.godot-binary }} + godot-binary: ${{ matrix.godot-binary }} + godot-args: ${{ matrix.godot-args }} with-llvm: ${{ matrix.name == 'macos' }} diff --git a/.github/workflows/minimal-ci.yml b/.github/workflows/minimal-ci.yml index 4668382c7..662e044ff 100644 --- a/.github/workflows/minimal-ci.yml +++ b/.github/workflows/minimal-ci.yml @@ -57,7 +57,7 @@ jobs: uses: ./.github/composite/godot-install with: artifact-name: godot-linux - binary-filename: godot.linuxbsd.editor.dev.x86_64 + godot-binary: godot.linuxbsd.editor.dev.x86_64 - name: "Check clippy" run: | @@ -78,7 +78,7 @@ jobs: uses: ./.github/composite/godot-install with: artifact-name: godot-linux - binary-filename: godot.linuxbsd.editor.dev.x86_64 + godot-binary: godot.linuxbsd.editor.dev.x86_64 - name: "Compile tests" run: cargo test $GDEXT_FEATURES --no-run @@ -98,7 +98,7 @@ jobs: uses: ./.github/composite/godot-itest with: artifact-name: godot-linux - binary-filename: godot.linuxbsd.editor.dev.x86_64 + godot-binary: godot.linuxbsd.editor.dev.x86_64 #godot_ver: ${{ matrix.godot }} diff --git a/godot-ffi/src/lib.rs b/godot-ffi/src/lib.rs index 596158b95..e71ed2dd5 100644 --- a/godot-ffi/src/lib.rs +++ b/godot-ffi/src/lib.rs @@ -151,6 +151,7 @@ pub fn default_call_error() -> GDExtensionCallError { #[doc(hidden)] #[inline] +#[track_caller] // panic message points to call site pub fn panic_on_call_error(err: &GDExtensionCallError) { let actual = err.error; diff --git a/godot-macros/src/derive_godot_class.rs b/godot-macros/src/derive_godot_class.rs index f98c9be0f..6b29f3333 100644 --- a/godot-macros/src/derive_godot_class.rs +++ b/godot-macros/src/derive_godot_class.rs @@ -4,16 +4,13 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ -use crate::util::{bail, ensure_kv_empty, ident, parse_kv_group, path_is_single, KvMap, KvValue}; -use crate::{util, ParseResult}; -use proc_macro2::{Ident, Punct, Span, TokenStream}; -use quote::spanned::Spanned; +use crate::util::{bail, ident, KvParser}; +use crate::ParseResult; +use proc_macro2::{Ident, Punct, TokenStream}; use quote::{format_ident, quote}; -use venial::{Attribute, NamedField, Struct, StructFields, TyExpr}; - -pub fn transform(input: TokenStream) -> ParseResult { - let decl = venial::parse_declaration(input)?; +use venial::{Declaration, NamedField, Struct, StructFields, TyExpr}; +pub fn transform(decl: Declaration) -> ParseResult { let class = decl .as_struct() .ok_or_else(|| venial::Error::new("Not a valid struct"))?; @@ -68,43 +65,24 @@ pub fn transform(input: TokenStream) -> ParseResult { /// Returns the name of the base and the default mode fn parse_struct_attributes(class: &Struct) -> ParseResult { - let mut base = ident("RefCounted"); - //let mut new_mode = GodotConstructMode::AutoGenerated; + let mut base_ty = ident("RefCounted"); let mut has_generated_init = false; // #[func] attribute on struct - if let Some((span, mut map)) = parse_class_attr(&class.attributes)? { - //println!(">>> CLASS {class}, MAP: {map:?}", class = class.name); - - if let Some(kv_value) = map.remove("base") { - if let KvValue::Ident(override_base) = kv_value { - base = override_base; - } else { - bail("Invalid value for 'base' argument", span)?; - } + if let Some(mut parser) = KvParser::parse(&class.attributes, "class")? { + if let Some(base) = parser.handle_ident("base")? { + base_ty = base; } - /*if let Some(kv_value) = map.remove("new") { - match kv_value { - KvValue::Ident(ident) if ident == "fn" => new_mode = GodotConstructMode::FnNew, - KvValue::Ident(ident) if ident == "none" => new_mode = GodotConstructMode::None, - _ => bail( - "Invalid value for 'new' argument; must be 'fn' or 'none'", - span, - )?, - } - }*/ - if let Some(kv_value) = map.remove("init") { - match kv_value { - KvValue::None => has_generated_init = true, - _ => bail("Argument 'init' must not have a value", span)?, - } + if parser.handle_alone("init")? { + has_generated_init = true; } - ensure_kv_empty(map, span)?; + + parser.finish()?; } Ok(ClassAttributes { - base_ty: base, + base_ty, has_generated_init, }) } @@ -130,34 +108,27 @@ fn parse_fields(class: &Struct) -> ParseResult { for (field, _punct) in fields { let mut is_base = false; - // #[base] or #[export] - for attr in field.attributes.iter() { - if let Some(path) = attr.get_single_path_segment() { - if path == "base" { - is_base = true; - if let Some(prev_base) = base_field { - bail( - format!( - "#[base] allowed for at most 1 field, already applied to '{}'", - prev_base.name - ), - attr, - )?; - } - base_field = Some(Field::new(&field)) - } else if path == "export" { - match parse_kv_group(&attr.value) { - Ok(export_kv) => { - let exported_field = - ExportedField::new_from_kv(Field::new(&field), attr, export_kv)?; - exported_fields.push(exported_field); - } - Err(error) => { - return Err(error); - } - } - } + // #[base] + if let Some(parser) = KvParser::parse(&field.attributes, "base")? { + if let Some(prev_base) = base_field { + bail( + format!( + "#[base] allowed for at most 1 field, already applied to '{}'", + prev_base.name + ), + parser.span(), + )?; } + is_base = true; + base_field = Some(Field::new(&field)); + parser.finish()?; + } + + // #[export] + if let Some(mut parser) = KvParser::parse(&field.attributes, "export")? { + let exported_field = ExportedField::new_from_kv(Field::new(&field), &mut parser)?; + exported_fields.push(exported_field); + parser.finish()?; } // Exported or Rust-only fields @@ -173,26 +144,6 @@ fn parse_fields(class: &Struct) -> ParseResult { }) } -/// Parses a `#[class(...)]` attribute -fn parse_class_attr(attributes: &[Attribute]) -> ParseResult> { - let mut godot_attr = None; - for attr in attributes.iter() { - let path = &attr.path; - if path_is_single(path, "class") { - if godot_attr.is_some() { - bail( - "Only one #[class] attribute per item (struct, fn, ...) allowed", - attr, - )?; - } - - let map = util::parse_kv_group(&attr.value)?; - godot_attr = Some((attr.__span(), map)); - } - } - Ok(godot_attr) -} - // ---------------------------------------------------------------------------------------------------------------------------------------------- // General helpers @@ -229,16 +180,10 @@ struct ExportedField { } impl ExportedField { - pub fn new_from_kv( - field: Field, - attr: &Attribute, - mut map: KvMap, - ) -> ParseResult { - let getter = Self::require_key_value(&mut map, "getter", attr)?; - let setter = Self::require_key_value(&mut map, "setter", attr)?; - let variant_type = Self::require_key_value(&mut map, "variant_type", attr)?; - - ensure_kv_empty(map, attr.__span())?; + pub fn new_from_kv(field: Field, parser: &mut KvParser) -> ParseResult { + let getter = parser.handle_lit_required("getter")?; + let setter = parser.handle_lit_required("setter")?; + let variant_type = parser.handle_lit_required("variant_type")?; Ok(ExportedField { field, @@ -247,21 +192,6 @@ impl ExportedField { variant_type, }) } - - fn require_key_value(map: &mut KvMap, key: &str, attr: &Attribute) -> ParseResult { - if let Some(value) = map.remove(key) { - if let KvValue::Lit(value) = value { - Ok(value) - } else { - bail( - format!("#[export] attribute {key} with a non-literal variant_type",), - attr, - )? - } - } else { - bail(format!("#[export] attribute without a {key}"), attr) - } - } } fn make_godot_init_impl(class_name: &Ident, fields: Fields) -> TokenStream { diff --git a/godot-macros/src/gdextension.rs b/godot-macros/src/gdextension.rs index aaf0e7e4e..a37dd0279 100644 --- a/godot-macros/src/gdextension.rs +++ b/godot-macros/src/gdextension.rs @@ -9,15 +9,7 @@ use proc_macro2::TokenStream; use quote::quote; use venial::Declaration; -pub fn transform(meta: TokenStream, input: TokenStream) -> Result { - // Hack because venial doesn't support direct meta parsing yet - let input = quote! { - #[gdextension(#meta)] - #input - }; - - let decl = venial::parse_declaration(input)?; - +pub fn transform(decl: Declaration) -> Result { let mut impl_decl = match decl { Declaration::Impl(item) => item, _ => return bail("#[gdextension] can only be applied to trait impls", &decl), diff --git a/godot-macros/src/godot_api.rs b/godot-macros/src/godot_api.rs index bff0acbf7..a9366479c 100644 --- a/godot-macros/src/godot_api.rs +++ b/godot-macros/src/godot_api.rs @@ -13,8 +13,7 @@ use venial::{AttributeValue, Declaration, Error, Function, Impl, ImplMember}; // Note: keep in sync with trait GodotExt const VIRTUAL_METHOD_NAMES: [&str; 3] = ["ready", "process", "physics_process"]; -pub fn transform(input: TokenStream) -> Result { - let input_decl = venial::parse_declaration(input)?; +pub fn transform(input_decl: Declaration) -> Result { let decl = match input_decl { Declaration::Impl(decl) => decl, _ => bail( diff --git a/godot-macros/src/itest.rs b/godot-macros/src/itest.rs index 0adfc7875..dc37c77b2 100644 --- a/godot-macros/src/itest.rs +++ b/godot-macros/src/itest.rs @@ -4,14 +4,13 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ -use crate::util::bail; +use crate::util::{bail, KvParser}; +use crate::ParseResult; use proc_macro2::TokenStream; use quote::quote; -use venial::{Declaration, Error}; - -pub fn transform(input: TokenStream) -> Result { - let input_decl = venial::parse_declaration(input)?; +use venial::Declaration; +pub fn transform(input_decl: Declaration) -> ParseResult { let func = match input_decl { Declaration::Function(f) => f, _ => return bail("#[itest] can only be applied to functions", &input_decl), @@ -29,32 +28,31 @@ pub fn transform(input: TokenStream) -> Result { ); } + let mut attr = KvParser::parse_required(&func.attributes, "itest", &func.name)?; + let skipped = attr.handle_alone("skip")?; + let focused = attr.handle_alone("focus")?; + attr.finish()?; + + if skipped && focused { + return bail( + "#[itest]: keys `skip` and `focus` are mutually exclusive", + func.name, + ); + } + let test_name = &func.name; let test_name_str = func.name.to_string(); let body = &func.body; Ok(quote! { - /*#[doc(hidden)] - #[must_use] - pub fn #test_name() -> bool { - println!(#init_msg); - - // Explicit type to prevent tests from returning a value - let success: Option<()> = godot::private::handle_panic( - || #error_msg, - || #body - ); - - success.is_some() - }*/ - pub fn #test_name() { #body } ::godot::sys::plugin_add!(__GODOT_ITEST in crate; crate::RustTestCase { name: #test_name_str, - skipped: false, + skipped: #skipped, + focused: #focused, file: std::file!(), line: std::line!(), function: #test_name, diff --git a/godot-macros/src/lib.rs b/godot-macros/src/lib.rs index 8dbe00d04..35eea339c 100644 --- a/godot-macros/src/lib.rs +++ b/godot-macros/src/lib.rs @@ -4,8 +4,11 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ +use crate::util::ident; use proc_macro::TokenStream; use proc_macro2::TokenStream as TokenStream2; +use quote::quote; +use venial::Declaration; mod derive_godot_class; mod gdextension; @@ -27,8 +30,8 @@ pub fn godot_api(_meta: TokenStream, input: TokenStream) -> TokenStream { /// /// Transforms the `fn` into one returning `bool` (success of the test), which must be called explicitly. #[proc_macro_attribute] -pub fn itest(_meta: TokenStream, input: TokenStream) -> TokenStream { - translate(input, itest::transform) +pub fn itest(meta: TokenStream, input: TokenStream) -> TokenStream { + translate_meta("itest", meta, input, itest::transform) } /// Proc-macro attribute to be used in combination with the [`ExtensionLibrary`] trait. @@ -37,7 +40,7 @@ pub fn itest(_meta: TokenStream, input: TokenStream) -> TokenStream { // FIXME intra-doc link #[proc_macro_attribute] pub fn gdextension(meta: TokenStream, input: TokenStream) -> TokenStream { - translate_meta(meta, input, gdextension::transform) + translate_meta("gdextension", meta, input, gdextension::transform) } // ---------------------------------------------------------------------------------------------------------------------------------------------- @@ -46,25 +49,39 @@ type ParseResult = Result; fn translate(input: TokenStream, transform: F) -> TokenStream where - F: FnOnce(TokenStream2) -> ParseResult, + F: FnOnce(Declaration) -> ParseResult, { let input2 = TokenStream2::from(input); - let result2: TokenStream2 = match transform(input2) { - Ok(output) => output, - Err(error) => error.to_compile_error(), - }; + + let result2 = venial::parse_declaration(input2) + .and_then(transform) + .unwrap_or_else(|e| e.to_compile_error()); + TokenStream::from(result2) } -fn translate_meta(meta: TokenStream, input: TokenStream, transform: F) -> TokenStream +fn translate_meta( + self_name: &str, + meta: TokenStream, + input: TokenStream, + transform: F, +) -> TokenStream where - F: FnOnce(TokenStream2, TokenStream2) -> ParseResult, + F: FnOnce(Declaration) -> ParseResult, { + let self_name = ident(self_name); let input2 = TokenStream2::from(input); let meta2 = TokenStream2::from(meta); - let result2: TokenStream2 = match transform(meta2, input2) { - Ok(output) => output, - Err(error) => error.to_compile_error(), + + // Hack because venial doesn't support direct meta parsing yet + let input = quote! { + #[#self_name(#meta2)] + #input2 }; + + let result2 = venial::parse_declaration(input) + .and_then(transform) + .unwrap_or_else(|e| e.to_compile_error()); + TokenStream::from(result2) } diff --git a/godot-macros/src/util.rs b/godot-macros/src/util.rs index 1f703d5fd..aabc0be54 100644 --- a/godot-macros/src/util.rs +++ b/godot-macros/src/util.rs @@ -8,10 +8,10 @@ use crate::ParseResult; use proc_macro2::{Ident, Literal, Span, TokenTree}; -use quote::format_ident; use quote::spanned::Spanned; +use quote::{format_ident, ToTokens}; use std::collections::HashMap; -use venial::{Error, Function, Impl}; +use venial::{Attribute, Error, Function, Impl}; pub fn ident(s: &str) -> Ident { format_ident!("{}", s) @@ -45,7 +45,7 @@ pub fn reduce_to_signature(function: &Function) -> Function { #[derive(Clone, Eq, PartialEq, Debug)] pub(crate) enum KvValue { /// Key only, no value. - None, + None, // TODO rename to `Alone`; pending merge conflicts /// Literal like `"hello"`, `20`, `3.4`. /// Unlike the proc macro type, this includes `true` and `false` as well as negative literals `-32`. @@ -58,6 +58,147 @@ pub(crate) enum KvValue { pub(crate) type KvMap = HashMap; +/// Struct to parse attributes like `#[attr(key, key2="value", key3=123)]` in a very user-friendly way. +pub(crate) struct KvParser { + attr_name: String, + map: KvMap, + span: Span, + #[cfg(debug_assertions)] + finished: bool, +} + +impl KvParser { + /// Create a new parser which requires a `#[expected]` attribute. + /// + /// `context` is used for the span in error messages. + #[allow(dead_code)] // will be used later + pub fn parse_required( + attributes: &[Attribute], + expected: &str, + context: impl ToTokens, + ) -> ParseResult { + match Self::parse(attributes, expected) { + Ok(Some(result)) => Ok(result), + Ok(None) => { + return bail( + format!("expected attribute #[{expected}], but not present"), + context, + ) + } + Err(e) => Err(e), + } + } + + /// Create a new parser which checks for presence of an `#[expected]` attribute. + pub fn parse(attributes: &[Attribute], expected: &str) -> ParseResult> { + let mut found_attr = None; + + for attr in attributes.iter() { + let path = &attr.path; + if path_is_single(path, expected) { + if found_attr.is_some() { + return bail( + format!("only a single #[{expected}] attribute allowed"), + attr, + ); + } + + found_attr = Some(Self { + attr_name: expected.to_string(), + span: attr.__span(), + map: parse_kv_group(&attr.value)?, + #[cfg(debug_assertions)] + finished: false, + }); + } + } + + Ok(found_attr) + } + + pub fn span(&self) -> Span { + self.span + } + + /// `#[attr(key)]` + pub fn handle_alone(&mut self, key: &str) -> ParseResult { + match self.map.remove(key) { + None => Ok(false), + Some(KvValue::None) => Ok(true), + Some(_) => self.bail_key(key, "must not have a value"), + } + } + + /// `#[attr(key=Ident)]` + pub fn handle_ident(&mut self, key: &str) -> ParseResult> { + match self.map.remove(key) { + None => Ok(None), + Some(KvValue::Ident(ident)) => Ok(Some(ident)), + Some(_) => self.bail_key(key, "must have an identifier value (no quotes)"), + } + } + + /// `#[attr(key="string", key2=123, key3=true)]` + pub fn handle_lit(&mut self, key: &str) -> ParseResult> { + match self.map.remove(key) { + None => Ok(None), + Some(KvValue::Lit(string)) => Ok(Some(string)), + Some(_) => self.bail_key(key, "must have a literal value (\"text\", 3.4, true, ...)"), + } + } + + /// `#[attr(key="string", key2=123, key3=true)]`, with a given key being required + pub fn handle_lit_required(&mut self, key: &str) -> ParseResult { + match self.handle_lit(key) { + Ok(Some(string)) => Ok(string), + Ok(None) => self.bail_key(key, "expected to have literal value, but is absent"), + Err(err) => Err(err), + } + } + + /// Explicit "pre-destructor" that must be called, because Drop cannot propagate errors. + // Note: this could possibly be modeled using a closure: KvParser::parse(...) .with(|parser| ...)? + pub fn finish(mut self) -> ParseResult<()> { + #[cfg(debug_assertions)] + { + self.finished = true; // disarm destructor + } + + if self.map.is_empty() { + Ok(()) + } else { + // Useless allocation, but there seems to be no join() on map iterators. Anyway, this is slow/error path. + let keys = self.map.keys().cloned().collect::>().join(", "); + + let s = if self.map.len() > 1 { "s" } else { "" }; // plural + return bail( + format!( + "#[{attr}]: unrecognized key{s}: {keys}", + attr = self.attr_name + ), + self.span, + ); + } + } + + fn bail_key(&self, key: &str, msg: &str) -> ParseResult { + return bail( + format!("#[{attr}]: key `{key}` {msg}", attr = self.attr_name), + self.span, + ); + } +} + +#[cfg(debug_assertions)] +impl Drop for KvParser { + fn drop(&mut self) { + assert!( + self.finished, + "proc-macro did not check for remaining elements; this is a bug in the library" + ); + } +} + // parses (a="hey", b=342) pub(crate) fn parse_kv_group(value: &venial::AttributeValue) -> ParseResult { // FSM with possible flows: @@ -189,16 +330,8 @@ pub(crate) fn parse_kv_group(value: &venial::AttributeValue) -> ParseResult ParseResult<()> { - if map.is_empty() { - Ok(()) - } else { - let msg = &format!("Attribute contains unknown keys: {:?}", map.keys()); - bail(msg, span) - } -} +// ---------------------------------------------------------------------------------------------------------------------------------------------- +// Validation for trait/impl /// Validates either: /// a) the declaration is `impl Trait for SomeType`, if `expected_trait` is `Some("Trait")` diff --git a/itest/godot/TestRunner.gd b/itest/godot/TestRunner.gd index 98376e561..89146ea47 100644 --- a/itest/godot/TestRunner.gd +++ b/itest/godot/TestRunner.gd @@ -5,6 +5,20 @@ extends Node func _ready(): + var allow_focus := true + var unrecognized_args: Array = [] + for arg in OS.get_cmdline_user_args(): + match arg: + "--disallow-focus": + allow_focus = false + _: + unrecognized_args.push_back(arg) + + if unrecognized_args: + push_error("Unrecognized arguments: ", unrecognized_args) + get_tree().quit(2) + return + var rust_runner = IntegrationTests.new() var gdscript_suites: Array = [ @@ -19,7 +33,7 @@ func _ready(): if method_name.begins_with("test_"): gdscript_tests.push_back(GDScriptTestCase.new(suite, method_name)) - var success: bool = rust_runner.run_all_tests(gdscript_tests, gdscript_suites.size()) + var success: bool = rust_runner.run_all_tests(gdscript_tests, gdscript_suites.size(), allow_focus) var exit_code: int = 0 if success else 1 get_tree().quit(exit_code) diff --git a/itest/rust/src/base_test.rs b/itest/rust/src/base_test.rs index c9fdf8f13..fce5f1b79 100644 --- a/itest/rust/src/base_test.rs +++ b/itest/rust/src/base_test.rs @@ -7,12 +7,11 @@ use crate::itest; use godot::prelude::*; -/* -#[itest] +#[itest(skip)] fn base_test_is_weak() { - let obj = RefCounted::new(); + // TODO check that Base is a weak pointer (doesn't keep the object alive) + // This might not be needed, as we have leak detection, but it could highlight regressions faster } -*/ #[itest] fn base_instance_id() { diff --git a/itest/rust/src/lib.rs b/itest/rust/src/lib.rs index 099d5e488..9b5d8074a 100644 --- a/itest/rust/src/lib.rs +++ b/itest/rust/src/lib.rs @@ -61,19 +61,30 @@ unsafe impl ExtensionLibrary for runner::IntegrationTests {} sys::plugin_registry!(__GODOT_ITEST: RustTestCase); /// Finds all `#[itest]` tests. -fn collect_rust_tests() -> (Vec, usize) { +fn collect_rust_tests() -> (Vec, usize, bool) { let mut all_files = std::collections::HashSet::new(); let mut tests: Vec = vec![]; + let mut is_focus_run = false; sys::plugin_foreach!(__GODOT_ITEST; |test: &RustTestCase| { - all_files.insert(test.file); - tests.push(*test); + // First time a focused test is encountered, switch to "focused" mode and throw everything away. + if !is_focus_run && test.focused { + tests.clear(); + all_files.clear(); + is_focus_run = true; + } + + // Only collect tests if normal mode, or focus mode and test is focused. + if !is_focus_run || test.focused { + all_files.insert(test.file); + tests.push(*test); + } }); // Sort alphabetically for deterministic run order tests.sort_by_key(|test| test.file); - (tests, all_files.len()) + (tests, all_files.len(), is_focus_run) } #[derive(Copy, Clone)] @@ -81,6 +92,8 @@ struct RustTestCase { name: &'static str, file: &'static str, skipped: bool, + /// If one or more tests are focused, only they will be executed. Helpful for debugging and working on specific features. + focused: bool, #[allow(dead_code)] line: u32, function: fn(), diff --git a/itest/rust/src/node_test.rs b/itest/rust/src/node_test.rs index a5aee952c..439cff36c 100644 --- a/itest/rust/src/node_test.rs +++ b/itest/rust/src/node_test.rs @@ -6,7 +6,7 @@ use crate::itest; use godot::builtin::NodePath; -use godot::engine::{node, Node, Node3D, NodeExt}; +use godot::engine::{global, node, Node, Node3D, NodeExt, PackedScene, SceneTree}; use godot::obj::Share; #[itest] @@ -54,8 +54,7 @@ fn node_get_node_fail() { child.free(); } -/* -#[itest] +#[itest(skip)] fn node_scene_tree() { let mut child = Node::new_alloc(); child.set_name("kid".into()); @@ -82,4 +81,3 @@ fn node_scene_tree() { parent.free(); child.free(); } -*/ diff --git a/itest/rust/src/packed_array_test.rs b/itest/rust/src/packed_array_test.rs index 4e8855928..08e2990b1 100644 --- a/itest/rust/src/packed_array_test.rs +++ b/itest/rust/src/packed_array_test.rs @@ -41,14 +41,16 @@ fn packed_array_to_vec() { assert_eq!(array.to_vec(), vec![1, 2]); } -// #[itest] -// fn packed_array_into_iterator() { -// let array = Array::from(&[1, 2]); -// let mut iter = array.into_iter(); -// assert_eq!(iter.next(), Some(1)); -// assert_eq!(iter.next(), Some(2)); -// assert_eq!(iter.next(), None); -// } +/* +#[itest(skip)] +fn packed_array_into_iterator() { + let array = PackedByteArray::from(&[1, 2]); + let mut iter = array.into_iter(); + assert_eq!(iter.next(), Some(1)); + assert_eq!(iter.next(), Some(2)); + assert_eq!(iter.next(), None); +} +*/ #[itest] fn packed_array_eq() { diff --git a/itest/rust/src/runner.rs b/itest/rust/src/runner.rs index e697eccb3..e617ecb05 100644 --- a/itest/rust/src/runner.rs +++ b/itest/rust/src/runner.rs @@ -16,37 +16,50 @@ pub(crate) struct IntegrationTests { total: i64, passed: i64, skipped: i64, + focus_run: bool, } #[godot_api] impl IntegrationTests { #[allow(clippy::uninlined_format_args)] #[func] - fn run_all_tests(&mut self, gdscript_tests: Array, gdscript_file_count: i64) -> bool { - println!( - "{}Run{} Godot integration tests...", - FMT_GREEN_BOLD, FMT_END - ); - - let (rust_tests, rust_file_count) = super::collect_rust_tests(); + fn run_all_tests( + &mut self, + gdscript_tests: Array, + gdscript_file_count: i64, + allow_focus: bool, + ) -> bool { + println!("{}Run{} Godot integration tests...", FMT_CYAN_BOLD, FMT_END); + + let (rust_tests, rust_file_count, focus_run) = super::collect_rust_tests(); + self.focus_run = focus_run; + if focus_run { + println!(" {FMT_CYAN}Focused run{FMT_END} -- execute only selected Rust tests.") + } println!( " Rust: found {} tests in {} files.", rust_tests.len(), rust_file_count ); - println!( - " GDScript: found {} tests in {} files.", - gdscript_tests.len(), - gdscript_file_count - ); + if !focus_run { + println!( + " GDScript: found {} tests in {} files.", + gdscript_tests.len(), + gdscript_file_count + ); + } let clock = Instant::now(); self.run_rust_tests(rust_tests); let rust_time = clock.elapsed(); - self.run_gdscript_tests(gdscript_tests); - let gdscript_time = clock.elapsed() - rust_time; + let gdscript_time = if !focus_run { + self.run_gdscript_tests(gdscript_tests); + Some(clock.elapsed() - rust_time) + } else { + None + }; - self.conclude(rust_time, gdscript_time) + self.conclude(rust_time, gdscript_time, allow_focus) } fn run_rust_tests(&mut self, tests: Vec) { @@ -76,7 +89,12 @@ impl IntegrationTests { } } - fn conclude(&self, rust_time: Duration, gdscript_time: Duration) -> bool { + fn conclude( + &self, + rust_time: Duration, + gdscript_time: Option, + allow_focus: bool, + ) -> bool { let Self { total, passed, @@ -91,12 +109,33 @@ impl IntegrationTests { let outcome = TestOutcome::from_bool(all_passed); let rust_time = rust_time.as_secs_f32(); - let gdscript_time = gdscript_time.as_secs_f32(); - let total_time = rust_time + gdscript_time; + let gdscript_time = gdscript_time.map(|t| t.as_secs_f32()); + let focused_run = gdscript_time.is_none(); - println!("\nTest result: {outcome}. {passed} passed; {failed} failed."); - println!(" Time: {total_time:.2}s. (Rust {rust_time:.2}s, GDScript {gdscript_time:.2}s)"); - all_passed + let extra = if skipped > 0 { + format!(", {skipped} skipped") + } else if focused_run { + " (focused run)".to_string() + } else { + "".to_string() + }; + + println!("\nTest result: {outcome}. {passed} passed; {failed} failed{extra}."); + if let Some(gdscript_time) = gdscript_time { + let total_time = rust_time + gdscript_time; + println!( + " Time: {total_time:.2}s. (Rust {rust_time:.2}s, GDScript {gdscript_time:.2}s)" + ); + } else { + println!(" Time: {rust_time:.2}s."); + } + + if focused_run && !allow_focus { + println!(" {FMT_YELLOW}Focus run disallowed; return failure.{FMT_END}"); + false + } else { + all_passed + } } fn update_stats(&mut self, outcome: &TestOutcome) { @@ -114,7 +153,8 @@ impl IntegrationTests { // use rand::seq::SliceRandom; // let outcome = [TestOutcome::Passed, TestOutcome::Failed, TestOutcome::Skipped]; // let outcome = outcome.choose(&mut rand::thread_rng()).unwrap(); -const FMT_GREEN_BOLD: &str = "\x1b[32;1;1m"; +const FMT_CYAN_BOLD: &str = "\x1b[36;1;1m"; +const FMT_CYAN: &str = "\x1b[36m"; const FMT_GREEN: &str = "\x1b[32m"; const FMT_YELLOW: &str = "\x1b[33m"; const FMT_RED: &str = "\x1b[31m"; @@ -191,7 +231,7 @@ impl std::fmt::Display for TestOutcome { let (col, outcome) = match self { TestOutcome::Passed => (FMT_GREEN, "ok"), TestOutcome::Failed => (FMT_RED, "FAILED"), - TestOutcome::Skipped => (FMT_YELLOW, "ignored"), + TestOutcome::Skipped => (FMT_YELLOW, "skipped"), }; write!(f, "{col}{outcome}{end}") diff --git a/itest/rust/src/string_test.rs b/itest/rust/src/string_test.rs index aa9aea2c1..028783460 100644 --- a/itest/rust/src/string_test.rs +++ b/itest/rust/src/string_test.rs @@ -74,17 +74,17 @@ fn string_name_default_construct() { assert_eq!(back, GodotString::new()); } -#[itest] +#[itest(skip)] fn string_name_eq_hash() { // TODO } -#[itest] +#[itest(skip)] fn string_name_ord() { // TODO } -#[itest] +#[itest(skip)] fn string_name_clone() { // TODO } diff --git a/itest/rust/src/variant_test.rs b/itest/rust/src/variant_test.rs index 2a92472c7..ae5c3e8db 100644 --- a/itest/rust/src/variant_test.rs +++ b/itest/rust/src/variant_test.rs @@ -245,6 +245,28 @@ fn variant_sys_conversion() { assert_eq!(v2, v); } +#[itest(skip)] +fn variant_sys_conversion2() { + use godot::sys; + + // FIXME alignment, maybe use alloc() + let mut buffer = [0u8; 50]; + + let v = Variant::from(7); + unsafe { v.write_sys(buffer.as_mut_ptr() as sys::GDExtensionTypePtr) }; + + let v2 = unsafe { + Variant::from_sys_init(|ptr| { + std::ptr::copy( + buffer.as_ptr(), + ptr as *mut u8, + std::mem::size_of_val(&*ptr), + ) + }) + }; + assert_eq!(v2, v); +} + #[itest] fn variant_null_object_is_nil() { use godot::sys; @@ -265,23 +287,6 @@ fn variant_null_object_is_nil() { node.free(); } -#[itest] -fn variant_sys_conversion2() { - /* - let buffer = [0u8; 50]; - - let v = Variant::from(7); - unsafe { v.write_sys(buffer.as_mut_ptr()) }; - - let v2 = unsafe { - Variant::from_sys_init(|ptr| { - std::ptr::copy(buffer.as_ptr(), ptr as *mut u8, std::mem::size_of_val(*ptr)) - }) - }; - assert_eq!(v2, v); - */ -} - #[itest] fn variant_conversion_fails() { assert_eq!(