Skip to content

Commit

Permalink
feat(workspace_tests): test to enforce no self-dependencies
Browse files Browse the repository at this point in the history
  • Loading branch information
dorimedini-starkware committed Jan 24, 2025
1 parent 970e3e6 commit 68106d3
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 1 deletion.
25 changes: 25 additions & 0 deletions workspace_tests/package_integrity_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,31 @@ use std::path::PathBuf;

use crate::toml_utils::{CrateCargoToml, DependencyValue, PackageEntryValue, MEMBER_TOMLS};

/// Tests that no member crate has itself in it's dependency tree.
/// This may occur if, for example, a developer wants to activate a feature of a crate in tests, and
/// adds a dependency on itself in dev-dependencies with this feature active.
/// Note: a common (erroneous) use case would be to activate the "testing" feature of a crate in
/// tests by adding a dependency on itself in dev-dependencies. This is not allowed; any code gated
/// by the testing feature should also be gated by `test`.
#[test]
fn test_no_self_dependencies() {
let members_with_self_deps: Vec<String> = MEMBER_TOMLS
.iter()
.filter_map(|(name, toml)| {
if toml.member_dependency_names_recursive(true).contains(toml.package_name()) {
Some(name.clone())
} else {
None
}
})
.collect();
assert!(
members_with_self_deps.is_empty(),
"The following crates have themselves in their dependency tree: \
{members_with_self_deps:?}. This is not allowed."
);
}

#[test]
fn test_package_names_match_directory() {
let mismatched_packages: Vec<_> = MEMBER_TOMLS
Expand Down
62 changes: 61 additions & 1 deletion workspace_tests/toml_utils.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::collections::HashMap;
use std::collections::{HashMap, HashSet};
use std::fs;
use std::path::Path;
use std::sync::LazyLock;
Expand Down Expand Up @@ -53,6 +53,10 @@ pub(crate) struct CrateCargoToml {
}

impl CrateCargoToml {
pub(crate) fn from_name(name: &String) -> Self {
MEMBER_TOMLS.get(name).unwrap_or_else(|| panic!("No member crate '{name}' found.")).clone()
}

pub(crate) fn package_name(&self) -> &String {
match self.package.get("name") {
Some(PackageEntryValue::String(name)) => name,
Expand All @@ -69,6 +73,62 @@ impl CrateCargoToml {
}
})
}

/// Returns all direct member dependencies of self.
pub(crate) fn member_dependency_names(
&self,
include_dev_dependencies: bool,
) -> HashSet<String> {
let member_crate_names: HashSet<&String> =
MEMBER_TOMLS.values().map(CrateCargoToml::package_name).collect();

self.dependencies
.iter()
.flatten()
.chain(if include_dev_dependencies {
self.dev_dependencies.iter().flatten()
} else {
None.iter().flatten()
})
.filter_map(
|(name, _value)| {
if member_crate_names.contains(name) { Some(name.clone()) } else { None }
},
)
.collect()
}

/// Helper function for member_dependency_names_recursive.
fn member_dependency_names_recursive_aux(
&self,
include_dev_dependencies: bool,
processed_member_names: &mut HashSet<String>,
) -> HashSet<String> {
let direct_member_dependencies = self.member_dependency_names(include_dev_dependencies);
let mut members = HashSet::new();
for toml in direct_member_dependencies.iter().map(CrateCargoToml::from_name) {
// To prevent infinite recursion, we only recurse on members that have not been
// processed yet. If a member depends on itself, this can lead to a loop.
let dep_name = toml.package_name();
members.insert(dep_name.clone());
if !processed_member_names.contains(dep_name) {
processed_member_names.insert(dep_name.clone());
members.extend(toml.member_dependency_names_recursive_aux(
include_dev_dependencies,
processed_member_names,
));
}
}
members
}

/// Returns all member dependencies of self in the dependency tree.
pub(crate) fn member_dependency_names_recursive(
&self,
include_dev_dependencies: bool,
) -> HashSet<String> {
self.member_dependency_names_recursive_aux(include_dev_dependencies, &mut HashSet::new())
}
}

#[derive(Debug)]
Expand Down

0 comments on commit 68106d3

Please sign in to comment.