Skip to content

Commit

Permalink
Make ignore lists run time and not compile time
Browse files Browse the repository at this point in the history
  • Loading branch information
aqrln committed Feb 7, 2025
1 parent f14aa0c commit 4f41af3
Show file tree
Hide file tree
Showing 25 changed files with 195 additions and 135 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/test-query-compiler-template.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ on:
ignored_tests_list:
type: string
required: true
should_panic_tests_list:
should_fail_tests_list:
type: string
required: true

Expand All @@ -34,7 +34,7 @@ jobs:
WASM_BUILD_PROFILE: "profiling" # Include debug info for proper backtraces
WORKSPACE_ROOT: ${{ github.workspace }}
IGNORED_TESTS: ${{ inputs.ignored_tests_list }}
SHOULD_PANIC_TESTS: ${{ inputs.should_panic_tests_list }}
SHOULD_FAIL_TESTS: ${{ inputs.should_fail_tests_list }}

runs-on: ubuntu-latest
steps:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/test-query-compiler.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,4 @@ jobs:
with:
setup_task: ${{ matrix.adapter.setup_task }}
ignored_tests_list: ${{ matrix.ignored_tests_list }}
should_panic_tests_list: ${{ matrix.should_panic_tests_list }}
should_fail_tests_list: ${{ matrix.should_fail_tests_list }}
11 changes: 11 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions libs/driver-adapters/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ workspace = true
async-trait.workspace = true
futures.workspace = true
once_cell = "1.15"
panic-utils.path = "../../libs/panic-utils"
prisma-metrics.path = "../../libs/metrics"
serde.workspace = true
serde_json.workspace = true
Expand Down
5 changes: 1 addition & 4 deletions libs/driver-adapters/src/napi/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,7 @@ where
}

fn panic_to_napi_err<R>(panic_payload: Box<dyn Any + Send>) -> napi::Result<R> {
panic_payload
.downcast_ref::<&str>()
.map(|s| -> String { (*s).to_owned() })
.or_else(|| panic_payload.downcast_ref::<String>().map(|s| s.to_owned()))
panic_utils::downcast_box_to_string(panic_payload)
.map(|message| Err(napi::Error::from_reason(format!("PANIC: {message}"))))
.ok_or(napi::Error::from_reason("PANIC: unknown panic".to_string()))
.unwrap()
Expand Down
9 changes: 9 additions & 0 deletions libs/panic-utils/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[package]
name = "panic-utils"
version = "0.1.0"
edition = "2021"

[dependencies]

[lints]
workspace = true
18 changes: 18 additions & 0 deletions libs/panic-utils/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
use std::{any::Any, borrow::Cow};

/// Downcasts a boxed [`Any`] from a panic payload to a string.
pub fn downcast_box_to_string(object: Box<dyn Any>) -> Option<Cow<'static, str>> {
object
.downcast::<&'static str>()
.map(|s| Cow::Borrowed(*s))
.or_else(|object| object.downcast::<String>().map(|s| Cow::Owned(*s)))
.ok()
}

/// Downcasts a reference to [`Any`] from panic info hook to a string.
pub fn downcast_ref_to_string(object: &dyn Any) -> Option<&str> {
object
.downcast_ref::<&str>()
.copied()
.or_else(|| object.downcast_ref::<String>().map(|s| s.as_str()))
}
1 change: 1 addition & 0 deletions libs/user-facing-errors/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ version = "0.1.0"
edition = "2021"

