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

refactor(test): rust integration tests #9314

Merged
merged 17 commits into from
Oct 25, 2024
Merged
Show file tree
Hide file tree
Changes from 5 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
56 changes: 56 additions & 0 deletions crates/turborepo/tests/common/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
use std::{path::Path, process::Command};

use camino::Utf8Path;

pub fn setup_fixture(
fixture: &str,
package_manager: Option<&str>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: it would be nice to keep this explicit so it's really clear from looking at the test what it's gonna run with. or, if not, it'd be nice if there was some way to make check! variadic (is there?).

test_dir: &Path,
) -> Result<(), anyhow::Error> {
let script_path = Utf8Path::new(env!("CARGO_MANIFEST_DIR"))
.join("../../turborepo-tests/helpers/setup_integration_test.sh");

Command::new("bash")
.arg("-c")
.arg(format!(
"{} {} {}",
script_path,
fixture,
package_manager.unwrap_or("npm@10.5.0")
))
.current_dir(test_dir)
.spawn()?
.wait()?;
chris-olszewski marked this conversation as resolved.
Show resolved Hide resolved

Ok(())
}

/// Executes a command with different arguments in a specific fixture and
/// package manager. Creates a snapshot file for each set of arguments
#[macro_export]
macro_rules! check {
($fixture:expr, $package_manager:expr, $command:expr, $($name:expr => $query:expr,)*) => {
{
let tempdir = tempfile::tempdir()?;
crate::common::setup_fixture($fixture, $package_manager, tempdir.path())?;
$(
let output = assert_cmd::Command::cargo_bin("turbo")?
.arg($command)
.arg($query)
.current_dir(tempdir.path())
.output()?;

let stdout = String::from_utf8(output.stdout)?;
let query_output: serde_json::Value = serde_json::from_str(&stdout)?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be worth changing the macro name/doc comments to make it clear this is useful only for commands that produce JSON output

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor, but there still isn't mention in the doc comment or macro name that this is for JSON command only.

let test_name = format!(
"{}_{}_{}_({})",
module_path!(),
$fixture,
$name.replace(' ', "_"),
$package_manager
);
insta::assert_json_snapshot!(test_name, query_output);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we put each of these asserts in it's own test? Currently if the first test case fails, none of the other ones will run. Prysk will continue running if one of the commands doesn't produce matching output.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to do this while also using the same fixture? I don't want to spin up a fixture for each test. Insta does also let you do the prysk behavior with cargo insta test --review or INSTA_FORCE_PASS=1 cargo test --no-fail-fast

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose we'd have to store the tempfile in something like a static ref and have special cleanup logic. I think eventually we'll want beforeAll/afterAll behavior to make debugging failures easier. ITG

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Insta does also let you do the prysk behavior with cargo insta test --review or INSTA_FORCE_PASS=1 cargo test --no-fail-fast

I don't think these do what we want, INSTA_FORCE_PASS=1 seems to just forcibly pass all snapshot assertions without printing any diffs. cargo insta test --review is just equivalent to --interactive in prysk.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to this those are the recommended ways to not fail on the first test

)*
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
---
source: crates/turborepo-lib/tests/common/mod.rs
expression: query_output
---
{
"data": {
"file": {
"path": "index.js",
"dependencies": {
"files": {
"items": [
{
"path": "nm/index.js"
}
]
},
"errors": {
"items": []
}
}
}
}
}
69 changes: 24 additions & 45 deletions crates/turborepo/tests/query.rs
Original file line number Diff line number Diff line change
@@ -1,52 +1,13 @@
use std::{path::Path, process::Command};

use camino::Utf8Path;

fn setup_fixture(
fixture: &str,
package_manager: Option<&str>,
test_dir: &Path,
) -> Result<(), anyhow::Error> {
let script_path = Utf8Path::new(env!("CARGO_MANIFEST_DIR"))
.join("../../turborepo-tests/helpers/setup_integration_test.sh");

Command::new("bash")
.arg("-c")
.arg(format!(
"{} {} {}",
script_path,
fixture,
package_manager.unwrap_or("npm@10.5.0")
))
.current_dir(test_dir)
.spawn()?
.wait()?;

Ok(())
}

fn check_query(fixture: &str, query: &str) -> Result<(), anyhow::Error> {
let tempdir = tempfile::tempdir()?;
setup_fixture(fixture, None, tempdir.path())?;
let output = assert_cmd::Command::cargo_bin("turbo")?
.arg("query")
.arg(query)
.current_dir(tempdir.path())
.output()?;

let stdout = String::from_utf8(output.stdout)?;
let query_output: serde_json::Value = serde_json::from_str(&stdout)?;
insta::assert_json_snapshot!(query_output);

Ok(())
}
mod common;

#[cfg(not(windows))]
#[test]
fn test_double_symlink() -> Result<(), anyhow::Error> {
check_query(
check!(
"oxc_repro",
"query {
"npm@10.5.0",

Check failure on line 8 in crates/turborepo/tests/query.rs

View workflow job for this annotation

GitHub Actions / Turborepo Rust testing on ubuntu

mismatched types

Check failure on line 8 in crates/turborepo/tests/query.rs

View workflow job for this annotation

GitHub Actions / Turborepo Rust testing on macos

mismatched types
"query",
"get_dependencies" => "query {
file(path: \"./index.js\") {
path
dependencies {
Expand All @@ -55,6 +16,24 @@
}
}
}",
)?;
);
Ok(())
}

#[test]
fn test_trace() -> Result<(), anyhow::Error> {
check!(
"turbo_trace",
"npm@10.5.0",

Check failure on line 27 in crates/turborepo/tests/query.rs

View workflow job for this annotation

GitHub Actions / Turborepo Rust testing on ubuntu

mismatched types

Check failure on line 27 in crates/turborepo/tests/query.rs

View workflow job for this annotation

GitHub Actions / Turborepo Rust testing on macos

mismatched types

Check failure on line 27 in crates/turborepo/tests/query.rs

View workflow job for this annotation

GitHub Actions / Turborepo Rust testing on windows

mismatched types
"query",
"get `main.ts`" => "query { file(path: \"main.ts\") { path } }",
"get `main.ts` with dependencies" => "query { file(path: \"main.ts\") { path, dependencies { files { items { path } } } } }",
"get `button.tsx` with dependencies" => "query { file(path: \"button.tsx\") { path, dependencies { files { items { path } } } } }",
"get `circular.ts` with dependencies" => "query { file(path: \"circular.ts\") { path dependencies { files { items { path } } } } }",
"get `invalid.ts` with dependencies" => "query { file(path: \"invalid.ts\") { path dependencies { files { items { path } } errors { items { message } } } } }",
"get `main.ts` with ast" => "query { file(path: \"main.ts\") { path ast } }",
"get `main.ts` with depth = 0" => "query { file(path: \"main.ts\") { path dependencies(depth: 1) { files { items { path } } } } }",
);

Ok(())
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
---
source: crates/turborepo/tests/query.rs
expression: query_output
---
{
"data": {
"file": {
"path": "index.js",
"dependencies": {
"files": {
"items": [
{
"path": "nm/index.js"
}
]
},
"errors": {
"items": []
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
---
source: crates/turborepo/tests/query.rs
expression: query_output
---
{
"data": {
"file": {
"path": "button.tsx",
"dependencies": {
"files": {
"items": []
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
---
source: crates/turborepo/tests/query.rs
expression: query_output
---
{
"data": {
"file": {
"path": "circular.ts",
"dependencies": {
"files": {
"items": [
{
"path": "circular2.ts"
}
]
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
---
source: crates/turborepo/tests/query.rs
expression: query_output
---
{
"data": {
"file": {
"path": "invalid.ts",
"dependencies": {
"files": {
"items": [
{
"path": "button.tsx"
}
]
},
"errors": {
"items": [
{
"message": "failed to resolve import"
}
]
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
source: crates/turborepo/tests/query.rs
expression: query_output
---
{
"data": {
"file": {
"path": "main.ts"
}
}
}
Loading
Loading