Minor cleanup for crate_universe (#2644)

crate_universe/BUILD.bazel

@@ -128,6 +128,7 @@ rust_test(
         "CARGO": "$(rootpath @rules_rust//rust/toolchain:current_cargo_files)",
         "RUSTC": "$(rootpath @rules_rust//rust/toolchain:current_rustc_files)",
     },
+    tags = ["requires-network"],
     deps = [
         ":cargo_bazel",
         "@rules_rust//tools/runfiles",

crate_universe/src/api/lockfile.rs

@@ -20,7 +20,7 @@ pub fn parse(path: &Path) -> Result<impl CargoBazelLockfile> {
     Ok(lockfile)
 }
 
-/// CargoBazelLockfile provides a view over cargo-bazel's lockfile format,
+/// `CargoBazelLockfile` provides a view over `cargo-bazel`'s lockfile format,
 /// providing information about the third-party dependencies of a workspace.
 /// While the lockfile's format doesn't provide any kind of compatibility guarantees over time,
 /// this type offers an interface which is likely to be publicly supportable.

crate_universe/src/cli/query.rs

@@ -71,7 +71,7 @@ pub fn query(opt: QueryOptions) -> Result<()> {
         &opt.rustc,
     )?;
     if digest != expected {
-        return announce_repin(&format!("Digests do not match: {digest:?} != {expected:?}",));
+        return announce_repin(&format!("Digests do not match: {digest:?} != {expected:?}"));
     }
 
     // There is no need to repin

crate_universe/src/cli/splice.rs

@@ -45,7 +45,7 @@ pub struct SpliceOptions {
     #[clap(long)]
     pub cargo_config: Option<PathBuf>,
 
-    /// The path to the config file (containing `cargo_bazel::config::Config`.)
+    /// The path to the config file (containing [crate::config::Config].)
     #[clap(long)]
     pub config: PathBuf,
 

crate_universe/src/context.rs

@@ -83,7 +83,7 @@ impl Context {
             .collect();
 
         // Given a list of all conditional dependencies, build a set of platform
-        // triples which satsify the conditions.
+        // triples which satisfy the conditions.
         let conditions = resolve_cfg_platforms(
             crates.values().collect(),
             &annotations.config.supported_platform_triples,
@@ -254,7 +254,7 @@ mod test {
     }
 
     #[test]
-    fn workspace_member_deps() {
+    fn workspace_member_deps_collection() {
         let context = mock_context_common();
         let workspace_member_deps = context.workspace_member_deps();
 
@@ -295,7 +295,7 @@ mod test {
     fn serialization() {
         let context = mock_context_aliases();
 
-        // Seralize and deseralize the context object
+        // Serialize and deserialize the context object
         let json_text = serde_json::to_string(&context).unwrap();
         let deserialized_context: Context = serde_json::from_str(&json_text).unwrap();
 

crate_universe/src/context/platforms.rs

@@ -245,7 +245,7 @@ mod test {
             assert_eq!(
                 configurations,
                 BTreeMap::from([
-                    (configuration, expectation,),
+                    (configuration, expectation),
                     // All known triples.
                     (
                         "aarch64-apple-darwin".to_owned(),

crate_universe/test_data/private/metadata_generator.py

@@ -12,7 +12,7 @@
 
 
 def run_subprocess(command: List[str]) -> subprocess.CompletedProcess[str]:
-    proc = subprocess.run(command, capture_output=True)
+    proc = subprocess.run(command, capture_output=True, check=False)
 
     if proc.returncode:
         print("Subcommand exited with error", proc.returncode, file=sys.stderr)
@@ -24,11 +24,14 @@ def run_subprocess(command: List[str]) -> subprocess.CompletedProcess[str]:
     return proc
 
 
-if __name__ == "__main__":
-
+def main() -> None:
+    """The main entrypoint."""
     workspace_root = Path(
-        os.environ.get("BUILD_WORKSPACE_DIRECTORY",
-                       str(Path(__file__).parent.parent.parent.parent.parent)))
+        os.environ.get(
+            "BUILD_WORKSPACE_DIRECTORY",
+            str(Path(__file__).parent.parent.parent.parent.parent),
+        )
+    )
     metadata_dir = workspace_root / "crate_universe/test_data/metadata"
     cargo = os.getenv("CARGO", "cargo")
 
@@ -51,11 +54,15 @@ def run_subprocess(command: List[str]) -> subprocess.CompletedProcess[str]:
             lockfile = manifest_dir / "Cargo.lock"
 
             if lockfile.exists():
-                proc = run_subprocess([cargo, "update", "--manifest-path", str(manifest), "--workspace"])
+                proc = run_subprocess(
+                    [cargo, "update", "--manifest-path", str(manifest), "--workspace"]
+                )
             else:
                 # Generate Lockfile
-                proc = run_subprocess([cargo, "generate-lockfile", "--manifest-path", str(manifest)])
-
+                proc = run_subprocess(
+                    [cargo, "generate-lockfile", "--manifest-path", str(manifest)]
+                )
+
                 if not lockfile.exists():
                     print("Faield to generate lockfile")
                     print("Args:", proc.args, file=sys.stderr)
@@ -66,16 +73,16 @@ def run_subprocess(command: List[str]) -> subprocess.CompletedProcess[str]:
             shutil.copy2(str(lockfile), str(test_dir / "Cargo.lock"))
 
             # Generate metadata
-            proc = subprocess.run(
-                [cargo, "metadata", "--format-version", "1", "--manifest-path", str(manifest)],
-                capture_output=True)
-
-            if proc.returncode:
-                print("Subcommand exited with error", proc.returncode, file=sys.stderr)
-                print("Args:", proc.args, file=sys.stderr)
-                print("stderr:", proc.stderr.decode("utf-8"), file=sys.stderr)
-                print("stdout:", proc.stdout.decode("utf-8"), file=sys.stderr)
-                exit(proc.returncode)
+            proc = run_subprocess(
+                [
+                    cargo,
+                    "metadata",
+                    "--format-version",
+                    "1",
+                    "--manifest-path",
+                    str(manifest),
+                ]
+            )
 
             cargo_home = os.environ.get("CARGO_HOME", str(Path.home() / ".cargo"))
 
@@ -88,3 +95,7 @@ def run_subprocess(command: List[str]) -> subprocess.CompletedProcess[str]:
             metadata = json.loads(metadata_text)
             output = test_dir / "metadata.json"
             output.write_text(json.dumps(metadata, indent=4, sort_keys=True) + "\n")
+
+
+if __name__ == "__main__":
+    main()

crate_universe/tests/cargo_integration_test.rs

@@ -1,3 +1,5 @@
+//! cargo_bazel integration tests that run Cargo to test generating metadata.
+
 extern crate cargo_bazel;
 extern crate serde_json;
 extern crate tempfile;
@@ -38,9 +40,10 @@ fn setup_cargo_env() -> Result<(PathBuf, PathBuf)> {
     env::set_var("CARGO_HOME", cargo_home.as_os_str());
     fs::create_dir_all(&cargo_home)?;
 
-    println!("$RUSTC={}", rustc.display());
-    println!("$CARGO={}", cargo.display());
-    println!("$CARGO_HOME={}", cargo_home.display());
+    println!("Environment:");
+    println!("\tRUSTC={}", rustc.display());
+    println!("\tCARGO={}", cargo.display());
+    println!("\tCARGO_HOME={}", cargo_home.display());
 
     Ok((cargo, rustc))
 }
@@ -51,15 +54,6 @@ fn run(repository_name: &str, manifests: HashMap<String, String>, lockfile: &str
     let scratch = tempfile::tempdir().unwrap();
     let runfiles = runfiles::Runfiles::create().unwrap();
 
-    /*
-    let manifest_path = scratch.path().join("Cargo.toml");
-    fs::copy(
-        runfiles::rlocation!(runfiles, manifest),
-        manifest_path,
-    )
-    .unwrap();
-    */
-
     let splicing_manifest = scratch.path().join("splicing_manifest.json");
     fs::write(
         &splicing_manifest,