Skip to content

Commit

Permalink
Unskip tests for driver adapters core features, skip tests for low fi…
Browse files Browse the repository at this point in the history
…delity cases (#4400)
  • Loading branch information
Miguel Fernández authored Nov 13, 2023
1 parent b23895f commit 7711046
Show file tree
Hide file tree
Showing 15 changed files with 167 additions and 39 deletions.
2 changes: 0 additions & 2 deletions .github/workflows/query-engine-driver-adapters.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@ jobs:
setup_task: 'dev-neon-ws-postgres13'
- name: 'libsql'
setup_task: 'dev-libsql-sqlite'
- name: 'planetscale'
setup_task: 'dev-planetscale-vitess8'
node_version: ['18']
env:
LOG_LEVEL: 'info' # Set to "debug" to trace the query engine and node process running the driver adapter
Expand Down
8 changes: 4 additions & 4 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -130,10 +130,10 @@ test-pg-postgres13: dev-pg-postgres13 test-qe-st

test-driver-adapter-pg: test-pg-postgres13

start-neon-postgres13: build-qe-napi build-connector-kit-js
start-neon-postgres13:
docker compose -f docker-compose.yml up --wait -d --remove-orphans neon-postgres13

dev-neon-ws-postgres13: start-neon-postgres13
dev-neon-ws-postgres13: start-neon-postgres13 build-qe-napi build-connector-kit-js
cp $(CONFIG_PATH)/neon-ws-postgres13 $(CONFIG_FILE)

test-neon-ws-postgres13: dev-neon-ws-postgres13 test-qe-st
Expand Down Expand Up @@ -268,10 +268,10 @@ start-vitess_8_0:
dev-vitess_8_0: start-vitess_8_0
cp $(CONFIG_PATH)/vitess_8_0 $(CONFIG_FILE)

start-planetscale-vitess8: build-qe-napi build-connector-kit-js
start-planetscale-vitess8:
docker compose -f docker-compose.yml up -d --remove-orphans planetscale-vitess8

dev-planetscale-vitess8: start-planetscale-vitess8
dev-planetscale-vitess8: start-planetscale-vitess8 build-qe-napi build-connector-kit-js
cp $(CONFIG_PATH)/planetscale-vitess8 $(CONFIG_FILE)

test-planetscale-vitess8: dev-planetscale-vitess8 test-qe-st
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use query_engine_tests::*;

#[test_suite(schema(generic), only(Postgres))]
#[test_suite(schema(generic))]
mod raw_params {
#[connector_test]
#[connector_test(only(Postgres), exclude(JS))]
async fn value_too_many_bind_variables(runner: Runner) -> TestResult<()> {
let n = 32768;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ mod interactive_tx {
Ok(())
}

#[connector_test(exclude(JS))]
#[connector_test]
async fn batch_queries_failure(mut runner: Runner) -> TestResult<()> {
// Tx expires after five second.
let tx_id = runner.start_tx(5000, 5000, None).await?;
Expand Down Expand Up @@ -256,7 +256,7 @@ mod interactive_tx {
Ok(())
}

#[connector_test(exclude(JS))]
#[connector_test]
async fn tx_expiration_failure_cycle(mut runner: Runner) -> TestResult<()> {
// Tx expires after one seconds.
let tx_id = runner.start_tx(5000, 1000, None).await?;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ mod one2one_req {
}

/// Deleting the parent reconnects the child to the default and fails (the default doesn't exist).
#[connector_test(schema(required_with_default), exclude(MongoDb, MySQL, JS))]
#[connector_test(schema(required_with_default), exclude(MongoDb, MySQL))]
async fn delete_parent_no_exist_fail(runner: Runner) -> TestResult<()> {
insta::assert_snapshot!(
run_query!(&runner, r#"mutation { createOneParent(data: { id: 1, child: { create: { id: 1 }}}) { id }}"#),
Expand Down Expand Up @@ -167,7 +167,7 @@ mod one2one_opt {
}

/// Deleting the parent reconnects the child to the default and fails (the default doesn't exist).
#[connector_test(schema(optional_with_default), exclude(MongoDb, MySQL, JS))]
#[connector_test(schema(optional_with_default), exclude(MongoDb, MySQL))]
async fn delete_parent_no_exist_fail(runner: Runner) -> TestResult<()> {
insta::assert_snapshot!(
run_query!(&runner, r#"mutation { createOneParent(data: { id: 1, child: { create: { id: 1 }}}) { id }}"#),
Expand Down Expand Up @@ -270,7 +270,7 @@ mod one2many_req {
}

/// Deleting the parent reconnects the child to the default and fails (the default doesn't exist).
#[connector_test(schema(required_with_default), exclude(MongoDb, MySQL, JS))]
#[connector_test(schema(required_with_default), exclude(MongoDb, MySQL))]
async fn delete_parent_no_exist_fail(runner: Runner) -> TestResult<()> {
insta::assert_snapshot!(
run_query!(&runner, r#"mutation { createOneParent(data: { id: 1, children: { create: { id: 1 }}}) { id }}"#),
Expand Down Expand Up @@ -371,7 +371,7 @@ mod one2many_opt {
}

/// Deleting the parent reconnects the child to the default and fails (the default doesn't exist).
#[connector_test(schema(optional_with_default), exclude(MongoDb, MySQL, JS))]
#[connector_test(schema(optional_with_default), exclude(MongoDb, MySQL))]
async fn delete_parent_no_exist_fail(runner: Runner) -> TestResult<()> {
insta::assert_snapshot!(
run_query!(&runner, r#"mutation { createOneParent(data: { id: 1, children: { create: { id: 1 }}}) { id }}"#),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ mod one2one_req {
}

/// Updating the parent reconnects the child to the default and fails (the default doesn't exist).
#[connector_test(schema(required_with_default), exclude(MongoDb, MySQL, JS))]
#[connector_test(schema(required_with_default), exclude(MongoDb, MySQL))]
async fn update_parent_no_exist_fail(runner: Runner) -> TestResult<()> {
insta::assert_snapshot!(
run_query!(&runner, r#"mutation { createOneParent(data: { id: 1, uniq: "1", child: { create: { id: 1 }}}) { id }}"#),
Expand Down Expand Up @@ -171,7 +171,7 @@ mod one2one_opt {
}

/// Updating the parent reconnects the child to the default and fails (the default doesn't exist).
#[connector_test(schema(optional_with_default), exclude(MongoDb, MySQL, JS))]
#[connector_test(schema(optional_with_default), exclude(MongoDb, MySQL))]
async fn update_parent_no_exist_fail(runner: Runner) -> TestResult<()> {
insta::assert_snapshot!(
run_query!(&runner, r#"mutation { createOneParent(data: { id: 1, uniq: "1", child: { create: { id: 1 }}}) { id }}"#),
Expand Down Expand Up @@ -276,7 +276,7 @@ mod one2many_req {
}

/// Updating the parent reconnects the child to the default and fails (the default doesn't exist).
#[connector_test(schema(required_with_default), exclude(MongoDb, MySQL, JS))]
#[connector_test(schema(required_with_default), exclude(MongoDb, MySQL))]
async fn update_parent_no_exist_fail(runner: Runner) -> TestResult<()> {
insta::assert_snapshot!(
run_query!(&runner, r#"mutation { createOneParent(data: { id: 1, uniq: "1", children: { create: { id: 1 }}}) { id }}"#),
Expand Down Expand Up @@ -379,7 +379,7 @@ mod one2many_opt {
}

/// Updating the parent reconnects the child to the default and fails (the default doesn't exist).
#[connector_test(schema(optional_with_default), exclude(MongoDb, MySQL, JS))]
#[connector_test(schema(optional_with_default), exclude(MongoDb, MySQL))]
async fn update_parent_no_exist_fail(runner: Runner) -> TestResult<()> {
insta::assert_snapshot!(
run_query!(&runner, r#"mutation { createOneParent(data: { id: 1, uniq: "1", children: { create: { id: 1 }}}) { id }}"#),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,8 +187,8 @@ mod max_integer {
schema.to_owned()
}

#[connector_test(schema(overflow_pg), only(Postgres))]
async fn unfitted_int_should_fail_pg(runner: Runner) -> TestResult<()> {
#[connector_test(schema(overflow_pg), only(Postgres), exclude(JS))]
async fn unfitted_int_should_fail_pg_quaint(runner: Runner) -> TestResult<()> {
// int
assert_error!(
runner,
Expand Down Expand Up @@ -234,6 +234,55 @@ mod max_integer {
Ok(())
}

// The driver adapter for neon provides different error messages on overflow
#[connector_test(schema(overflow_pg), only(JS, Postgres))]
async fn unfitted_int_should_fail_pg_js(runner: Runner) -> TestResult<()> {
// int
assert_error!(
runner,
format!("mutation {{ createOneTest(data: {{ int: {I32_OVERFLOW_MAX} }}) {{ id }} }}"),
None,
"value \\\"2147483648\\\" is out of range for type integer"
);
assert_error!(
runner,
format!("mutation {{ createOneTest(data: {{ int: {I32_OVERFLOW_MIN} }}) {{ id }} }}"),
None,
"value \\\"-2147483649\\\" is out of range for type integer"
);

// smallint
assert_error!(
runner,
format!("mutation {{ createOneTest(data: {{ smallint: {I16_OVERFLOW_MAX} }}) {{ id }} }}"),
None,
"value \\\"32768\\\" is out of range for type smallint"
);
assert_error!(
runner,
format!("mutation {{ createOneTest(data: {{ smallint: {I16_OVERFLOW_MIN} }}) {{ id }} }}"),
None,
"value \\\"-32769\\\" is out of range for type smallint"
);

//oid
assert_error!(
runner,
format!("mutation {{ createOneTest(data: {{ oid: {U32_OVERFLOW_MAX} }}) {{ id }} }}"),
None,
"value \\\"4294967296\\\" is out of range for type oid"
);

// The underlying driver swallows a negative id by interpreting it as unsigned.
// {"data":{"createOneTest":{"id":1,"oid":4294967295}}}
run_query!(
runner,
format!("mutation {{ createOneTest(data: {{ oid: {OVERFLOW_MIN} }}) {{ id, oid }} }}")
);

Ok(())
}

#[connector_test(schema(overflow_pg), only(Postgres))]
async fn fitted_int_should_work_pg(runner: Runner) -> TestResult<()> {
// int
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ mod conversion_error {
schema.to_owned()
}

#[connector_test(schema(schema_int))]
async fn convert_to_int(runner: Runner) -> TestResult<()> {
#[connector_test(schema(schema_int), only(Sqlite), exclude(JS))]
async fn convert_to_int_sqlite_quaint(runner: Runner) -> TestResult<()> {
create_test_data(&runner).await?;

assert_error!(
Expand All @@ -38,8 +38,22 @@ mod conversion_error {
Ok(())
}

#[connector_test(schema(schema_bigint))]
async fn convert_to_bigint(runner: Runner) -> TestResult<()> {
#[connector_test(schema(schema_int), only(Sqlite, JS))]
async fn convert_to_int_sqlite_js(runner: Runner) -> TestResult<()> {
create_test_data(&runner).await?;

assert_error!(
runner,
r#"query { findManyTestModel { field } }"#,
2023,
"Inconsistent column data: Conversion failed: number must be an integer in column 'field'"
);

Ok(())
}

#[connector_test(schema(schema_bigint), only(Sqlite), exclude(JS))]
async fn convert_to_bigint_sqlite_quaint(runner: Runner) -> TestResult<()> {
create_test_data(&runner).await?;

assert_error!(
Expand All @@ -52,6 +66,20 @@ mod conversion_error {
Ok(())
}

#[connector_test(schema(schema_bigint), only(Sqlite, JS))]
async fn convert_to_bigint_sqlite_js(runner: Runner) -> TestResult<()> {
create_test_data(&runner).await?;

assert_error!(
runner,
r#"query { findManyTestModel { field } }"#,
2023,
"Inconsistent column data: Conversion failed: number must be an i64 in column 'field'"
);

Ok(())
}

async fn create_test_data(runner: &Runner) -> TestResult<()> {
run_query!(
runner,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,9 @@ mod json {
Ok(())
}

#[connector_test(schema(json_opt))]
// The external runner for driver adapters, in spite of the protocol being used in the test matrix
// uses the JSON representation of queries, so this test should not apply to driver adapters (exclude(JS))
#[connector_test(schema(json_opt), exclude(JS))]
async fn nested_not_shorthand(runner: Runner) -> TestResult<()> {
// Those tests pass with the JSON protocol because the entire object is parsed as JSON.
// They remain useful to ensure we don't ever allow a full JSON filter input object type at the schema level.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,20 @@ use query_engine_tests::*;
mod casts {
use query_engine_tests::{fmt_query_raw, run_query, RawParam};

#[connector_test]
// The following tests are excluded for driver adapters. The underlying
// driver rejects queries where the values of the positional arguments do
// not match the expected types. As an example, the following query to the
// driver
//
// ```json
// {
// sql: 'SELECT $1::int4 AS decimal_to_i4; ',
// args: [ 42.51 ]
// }
//
// Bails with: ERROR: invalid input syntax for type integer: "42.51"
//
#[connector_test(only(Postgres), exclude(JS))]
async fn query_numeric_casts(runner: Runner) -> TestResult<()> {
insta::assert_snapshot!(
run_query_pretty!(&runner, fmt_query_raw(r#"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ mod raw_errors {
Ok(())
}

#[connector_test(schema(common_nullable_types))]
async fn list_param_for_scalar_column_should_not_panic(runner: Runner) -> TestResult<()> {
#[connector_test(schema(common_nullable_types), only(Postgres), exclude(JS))]
async fn list_param_for_scalar_column_should_not_panic_quaint(runner: Runner) -> TestResult<()> {
assert_error!(
runner,
fmt_execute_raw(
Expand All @@ -48,4 +48,19 @@ mod raw_errors {

Ok(())
}

#[connector_test(schema(common_nullable_types), only(JS, Postgres))]
async fn list_param_for_scalar_column_should_not_panic_pg_js(runner: Runner) -> TestResult<()> {
assert_error!(
runner,
fmt_execute_raw(
r#"INSERT INTO "TestModel" ("id") VALUES ($1);"#,
vec![RawParam::array(vec![1])],
),
2010,
r#"invalid input syntax for type integer"#
);

Ok(())
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -296,19 +296,33 @@ pub(crate) fn should_run(
return false;
}

if !only.is_empty() {
return only
.iter()
.any(|only| ConnectorVersion::try_from(*only).unwrap().matches_pattern(&version));
}

// We skip tests that exclude JS driver adapters when an external test executor is configured.
// A test that you only want to run with rust drivers can be annotated with exclude(JS)
if CONFIG.external_test_executor().is_some() && exclude.iter().any(|excl| excl.0.to_uppercase() == "JS") {
println!("Excluded test execution for JS driver adapters. Skipping test");
return false;
};
// we consume the JS token to prevent it from being used in the following checks
let exclude: Vec<_> = exclude.iter().filter(|excl| excl.0.to_uppercase() != "JS").collect();

// We only run tests that include JS driver adapters when an external test executor is configured.
// A test that you only want to run with js driver adapters can be annotated with only(JS)
if CONFIG.external_test_executor().is_none() && only.iter().any(|incl| incl.0.to_uppercase() == "JS") {
println!("Excluded test execution for rust driver adapters. Skipping test");
return false;
}
// we consume the JS token to prevent it from being used in the following checks
let only: Vec<_> = only.iter().filter(|incl| incl.0.to_uppercase() != "JS").collect();

if !only.is_empty() {
return only
.iter()
.any(|incl| ConnectorVersion::try_from(**incl).unwrap().matches_pattern(&version));
}

if exclude.iter().any(|excl| {
ConnectorVersion::try_from(*excl).map_or(false, |connector_version| connector_version.matches_pattern(&version))
ConnectorVersion::try_from(**excl)
.map_or(false, |connector_version| connector_version.matches_pattern(&version))
}) {
println!("Connector excluded. Skipping test.");
return false;
Expand Down
2 changes: 1 addition & 1 deletion query-engine/driver-adapters/src/conversion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ impl ToNapiValue for JSArg {
for (index, item) in items.into_iter().enumerate() {
let js_value = ToNapiValue::to_napi_value(env.raw(), item)?;
// TODO: NapiRaw could be implemented for sys::napi_value directly, there should
// be no need for re-wrapping; submit a patch to napi-rs and simplify here.
// be no need for re-wrapping; submit a patch to napi-rs and simplify here.
array.set(index as u32, napi::JsUnknown::from_raw_unchecked(env.raw(), js_value))?;
}

Expand Down
2 changes: 1 addition & 1 deletion query-engine/driver-adapters/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ pub(crate) fn into_quaint_error(napi_err: NapiError) -> QuaintError {
QuaintError::raw_connector_error(status, reason)
}

/// catches a panic thrown during the executuin of an asynchronous closure and transforms it into
/// catches a panic thrown during the execution of an asynchronous closure and transforms it into
/// the Error variant of a napi::Result.
pub(crate) async fn async_unwinding_panic<F, R>(fut: F) -> napi::Result<R>
where
Expand Down
Loading

0 comments on commit 7711046

Please sign in to comment.