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

Address sanitizers in integration tests #109

Merged
merged 3 commits into from
Feb 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 17 additions & 4 deletions .github/workflows/full-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ on:
- trying

env:
GDEXT_FEATURES: 'godot-core/convenience'
GDEXT_FEATURES: '--features godot-core/convenience'
# GDEXT_CRATE_ARGS: '-p godot-codegen -p godot-ffi -p godot-core -p godot-macros -p godot'

defaults:
Expand Down Expand Up @@ -59,7 +59,9 @@ jobs:
binary-filename: godot.linuxbsd.editor.dev.x86_64

- name: "Check clippy"
run: cargo clippy -- -D clippy::style -D clippy::complexity -D clippy::perf -D clippy::dbg_macro -D clippy::todo -D clippy::unimplemented
run: |
cargo clippy --all-targets $GDEXT_FEATURES -- -D clippy::suspicious -D clippy::style -D clippy::complexity -D clippy::perf \
-D clippy::dbg_macro -D clippy::todo -D clippy::unimplemented -D warnings


unit-test:
Expand Down Expand Up @@ -126,7 +128,7 @@ jobs:
binary-filename: ${{ matrix.godot-binary }}

- name: "Compile tests"
run: cargo test --no-run
run: cargo test $GDEXT_FEATURES --no-run

- name: "Test"
run: cargo test $GDEXT_FEATURES
Expand All @@ -135,7 +137,8 @@ jobs:
godot-itest:
name: godot-itest (${{ matrix.name }})
runs-on: ${{ matrix.os }}
continue-on-error: false
# TODO: continue-on-error: false, as soon as memory errors are fixed
continue-on-error: ${{ contains(matrix.name, 'memcheck') }}
timeout-minutes: 24
strategy:
fail-fast: false # cancel all jobs as soon as one fails?
Expand All @@ -161,6 +164,16 @@ jobs:
rust-toolchain: stable
godot-binary: godot.linuxbsd.editor.dev.x86_64

- name: linux-memcheck-gcc
os: ubuntu-20.04
rust-toolchain: stable
godot-binary: godot.linuxbsd.editor.dev.x86_64.san

- name: linux-memcheck-clang
os: ubuntu-20.04
rust-toolchain: stable
godot-binary: godot.linuxbsd.editor.dev.x86_64.llvm.san

steps:
- uses: actions/checkout@v3

Expand Down
6 changes: 4 additions & 2 deletions .github/workflows/minimal-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ on:
- master

env:
GDEXT_FEATURES: 'godot-core/convenience'
GDEXT_FEATURES: '--features godot-core/convenience'
# GDEXT_CRATE_ARGS: '-p godot-codegen -p godot-ffi -p godot-core -p godot-macros -p godot'

defaults:
Expand Down Expand Up @@ -59,7 +59,9 @@ jobs:
binary-filename: godot.linuxbsd.editor.dev.x86_64

- name: "Check clippy"
run: cargo clippy -- -D clippy::style -D clippy::complexity -D clippy::perf -D clippy::dbg_macro -D clippy::todo -D clippy::unimplemented
run: |
cargo clippy --all-targets $GDEXT_FEATURES -- -D clippy::suspicious -D clippy::style -D clippy::complexity -D clippy::perf \
-D clippy::dbg_macro -D clippy::todo -D clippy::unimplemented -D warnings
unit-test:
Expand Down
2 changes: 1 addition & 1 deletion check.sh
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ for arg in "${args[@]}"; do
cmds+=("cargo fmt --all -- --check")
;;
clippy)
cmds+=("cargo clippy $features -- -D clippy::style -D clippy::complexity -D clippy::perf -D clippy::dbg_macro -D clippy::todo -D clippy::unimplemented -D warnings")
cmds+=("cargo clippy $features -- -D clippy::suspicious -D clippy::style -D clippy::complexity -D clippy::perf -D clippy::dbg_macro -D clippy::todo -D clippy::unimplemented -D warnings")
;;
test)
cmds+=("cargo test $features")
Expand Down
4 changes: 2 additions & 2 deletions godot-core/src/builtin/dictionary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -328,15 +328,15 @@ impl Share for Dictionary {
///
/// Example
/// ```no_run
/// use godot::builtin::dict;
/// use godot::builtin::{dict, Variant};
///
/// let key = "my_key";
/// let d = dict! {
/// "key1": 10,
/// "another": Variant::nil(),
/// key: true,
/// (1 + 2): "final",
/// }
/// };
/// ```
#[macro_export]
macro_rules! dict {
Expand Down
18 changes: 16 additions & 2 deletions godot-ffi/src/godot_ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,19 @@ use crate as sys;
use std::fmt::Debug;

/// Adds methods to convert from and to Godot FFI pointers.
#[doc(hidden)]
/// See [crate::ffi_methods] for ergonomic implementation.
pub trait GodotFfi {
/// Construct from Godot opaque pointer.
///
/// # Safety
/// `ptr` must be a valid _type ptr_: it must follow Godot's convention to encode `Self`,
/// which is different depending on the type.
unsafe fn from_sys(ptr: sys::GDExtensionTypePtr) -> Self;

/// Construct uninitialized opaque data, then initialize it with `init` function.
/// Construct uninitialized opaque data, then initialize it with `init_fn` function.
///
/// # Safety
/// `init_fn` must be a function that correctly handles a (possibly-uninitialized) _type ptr_.
unsafe fn from_sys_init(init_fn: impl FnOnce(sys::GDExtensionTypePtr)) -> Self;

/// Return Godot opaque pointer, for an immutable operation.
Expand All @@ -38,6 +45,11 @@ pub trait GodotFfi {
self.sys()
}

/// Write the contents of `self` into the pointer `dst`.
///
/// # Safety
/// `dst` must be a valid _type ptr_: it must follow Godot's convention to encode `Self`,
/// which is different depending on the type.
unsafe fn write_sys(&self, dst: sys::GDExtensionTypePtr);
}

Expand Down Expand Up @@ -66,6 +78,7 @@ pub trait GodotFuncMarshal: Sized {
// See doc comment of `ffi_methods!` for information

#[macro_export]
#[doc(hidden)]
macro_rules! ffi_methods_one {
// type $Ptr = *mut Opaque
(OpaquePtr $Ptr:ty; $( #[$attr:meta] )? $vis:vis $from_sys:ident = from_sys) => {
Expand Down Expand Up @@ -159,6 +172,7 @@ macro_rules! ffi_methods_one {
}

#[macro_export]
#[doc(hidden)]
macro_rules! ffi_methods_rest {
( // impl T: each method has a custom name and is annotated with 'pub'
$Impl:ident $Ptr:ty; $( fn $user_fn:ident = $sys_fn:ident; )*
Expand Down
1 change: 1 addition & 0 deletions godot/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ pub use godot_core::private;
/// Often-imported symbols.
pub mod prelude {
pub use super::bind::{godot_api, GodotClass, GodotExt};
pub use super::builtin::dict; // Re-export macros.
pub use super::builtin::*;
pub use super::engine::{
load, try_load, utilities, AudioStreamPlayer, Camera2D, Camera3D, Input, Node, Node2D,
Expand Down