Skip to content

Commit

Permalink
fix: 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 20a5343 commit b97aa4c
Show file tree
Hide file tree
Showing 12 changed files with 134 additions and 101 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 }}
2 changes: 2 additions & 0 deletions Cargo.lock

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

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
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
use std::{
fs::File,
io::{BufRead, BufReader},
path::PathBuf,
sync::OnceLock,
};

static IGNORED_TESTS: OnceLock<Vec<String>> = OnceLock::new();
static SHOULD_FAIL_TESTS: OnceLock<Vec<String>> = OnceLock::new();

pub fn is_ignored(test_name: &str) -> bool {
is_in_list(test_name, "IGNORED_TESTS", &IGNORED_TESTS)
}

pub fn is_expected_to_fail(test_name: &str) -> bool {
is_in_list(test_name, "SHOULD_FAIL_TESTS", &SHOULD_FAIL_TESTS)
}

fn is_in_list(test_name: &str, env_var: &'static str, cache: &OnceLock<Vec<String>>) -> bool {
let list_file = match std::env::var(env_var) {
Ok(file) => file,
Err(_) => return false,
};

let tests = cache.get_or_init(|| {
let workspace_root = std::env::var("WORKSPACE_ROOT").expect("WORKSPACE_ROOT env must be set");

let path = PathBuf::from(workspace_root).join(list_file);
let file = File::open(path).expect("could not open file");
let reader = BufReader::new(file);

reader
.lines()
.map(|line| line.expect("could not read line"))
.map(|line| {
let trimmed = line.trim();
if line != trimmed {
trimmed.to_owned()
} else {
line
}
})
.filter(|line| !line.is_empty() && !line.starts_with('#'))
.collect()
});

tests.iter().any(|t| t == test_name)
}
Loading

0 comments on commit b97aa4c

Please sign in to comment.