[dependencies]
panic-utils.path = "../panic-utils"
user-facing-error-macros = { path = "../user-facing-error-macros" }
serde_json.workspace = true
serde.workspace = true
Expand Down
7 changes: 1 addition & 6 deletions libs/user-facing-errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,12 +122,7 @@ impl Error {
/// Construct a new UnknownError from a [`PanicHookInfo`] in a panic hook. [`UnknownError`]s
/// created with this constructor will have a proper, useful backtrace.
pub fn new_in_panic_hook(panic_info: &std::panic::PanicHookInfo<'_>) -> Self {
let message = panic_info
.payload()
.downcast_ref::<&str>()
.map(|s| -> String { (*s).to_owned() })
.or_else(|| panic_info.payload().downcast_ref::<String>().map(|s| s.to_owned()))
.unwrap_or_else(|| "<unknown panic>".to_owned());
let message = panic_utils::downcast_ref_to_string(panic_info.payload()).unwrap_or("<unknown panic>");

let backtrace = Some(format!("{:?}", backtrace::Backtrace::new()));
let location = panic_info
Expand Down
19 changes: 0 additions & 19 deletions query-engine/connector-test-kit-rs/query-test-macros/build.rs

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ pub fn connector_test_impl(attr: TokenStream, input: TokenStream) -> TokenStream

// The shell function retains the name of the original test definition.
let test_fn_ident = test_function.sig.ident;
let test_fn_ident_string = test_fn_ident.to_string();

// Rename original test function to run_<orig_name>.
let runner_fn_ident = Ident::new(&format!("run_{test_fn_ident}"), Span::call_site());
Expand All @@ -63,13 +64,10 @@ pub fn connector_test_impl(attr: TokenStream, input: TokenStream) -> TokenStream
None => quote! { None },
};

let test_attrs = skip_or_ignore_attr(&test_name, &suite_name);

// The actual test is a shell function that gets the name of the original function,
// which is then calling `{orig_name}_run` in the end (see `runner_fn_ident`).
let test = quote! {
#[test]
#test_attrs
fn #test_fn_ident() {
query_tests_setup::run_connector_test(
#test_database_name,
Expand All @@ -82,6 +80,7 @@ pub fn connector_test_impl(attr: TokenStream, input: TokenStream) -> TokenStream
&[#(#db_extensions),*],
#referential_override,
#runner_fn_ident,
#test_fn_ident_string
);
}

Expand All @@ -90,4 +89,3 @@ pub fn connector_test_impl(attr: TokenStream, input: TokenStream) -> TokenStream

test.into()
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,11 @@ extern crate proc_macro;
mod args;
mod attr_map;
mod connector_test;
mod ignore_lists;
mod relation_link_test;
mod test_suite;

use args::*;
use connector_test::*;
use ignore_lists::*;
use proc_macro::TokenStream;
use relation_link_test::*;
use test_suite::*;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ pub fn relation_link_test_impl(attr: TokenStream, input: TokenStream) -> TokenSt

// The shell function retains the name of the original test definition.
let test_fn_ident = test_function.sig.ident;
let test_fn_ident_string = test_fn_ident.to_string();

// Rename original test function to run_<orig_name>.
let runner_fn_ident = Ident::new(&format!("run_{test_fn_ident}"), Span::call_site());
Expand All @@ -52,11 +53,8 @@ pub fn relation_link_test_impl(attr: TokenStream, input: TokenStream) -> TokenSt
let suite_name = args.suite.expect("A test must have a test suite.");
let required_capabilities = &args.capabilities.idents;

let test_attrs = skip_or_ignore_attr(&test_name, &suite_name);

let ts = quote! {
#[test]
#test_attrs
fn #test_fn_ident() {
query_tests_setup::run_relation_link_test(
#on_parent,
Expand All @@ -66,7 +64,8 @@ pub fn relation_link_test_impl(attr: TokenStream, input: TokenStream) -> TokenSt
&[#exclude],
enumflags2::make_bitflags!(ConnectorCapability::{#(#required_capabilities)|*}),
(#suite_name, #test_name),
#runner_fn_ident
#runner_fn_ident,
#test_fn_ident_string
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ prisma-metrics.path = "../../../libs/metrics"
quaint.workspace = true
jsonrpc-core = "17"
insta.workspace = true
futures.workspace = true
panic-utils.path = "../../../libs/panic-utils"

# Only this version is vetted, upgrade only after going through the code,
# as this is a small crate with little user base.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ impl ExecutorProcess {
Ok(Ok(process)) => process,
Ok(Err(err)) => exit_with_message(1, &format!("Failed to start node process. Details: {err}")),
Err(err) => {
let err = err.downcast_ref::<String>().map(ToOwned::to_owned).unwrap_or_default();
let err = panic_utils::downcast_box_to_string(err).unwrap_or_default();
exit_with_message(1, &format!("Panic while trying to start node process.\nDetails: {err}"))
}
}
Expand All @@ -50,9 +50,7 @@ impl ExecutorProcess {
1,
&format!(
"rpc thread panicked with: {}",
e.downcast_ref::<&str>()
.map(|s| s.to_string())
.unwrap_or_else(|| *e.downcast::<String>().unwrap_or_default())
panic_utils::downcast_box_to_string(e).unwrap_or_default()
),
);
}
Expand Down
Loading

0 comments on commit 4f41af3

Please sign in to comment.