Merge branch 'main' into vk/more-full-compiler-passes-in-tests

.github/actions/rust-targeted-unit-tests/action.yaml

@@ -0,0 +1,49 @@
+name: Rust Targeted Unit Tests
+description: Runs only the targeted rust unit tests
+inputs:
+  GIT_CREDENTIALS:
+    description: "Optional credentials to pass to git. Useful if you need to pull private repos for dependencies"
+    required: false
+
+runs:
+  using: composite
+  steps:
+    # The source code must be checked out by the workflow that invokes this action.
+    - uses: aptos-labs/aptos-core/.github/actions/rust-setup@main
+      with:
+        GIT_CREDENTIALS: ${{ inputs.GIT_CREDENTIALS }}
+
+    # Install nextest
+    - uses: taiki-e/[email protected]
+      with:
+        tool: nextest
+
+    # Run a postgres database
+    - name: Run postgres database
+      run: docker run --detach -p 5432:5432 cimg/postgres:14.2
+      shell: bash
+
+    # Output the changed files
+    - name: Output the changed files
+      run: cargo x changed-files -vv
+      shell: bash
+
+    # Output the affected packages
+    - name: Output the affected packages
+      run: cargo x affected-packages -vv
+      shell: bash
+
+    # Run only the targeted rust unit tests
+    - name: Run only the targeted unit tests
+      run: |
+        cargo x targeted-unit-tests -vv run --profile ci --cargo-profile ci --locked --no-fail-fast --retries 3
+      shell: bash
+      env:
+        INDEXER_DATABASE_URL: postgresql://postgres@localhost/postgres
+        RUST_MIN_STACK: "4297152"
+        MVP_TEST_ON_CI: "true"
+        SOLC_EXE: /home/runner/bin/solc
+        Z3_EXE: /home/runner/bin/z3
+        CVC5_EXE: /home/runner/bin/cvc5
+        DOTNET_ROOT: /home/runner/.dotnet
+        BOOGIE_EXE: /home/runner/.dotnet/tools/boogie

.github/actions/rust-unit-tests/action.yaml

@@ -12,7 +12,6 @@ runs:
   using: composite
   steps:
     # The source code must be checkout out by the workflow that invokes this action.
-
     - uses: aptos-labs/aptos-core/.github/actions/rust-setup@main
       with:
         GIT_CREDENTIALS: ${{ inputs.GIT_CREDENTIALS }}
@@ -31,7 +30,7 @@ runs:
     - uses: taiki-e/[email protected]
       with:
         tool: nextest
-    
+
     # Install buildkite-test-collector
     - name: Install buildkite-test-collector
       run: cargo install buildkite-test-collector

.github/workflows/lint-test.yaml

@@ -106,9 +106,30 @@ jobs:
       - run: echo "Skipping rust smoke tests! Unrelated changes detected."
         if: needs.file_change_determinator.outputs.only_docs_changed == 'true'
 
+  # Run only the targeted rust unit tests. This is a PR required job.
+  rust-targeted-unit-tests:
+    needs: file_change_determinator
+    runs-on: high-perf-docker
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          ref: ${{ github.event.pull_request.head.sha }}
+          fetch-depth: 0 # Fetch all git history for accurate target determination
+      - name: Run targeted rust unit tests
+        uses: ./.github/actions/rust-targeted-unit-tests
+        with:
+          GIT_CREDENTIALS: ${{ secrets.GIT_CREDENTIALS }}
+
   # Run all rust unit tests. This is a PR required job.
   rust-unit-tests:
     needs: file_change_determinator
+    if: | # Only run on each PR once an appropriate event occurs
+      (
+        github.event_name == 'workflow_dispatch' ||
+        github.event_name == 'push' ||
+        contains(github.event.pull_request.labels.*.name, 'CICD:run-all-unit-tests') ||
+        github.event.pull_request.auto_merge != null
+      )
     runs-on: high-perf-docker
     steps:
       - uses: actions/checkout@v4

devtools/aptos-cargo-cli/src/cargo.rs

@@ -1,12 +1,13 @@
 // Copyright © Aptos Foundation
 // SPDX-License-Identifier: Apache-2.0
 
-use log::debug;
+use log::info;
 use std::{
     ffi::{OsStr, OsString},
     process::{Command, Stdio},
 };
 
+#[derive(Debug)]
 pub struct Cargo {
     inner: Command,
     pass_through_args: Vec<OsString>,
@@ -46,14 +47,36 @@ impl Cargo {
     }
 
     pub fn run(&mut self) {
+        // Set up the output and arguments
         self.inner.stdout(Stdio::inherit()).stderr(Stdio::inherit());
-
         if !self.pass_through_args.is_empty() {
             self.inner.arg("--").args(&self.pass_through_args);
         }
 
-        debug!("Executing command: {:?}", self.inner);
+        // Log the command
+        let command_to_execute = format!("{:?}", self.inner);
+        info!("Executing command: {:?}", command_to_execute);
+
+        // Execute the command
+        let result = self.inner.output();
 
-        let _ = self.inner.output();
+        // If the command failed, panic immediately with the error.
+        // This will ensure that failures are not dropped silently.
+        match result {
+            Ok(output) => {
+                if !output.status.success() {
+                    panic!(
+                        "Command failed: {:?}. Output: {:?}",
+                        command_to_execute, output
+                    );
+                }
+            },
+            Err(error) => {
+                panic!(
+                    "Unexpected error executing command: {:?}. Error: {:?}",
+                    command_to_execute, error
+                );
+            },
+        }
     }
 }

