Skip to content

Commit

Permalink
Try to use current package in the add command
Browse files Browse the repository at this point in the history
Also isolate each test run. Previously all tests were sharing
the same target/tests/Cargo.toml, which each test was modifying. This
made order in which tests run significant and difficult to debug.
Add test `two_projects_in_one_workspace_validate_add_from_path` which
cretes two projects in the same workspace to make sure we modify the
current package's manifest.
  • Loading branch information
tomasol committed Jan 11, 2024
1 parent 205c6ee commit 1528db8
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 31 deletions.
24 changes: 22 additions & 2 deletions src/commands/add.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use cargo_component_core::{
registry::{Dependency, DependencyResolution, DependencyResolver, RegistryPackage},
VersionedPackageId,
};
use cargo_metadata::Package;
use cargo_metadata::{Metadata, Package};
use clap::Args;
use semver::VersionReq;
use std::{
Expand Down Expand Up @@ -62,13 +62,33 @@ pub struct AddCommand {
}

impl AddCommand {
fn find_current_package_spec(metadata: &Metadata) -> Option<CargoPackageSpec> {
let current_manifest = fs::read_to_string("Cargo.toml").ok()?;
let document: Document = current_manifest.parse().ok()?; // TODO do not parse the manifest twice
let name = document["package"]["name"].as_str()?;
let version = metadata
.packages
.iter()
.find(|found| found.name == name)
.map(|found| found.version.clone());
Some(CargoPackageSpec {
name: name.to_string(),
version: version,
})
}

/// Executes the command
pub async fn exec(self) -> Result<()> {
let config = Config::new(self.common.new_terminal())?;
let metadata = load_metadata(config.terminal(), self.manifest_path.as_deref(), false)?;

let spec = match &self.spec {
Some(spec) => Some(spec.clone()),
None => Self::find_current_package_spec(&metadata),
};

let PackageComponentMetadata { package, metadata }: PackageComponentMetadata<'_> =
match &self.spec {
match &spec {
Some(spec) => {
let pkgs = load_component_metadata(&metadata, std::iter::once(spec), false)?;
assert!(pkgs.len() == 1, "one package should be present");
Expand Down
21 changes: 17 additions & 4 deletions tests/add.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,10 +150,7 @@ async fn prints_modified_manifest_for_dry_run() -> Result<()> {
Ok(())
}

#[test]
fn validate_add_from_path() -> Result<()> {
let project = Project::new("foo")?;

fn validate_add_from_path(project: &Project) -> Result<()> {
project
.cargo_component("add --path foo/baz foo:baz")
.assert()
Expand All @@ -167,6 +164,22 @@ fn validate_add_from_path() -> Result<()> {
let manifest = fs::read_to_string(project.root().join("Cargo.toml"))?;
assert!(contains(r#""foo:baz" = { path = "foo/baz" }"#).eval(&manifest));
assert!(contains(r#""foo:qux" = { path = "foo/qux" }"#).eval(&manifest));
Ok(())
}

#[test]
fn test_validate_add_from_path() -> Result<()> {
let project = Project::new("foo")?;
validate_add_from_path(&project)?;
Ok(())
}

#[test]
fn two_projects_in_one_workspace_validate_add_from_path() -> Result<()> {
let root = create_root()?;
let foo = Project::with_root(&root, "foo", "")?;
let bar = Project::with_root(&root, "bar", "")?;
validate_add_from_path(&foo)?;
validate_add_from_path(&bar)?;
Ok(())
}
34 changes: 9 additions & 25 deletions tests/support/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,24 +50,19 @@ pub fn root() -> Result<PathBuf> {
path.pop(); // remove `debug` or `release`
path.push("tests");
path.push("cargo-component");
fs::create_dir_all(&path)?;

exclude_test_directories()?;
let root = path.join(format!("t{id}"));
fs::create_dir_all(&root)?;
exclude_test_directories(&root)?;

Ok(path.join(format!("t{id}")))
Ok(root)
}

// This works around an apparent bug in cargo where
// a directory is explicitly excluded from a workspace,
// but `cargo new` still detects `workspace.package` settings
// and sets them to be inherited in the new project.
fn exclude_test_directories() -> Result<()> {
let mut path = env::current_exe()?;
path.pop(); // remove test exe name
path.pop(); // remove `deps`
path.pop(); // remove `debug` or `release`
path.push("tests");
path.push("Cargo.toml");
fn exclude_test_directories(root: &PathBuf) -> Result<()> {
let path = root.join("Cargo.toml");

if !path.exists() {
fs::write(
Expand All @@ -84,10 +79,8 @@ fn exclude_test_directories() -> Result<()> {
}

pub fn create_root() -> Result<PathBuf> {
let root = root()?;
drop(fs::remove_dir_all(&root));
fs::create_dir_all(&root)?;
Ok(root)
drop(fs::remove_dir_all(&root()?));
root()
}

pub fn cargo_component(args: &str) -> Command {
Expand Down Expand Up @@ -276,16 +269,7 @@ impl ProjectBuilder {

impl Project {
pub fn new(name: &str) -> Result<Self> {
let root = create_root()?;

cargo_component(&format!("new --reactor {name}"))
.current_dir(&root)
.assert()
.try_success()?;

Ok(Self {
root: root.join(name),
})
Self::with_root(&create_root()?, name, "")
}

pub fn new_bin(name: &str) -> Result<Self> {
Expand Down

0 comments on commit 1528db8

Please sign in to comment.