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

Feature: eng 483 trim and fix the tests in shuttle-service #693

Merged
Show file tree
Hide file tree
Changes from 2 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
48 changes: 44 additions & 4 deletions service/src/builder.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use std::path::{Path, PathBuf};

use anyhow::{anyhow, Context};
use anyhow::{anyhow, bail, Context};
use cargo::core::compiler::{CompileKind, CompileMode, CompileTarget, MessageFormat};
use cargo::core::{Shell, Summary, Verbosity, Workspace};
use cargo::core::{Manifest, Shell, Summary, Verbosity, Workspace};
use cargo::ops::{clean, compile, CleanOptions, CompileOptions};
use cargo::util::interning::InternedString;
use cargo::util::{homedir, ToSemver};
Expand Down Expand Up @@ -51,14 +51,18 @@ pub async fn build_crate(
let manifest_path = project_path.join("Cargo.toml");
let mut ws = Workspace::new(&manifest_path, &config)?;

let current = ws.current_mut().map_err(|_| anyhow!("A Shuttle project cannot have a virtual manifest file - please ensure your Cargo.toml file specifies it as a library."))?;
let current = ws.current_mut().map_err(|_| anyhow!("A Shuttle project cannot have a virtual manifest file - please ensure the `package` table is present in your Cargo.toml file."))?;

let summary = current.manifest_mut().summary_mut();

let is_next = is_next(summary);

if !is_next {
check_version(summary)?;
ensure_binary(current.manifest())?;
} else {
ensure_cdylib(current.manifest_mut())?;
}

check_no_panic(&ws)?;

let opts = get_compile_options(&config, release_mode, is_next)?;
Expand Down Expand Up @@ -173,6 +177,42 @@ fn is_next(summary: &Summary) -> bool {
.any(|dependency| dependency.package_name() == NEXT_NAME)
}

/// Make sure the project is a binary for legacy projects.
fn ensure_binary(manifest: &Manifest) -> anyhow::Result<()> {
if manifest
.targets()
.iter()
.find(|target| target.is_bin())
.is_some()
oddgrd marked this conversation as resolved.
Show resolved Hide resolved
{
Ok(())
} else {
bail!("Your Shuttle project must be a binary.")
}
}

/// Make sure "cdylib" is set for shuttle-next projects, else set it if possible.
fn ensure_cdylib(manifest: &mut Manifest) -> anyhow::Result<()> {
if let Some(target) = manifest
.targets_mut()
.iter_mut()
.find(|target| target.is_lib())
{
if !target.is_cdylib() {
*target = cargo::core::manifest::Target::lib_target(
target.name(),
vec![cargo::core::compiler::CrateType::Cdylib],
target.src_path().path().unwrap().to_path_buf(),
target.edition(),
);
}

Ok(())
} else {
bail!("Your Shuttle project must be a library. Please add `[lib]` to your Cargo.toml file.")
}
}

/// Check that the crate being build is compatible with this version of loader
fn check_version(summary: &Summary) -> anyhow::Result<()> {
let valid_version = VERSION.to_semver().unwrap();
Expand Down
59 changes: 17 additions & 42 deletions service/tests/integration/build_crate.rs
Original file line number Diff line number Diff line change
@@ -1,64 +1,39 @@
use std::path::{Path, PathBuf};

use shuttle_service::loader::{build_crate, Runtime};
use shuttle_service::builder::{build_crate, Runtime};

#[tokio::test(flavor = "multi_thread")]
#[tokio::test]
#[should_panic]
oddgrd marked this conversation as resolved.
Show resolved Hide resolved
async fn not_shuttle() {
let (tx, _) = crossbeam_channel::unbounded();
let project_path = format!("{}/tests/resources/not-shuttle", env!("CARGO_MANIFEST_DIR"));
let so_path = match build_crate(Default::default(), Path::new(&project_path), false, tx)
build_crate(Path::new(&project_path), false, tx)
.await
.unwrap()
{
Runtime::Legacy(path) => path,
_ => unreachable!(),
};

assert!(
so_path
.display()
.to_string()
.ends_with("tests/resources/not-shuttle/target/debug/libnot_shuttle.so"),
"did not get expected so_path: {}",
so_path.display()
);
.unwrap();
}

#[tokio::test]
#[should_panic(
expected = "Your Shuttle project must be a library. Please add `[lib]` to your Cargo.toml file."
)]
async fn not_lib() {
#[should_panic(expected = "Your Shuttle project must be a binary.")]
async fn not_bin() {
let (tx, _) = crossbeam_channel::unbounded();
let project_path = format!("{}/tests/resources/not-lib", env!("CARGO_MANIFEST_DIR"));
build_crate(Default::default(), Path::new(&project_path), false, tx)
.await
.unwrap();
let project_path = format!("{}/tests/resources/not-bin", env!("CARGO_MANIFEST_DIR"));
match build_crate(Path::new(&project_path), false, tx).await {
Ok(_) => {}
Err(e) => panic!("{}", e.to_string()),
}
}

#[tokio::test(flavor = "multi_thread")]
async fn not_cdylib() {
async fn is_bin() {
let (tx, _) = crossbeam_channel::unbounded();
let project_path = format!("{}/tests/resources/not-cdylib", env!("CARGO_MANIFEST_DIR"));
assert!(matches!(
build_crate(Default::default(), Path::new(&project_path), false, tx).await,
Ok(Runtime::Legacy(_))
));
assert!(PathBuf::from(project_path)
.join("target/debug/libnot_cdylib.so")
.exists());
}
let project_path = format!("{}/tests/resources/is-bin", env!("CARGO_MANIFEST_DIR"));

#[tokio::test(flavor = "multi_thread")]
async fn is_cdylib() {
let (tx, _) = crossbeam_channel::unbounded();
let project_path = format!("{}/tests/resources/is-cdylib", env!("CARGO_MANIFEST_DIR"));
assert!(matches!(
build_crate(Default::default(), Path::new(&project_path), false, tx).await,
build_crate(Path::new(&project_path), false, tx).await,
Ok(Runtime::Legacy(_))
));
assert!(PathBuf::from(project_path)
.join("target/debug/libis_cdylib.so")
.join("target/debug/is-bin")
.exists());
}

Expand All @@ -70,7 +45,7 @@ async fn not_found() {
"{}/tests/resources/non-existing",
env!("CARGO_MANIFEST_DIR")
);
build_crate(Default::default(), Path::new(&project_path), false, tx)
build_crate(Path::new(&project_path), false, tx)
.await
.unwrap();
}
28 changes: 0 additions & 28 deletions service/tests/integration/helpers/loader.rs

This file was deleted.

4 changes: 0 additions & 4 deletions service/tests/integration/helpers/mod.rs

This file was deleted.

134 changes: 0 additions & 134 deletions service/tests/integration/helpers/sqlx.rs

This file was deleted.

Loading