devtools/aptos-cargo-cli/src/common.rs

@@ -3,8 +3,18 @@
 
 use anyhow::anyhow;
 use clap::Args;
-use determinator::{Determinator, Utf8Paths0};
-use guppy::{graph::DependencyDirection, CargoMetadata, MetadataCommand};
+use determinator::{
+    rules::{DeterminatorMarkChanged, DeterminatorPostRule, DeterminatorRules, PathRule},
+    Determinator, Utf8Paths0,
+};
+use guppy::{
+    graph::{
+        cargo::{CargoOptions, CargoResolverVersion},
+        DependencyDirection, PackageGraph,
+    },
+    CargoMetadata, MetadataCommand,
+};
+use log::{debug, info};
 use std::{
     fs,
     io::Read,
@@ -13,6 +23,23 @@ use std::{
 };
 use url::Url;
 
+// File types in `aptos-core` that are not relevant to the rust build and test process.
+// Note: this is a best effort list and will need to be updated as time goes on.
+const IGNORED_DETERMINATOR_FILE_TYPES: [&str; 4] = ["*.json", "*.md", "*.yaml", "*.yml"];
+
+// Paths in `aptos-core` that are not relevant to the rust build and test process.
+// Note: this is a best effort list and will need to be updated as time goes on.
+const IGNORED_DETERMINATOR_PATHS: [&str; 8] = [
+    ".assets/*",
+    ".github/*",
+    ".vscode/*",
+    "dashboards/*",
+    "developer-docs-site/*",
+    "docker/*",
+    "scripts/*",
+    "terraform/*",
+];
+
 fn workspace_dir() -> PathBuf {
     let output = Command::new("cargo")
         .arg("locate-project")
@@ -92,17 +119,17 @@ impl SelectedPackageArgs {
         Ok(CargoMetadata::parse_json(&contents)?)
     }
 
-    pub fn compute_packages(&self) -> anyhow::Result<Vec<String>> {
-        if !self.package.is_empty() {
-            return Ok(self.package.clone());
-        }
-
-        // Determine merge base
-        // TODO: support different merge bases
-        let merge_base = "origin/main";
+    /// Identifies the changed files compared to the merge base, and
+    /// returns the relevant package graphs and file list.
+    pub fn identify_changed_files(
+        &self,
+    ) -> anyhow::Result<(PackageGraph, PackageGraph, Utf8Paths0)> {
+        // Determine the merge base
+        let merge_base = self.identify_merge_base();
+        info!("Identified the merge base: {:?}", merge_base);
 
         // Download merge base metadata
-        let base_metadata = self.fetch_remote_metadata(merge_base)?;
+        let base_metadata = self.fetch_remote_metadata(&merge_base)?;
         let base_package_graph = base_metadata.build_graph().unwrap();
 
         // Compute head metadata
@@ -112,14 +139,72 @@ impl SelectedPackageArgs {
         let head_package_graph = head_metadata.build_graph().unwrap();
 
         // Compute changed files
-        let changed_files = self.compute_changed_files(merge_base)?;
+        let changed_files = self.compute_changed_files(&merge_base)?;
+        debug!("Identified the changed files: {:?}", changed_files);
+
+        // Return the package graphs and the changed files
+        Ok((base_package_graph, head_package_graph, changed_files))
+    }
+
+    /// Identifies the merge base to compare against. This is done by identifying
+    /// the commit at which the current branch forked off origin/main.
+    /// TODO: do we need to make this more intelligent?
+    fn identify_merge_base(&self) -> String {
+        // Run the git merge-base command
+        let output = Command::new("git")
+            .arg("merge-base")
+            .arg("HEAD")
+            .arg("origin/main")
+            .output()
+            .expect("failed to execute git merge-base");
+
+        // Return the output
+        String::from_utf8(output.stdout)
+            .expect("invalid UTF-8")
+            .trim()
+            .to_owned()
+    }
+
+    /// Computes the affected target packages based on the
+    /// merge base and changed file set.
+    pub fn compute_target_packages(&self) -> anyhow::Result<Vec<String>> {
+        if !self.package.is_empty() {
+            return Ok(self.package.clone());
+        }
+
+        // Compute changed files
+        let (base_package_graph, head_package_graph, changed_files) =
+            self.identify_changed_files()?;
 
-        // Run target determinator
+        // Create the determinator using the package graphs
         let mut determinator = Determinator::new(&base_package_graph, &head_package_graph);
-        // The determinator expects a list of changed files to be passed in.
+
+        // Add the changed files to the determinator
         determinator.add_changed_paths(&changed_files);
 
+        // Set the cargo options for the determinator
+        let mut cargo_options = CargoOptions::new();
+        cargo_options.set_resolver(CargoResolverVersion::V2);
+        determinator.set_cargo_options(&cargo_options);
+
+        // Set the ignore rules for the determinator
+        let mut rules = DeterminatorRules::default();
+        for globs in [
+            IGNORED_DETERMINATOR_FILE_TYPES.to_vec(),
+            IGNORED_DETERMINATOR_PATHS.to_vec(),
+        ] {
+            rules.path_rules.push(PathRule {
+                globs: globs.iter().map(|string| string.to_string()).collect(),
+                mark_changed: DeterminatorMarkChanged::Packages(vec![]),
+                post_rule: DeterminatorPostRule::Skip,
+            });
+        }
+        determinator.set_rules(&rules).unwrap();
+
+        // Run the target determinator
         let determinator_set = determinator.compute();
+
+        // Collect the affected packages
         let package_set = determinator_set
             .affected_set
             .packages(DependencyDirection::Forward)