Skip to content

Commit

Permalink
Fix mutation tests (#140)
Browse files Browse the repository at this point in the history
<!-- The PR description should answer 2 (maybe 3) important questions:
-->

### What

<!-- What is this PR trying to accomplish (and why, if it's not
obvious)? -->
This PR separates the testing configuration with the sample
configuration to enable mutation tests to run without any dependency on
the sample configuration.

### How

<!-- How is it trying to accomplish it (what are the implementation
steps)? -->
  • Loading branch information
codingkarthik authored Jun 28, 2024
1 parent cd6a57c commit 7f7efa7
Show file tree
Hide file tree
Showing 16 changed files with 19,117 additions and 95 deletions.
57 changes: 0 additions & 57 deletions Cargo.lock

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

1 change: 0 additions & 1 deletion crates/ndc-sqlserver/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -48,4 +48,3 @@ similar-asserts = "1.5.0"
tokio-util = "0.7.11"
uuid = { version = "1.8.0", features = ["v4"]}
anyhow = "1.0.82"
serial_test = "3.1.1"
43 changes: 28 additions & 15 deletions crates/ndc-sqlserver/tests/common/fresh_deployments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,43 +20,56 @@ impl FreshDeployment {
pub async fn create(
original_connection_db_config: MSSQLDatabaseConfig,
ndc_metadata_path: impl AsRef<Path>,
data_setup_file_paths: Vec<PathBuf>,
) -> anyhow::Result<FreshDeployment> {
let db_config =
let new_db_config =
database::create_fresh_database(original_connection_db_config.clone()).await;

let temp_deploys_path = PathBuf::from("static/temp-deploys");
let temp_deploys_path = PathBuf::from("static/tests/temp-deploys");

let new_connection_uri = db_config.construct_uri();
let new_connection_uri = new_db_config.construct_uri();

configuration::copy_ndc_metadata_with_new_mssql_url(
ndc_metadata_path,
&new_connection_uri,
temp_deploys_path,
&db_config.db_name,
&new_db_config.db_name,
)
.await?;

let new_ndc_metadata_path = PathBuf::from("static/temp-deploys").join(&db_config.db_name);
let new_ndc_metadata_path =
PathBuf::from("static/tests/temp-deploys").join(&new_db_config.db_name);

let init_db_sql_file_path = get_path_from_project_root("static/chinook-sqlserver.sql");
let init_db_sql_file_path =
get_path_from_project_root("static/tests/chinook-sqlserver.sql");

// The `chinook-sqlserver.sql` file contains `GO` after every statement. Running, it as it
// is leads to syntax errors. So, just replacing all `GO`s with an empty string. This works!
let init_db_sql = fs::read_to_string(init_db_sql_file_path)
.unwrap()
.replace("GO", "");
let init_db_sql = fs::read_to_string(init_db_sql_file_path).unwrap();

let mut new_db_connection = create_mssql_connection(&db_config).await;
let mut new_db_connection = create_mssql_connection(&new_db_config).await;

new_db_connection
.execute(init_db_sql, &[])
.simple_query(init_db_sql)
.await
.expect("Error initializing the newly created DB");

for file_path in data_setup_file_paths.into_iter() {
let query = fs::read_to_string(file_path.clone()).expect(&format!(
"Failed to read file from {}",
file_path.to_str().unwrap()
));

new_db_connection
.simple_query(query)
.await
.unwrap_or_else(|_| {
panic!("Error in running the query present in the file: {file_path:#?}")
});
}

Ok(FreshDeployment {
db_name: db_config.db_name,
db_name: new_db_config.db_name,
ndc_metadata_path: new_ndc_metadata_path,
connection_uri: new_connection_uri.clone(),
connection_uri: new_connection_uri,
admin_connection_uri: original_connection_db_config.construct_uri(),
})
}
Expand Down
24 changes: 18 additions & 6 deletions crates/ndc-sqlserver/tests/common/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,21 +21,33 @@ pub fn create_client(router: axum::Router) -> TestClient {

/// Run a query against the server, get the result, and compare against the snapshot.
pub async fn run_query(testname: &str) -> serde_json::Value {
let router = create_router(SQLSERVER_CONNECTION_STRING).await;
run_query_with_connection_uri(testname, SQLSERVER_CONNECTION_STRING).await
}

/// Run a query against the server, get the result, and compare against the snapshot.
pub async fn run_query_with_connection_uri(
testname: &str,
connection_string: &str,
) -> serde_json::Value {
let router = create_router(connection_string).await;
let client = create_client(router);
run_against_server(&client, "query", testname, StatusCode::OK).await
}

/// Run a query against the server, get the result, and compare against the snapshot.
pub async fn run_mutation(testname: &str) -> serde_json::Value {
let router = create_router(SQLSERVER_CONNECTION_STRING).await;
pub async fn run_mutation(testname: &str, db_connection_string: String) -> serde_json::Value {
let router = create_router(&db_connection_string).await;
let client = create_client(router);
run_against_server(&client, "mutation", testname, StatusCode::OK).await
}

/// Run a query against the server, get the result, and compare against the snapshot.
pub async fn run_mutation_fail(testname: &str, expected_status: StatusCode) -> serde_json::Value {
let router = create_router(SQLSERVER_CONNECTION_STRING).await;
pub async fn run_mutation_fail(
testname: &str,
db_connection_string: String,
expected_status: StatusCode,
) -> serde_json::Value {
let router = create_router(&db_connection_string).await;
let client = create_client(router);
run_against_server(&client, "mutation", testname, expected_status).await
}
Expand Down Expand Up @@ -162,6 +174,6 @@ pub fn is_contained_in_lines(keywords: Vec<&str>, lines: String) {
/// and get our single static configuration file.
pub fn get_deployment_file() -> PathBuf {
let mut d = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
d.push("../../static/");
d.push("../../static/tests/");
d
}
24 changes: 23 additions & 1 deletion crates/ndc-sqlserver/tests/configuration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ use configuration::environment::Variable;

use ndc_sqlserver_configuration as configuration;
use ndc_sqlserver_configuration::secret;
use query_engine_metadata::metadata::{NativeMutations, NativeQueries};
use serde::de::DeserializeOwned;
use serde_json::from_value;
use similar_asserts::assert_eq;

pub mod common;
Expand Down Expand Up @@ -41,10 +44,22 @@ pub async fn configure_is_idempotent(
)]);
let file_path = PathBuf::new();

let actual = configuration::configure(&file_path, &args, environment)
let mut actual = configuration::configure(&file_path, &args, environment)
.await
.expect("configuration::configure");

// Native queries and native mutations are defined by the user and cannot
// be obtained by introspecting the database. So, add the native queries and
// mutations back manually.
let native_queries_config: NativeQueries =
read_configuration_as("static/chinook-native-queries.json").unwrap();

let native_mutations_config: NativeMutations =
read_configuration_as("static/chinook-native-mutations.json").unwrap();

actual.metadata.native_mutations = native_mutations_config;
actual.metadata.native_queries = native_queries_config;

let actual_value = serde_json::to_value(actual).expect("serde_json::to_value");

assert_eq!(expected_value, actual_value);
Expand Down Expand Up @@ -77,3 +92,10 @@ fn read_configuration(chinook_deployment_path: impl AsRef<Path>) -> serde_json::
.expect("fs::File::open");
serde_json::from_reader(file).expect("serde_json::from_reader")
}

fn read_configuration_as<T: DeserializeOwned>(
chinook_deployment_path: impl AsRef<Path>,
) -> Result<T, serde_json::Error> {
let v = read_configuration(chinook_deployment_path);
from_value(v)
}
31 changes: 18 additions & 13 deletions crates/ndc-sqlserver/tests/mutation_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,31 +6,35 @@ mod native_mutations {

use super::common::fresh_deployments::FreshDeployment;

use serial_test::serial;

#[tokio::test(flavor = "multi_thread")]
#[serial]
async fn native_mutation_insert_artist_and_return_id() {
let original_db_config = MSSQLDatabaseConfig::original_db_config();
let _ndc_metadata = FreshDeployment::create(original_db_config, "static")
let fresh_deployment = FreshDeployment::create(original_db_config, "static", vec![])
.await
.unwrap();

let result = run_mutation("insert_artist_and_return_id").await;
let result = run_mutation(
"insert_artist_and_return_id",
fresh_deployment.connection_uri.clone(),
)
.await;

insta::assert_json_snapshot!(result);
}

#[tokio::test(flavor = "multi_thread")]
#[serial]
/// Native mutation that selects a relationship.
async fn native_mutation_insert_artist_and_return_artist() {
let original_db_config = MSSQLDatabaseConfig::original_db_config();
let _ndc_metadata = FreshDeployment::create(original_db_config, "static")
let fresh_deployment = FreshDeployment::create(original_db_config, "static", vec![])
.await
.unwrap();

let result = run_mutation("insert_artist_and_return_artist").await;
let result = run_mutation(
"insert_artist_and_return_artist",
fresh_deployment.connection_uri.clone(),
)
.await;

insta::assert_json_snapshot!(result);
}
Expand All @@ -39,19 +43,17 @@ mod native_mutations {
mod negative_native_mutations_test {
use crate::common::{
database::MSSQLDatabaseConfig,
helpers::{run_mutation_fail, run_query},
helpers::{run_mutation_fail, run_query_with_connection_uri},
};

use super::common::fresh_deployments::FreshDeployment;

use hyper::StatusCode;
use serial_test::serial;

#[tokio::test(flavor = "multi_thread")]
#[serial]
async fn test_atomicity_native_mutations() {
let original_db_config = MSSQLDatabaseConfig::original_db_config();
let _ndc_metadata = FreshDeployment::create(original_db_config, "static")
let fresh_deployment = FreshDeployment::create(original_db_config, "static", vec![])
.await
.unwrap();

Expand All @@ -61,11 +63,14 @@ mod negative_native_mutations_test {
// back.
let _ = run_mutation_fail(
"fail_insert_artist_and_return_id",
fresh_deployment.connection_uri.clone(),
StatusCode::INTERNAL_SERVER_ERROR,
)
.await;

let result = run_query("fetch_artist_count").await;
let result =
run_query_with_connection_uri("fetch_artist_count", &fresh_deployment.connection_uri)
.await;

insta::assert_json_snapshot!(result);
}
Expand Down
35 changes: 35 additions & 0 deletions static/chinook-native-mutations.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
{
"insert_artist_and_return_id": {
"sql": "INSERT INTO [dbo].[Artist] (ArtistId, Name) OUTPUT inserted.* VALUES ({{ArtistId}}, {{Name}})",
"columns": {
"ArtistId": {
"name": "ArtistId",
"type": "int",
"nullable": "nonNullable",
"description": null
},
"Name": {
"name": "Name",
"type": "varchar",
"nullable": "nullable",
"description": null,
"castAs": "varchar(100)"
}
},
"arguments": {
"ArtistId": {
"name": "ArtistId",
"type": "int",
"nullable": "nonNullable",
"description": null
},
"Name": {
"name": "Name",
"type": "varchar",
"nullable": "nullable",
"description": null
}
},
"description": null
}
}
Loading

0 comments on commit 7f7efa7

Please sign in to comment.