test(e2e): gate protocol runner by requested features (#2912)

2026-07-26 08:18:18 +00:00 · 2026-05-11 21:59:24 +08:00
parent aba65a448c
commit 941986b331
3 changed files with 192 additions and 40 deletions
@@ -41,7 +41,9 @@ use walkdir::WalkDir;
 // Common constants for all E2E tests
 pub const DEFAULT_ACCESS_KEY: &str = "rustfsadmin";
 pub const DEFAULT_SECRET_KEY: &str = "rustfsadmin";
+pub const ENV_RUSTFS_BUILD_FEATURES: &str = "RUSTFS_BUILD_FEATURES";
 pub const TEST_BUCKET: &str = "e2e-test-bucket";
+const RUSTFS_FULL_FEATURE: &str = "full";

 fn build_test_s3_config(endpoint_url: &str, access_key: &str, secret_key: &str, provider_name: &'static str) -> Config {
    let credentials = Credentials::new(access_key, secret_key, None, None, provider_name);
@@ -83,6 +85,7 @@ pub fn rustfs_binary_path_with_features(requested_features: Option<&str>) -> Pat
    if let Some(path) = std::env::var_os("CARGO_BIN_EXE_rustfs") {
        return PathBuf::from(path);
    }
+    let requested_features = requested_features.and_then(normalize_rustfs_build_features);

    let mut binary_path = workspace_root();
    binary_path.push("target");
@@ -90,7 +93,7 @@ pub fn rustfs_binary_path_with_features(requested_features: Option<&str>) -> Pat
    binary_path.push(profile_dir);
    binary_path.push(format!("rustfs{}", std::env::consts::EXE_SUFFIX));

-    let features_match = binary_features_match(&binary_path, requested_features);
+    let features_match = binary_features_match(&binary_path, requested_features.as_deref());
    let source_is_newer = workspace_sources_newer_than_binary(&binary_path);
    let can_reuse_inside_e2e = running_inside_e2e_test_binary() && requested_features.is_none() && features_match;
    if binary_path.is_file() && features_match && (!source_is_newer || can_reuse_inside_e2e) {
@@ -105,7 +108,7 @@ pub fn rustfs_binary_path_with_features(requested_features: Option<&str>) -> Pat
    }

    info!("Building RustFS binary to ensure it's up to date...");
-    build_rustfs_binary(requested_features);
+    build_rustfs_binary(requested_features.as_deref());

    info!("Using RustFS binary at {:?}", binary_path);
    binary_path
@@ -134,11 +137,31 @@ fn running_inside_e2e_test_binary() -> bool {
    std::env::var("CARGO_PKG_NAME").is_ok_and(|value| value == "e2e_test")
 }

-fn requested_rustfs_build_features() -> Option<String> {
-    std::env::var("RUSTFS_BUILD_FEATURES")
+pub fn requested_rustfs_build_features() -> Option<String> {
+    std::env::var(ENV_RUSTFS_BUILD_FEATURES)
        .ok()
-        .map(|value| value.trim().to_string())
-        .filter(|value| !value.is_empty())
+        .and_then(|value| normalize_rustfs_build_features(&value))
+}
+
+pub fn normalize_rustfs_build_features(features: &str) -> Option<String> {
+    let features = features
+        .split(',')
+        .map(str::trim)
+        .filter(|feature| !feature.is_empty())
+        .map(str::to_ascii_lowercase)
+        .collect::<Vec<_>>();
+
+    if features.is_empty() { None } else { Some(features.join(",")) }
+}
+
+pub fn rustfs_build_feature_enabled(requested_features: Option<&str>, required_feature: &str) -> bool {
+    let Some(requested_features) = requested_features.and_then(normalize_rustfs_build_features) else {
+        return true;
+    };
+
+    requested_features
+        .split(',')
+        .any(|feature| feature.eq_ignore_ascii_case(RUSTFS_FULL_FEATURE) || feature.eq_ignore_ascii_case(required_feature))
 }

 fn rustfs_binary_features_stamp_path(binary_path: &Path) -> PathBuf {
@@ -147,11 +170,14 @@ fn rustfs_binary_features_stamp_path(binary_path: &Path) -> PathBuf {

 fn binary_features_match(binary_path: &Path, requested_features: Option<&str>) -> bool {
    let stamp_path = rustfs_binary_features_stamp_path(binary_path);
-    let recorded = stdfs::read_to_string(stamp_path).ok().map(|value| value.trim().to_string());
+    let recorded = stdfs::read_to_string(stamp_path)
+        .ok()
+        .and_then(|value| normalize_rustfs_build_features(&value));
+    let requested = requested_features.and_then(normalize_rustfs_build_features);

-    match requested_features {
+    match requested.as_deref() {
        Some(features) => recorded.as_deref() == Some(features),
-        None => recorded.as_deref().is_none_or(str::is_empty),
+        None => recorded.is_none(),
    }
 }

@@ -900,3 +926,36 @@ impl Drop for RustFSTestClusterEnvironment {
        }
    }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn normalizes_rustfs_build_features() {
+        assert_eq!(
+            normalize_rustfs_build_features(" SFTP, ftps ,, WebDAV "),
+            Some("sftp,ftps,webdav".to_string())
+        );
+        assert_eq!(normalize_rustfs_build_features(" , "), None);
+    }
+
+    #[test]
+    fn full_feature_enables_any_required_feature() {
+        assert!(rustfs_build_feature_enabled(Some("full"), "sftp"));
+        assert!(rustfs_build_feature_enabled(Some("ftps, full"), "webdav"));
+    }
+
+    #[test]
+    fn binary_feature_stamp_matching_uses_normalized_features() {
+        let binary_path = std::env::temp_dir().join(format!("rustfs-feature-stamp-test-{}", Uuid::new_v4()));
+        let stamp_path = rustfs_binary_features_stamp_path(&binary_path);
+
+        stdfs::write(&stamp_path, " SFTP, ftps ").expect("write feature stamp");
+        assert!(binary_features_match(&binary_path, Some("sftp,ftps")));
+        assert!(binary_features_match(&binary_path, Some(" SFTP, FTPS ")));
+        assert!(!binary_features_match(&binary_path, Some("sftp")));
+
+        stdfs::remove_file(stamp_path).ok();
+    }
+}
@@ -16,9 +16,9 @@ RUSTFS_BUILD_FEATURES=ftps,webdav,sftp cargo test --package e2e_test test_protoc
 ```

 `RUSTFS_BUILD_FEATURES` controls which features the test rustfs binary is
-built with. The protocol test runner schedules every entry (FTPS, WebDAV,
-SFTP) regardless of the feature set, so the binary must include every
-protocol the runner spawns or the corresponding entries will fail.
+built with. When this variable is set, the protocol test runner schedules
+only entries whose protocol is present in the requested feature list. Leave
+it unset to run every protocol entry.
 `--test-threads=1` is required because every entry spawns a rustfs server
 on fixed bind ports.

@@ -15,6 +15,7 @@
 //! Protocol test runner

 use crate::common::init_logging;
+use crate::common::{requested_rustfs_build_features, rustfs_build_feature_enabled};
 use crate::protocols::ftps_core::test_ftps_core_operations;
 use crate::protocols::sftp_compliance::{
    test_sftp_compliance_readonly, test_sftp_compliance_standalone, test_sftp_compliance_suite,
@@ -59,36 +60,22 @@ pub struct ProtocolTestSuite {

 #[derive(Debug, Clone)]
 struct TestDefinition {
-    name: String,
+    name: &'static str,
+    required_feature: &'static str,
 }

 impl ProtocolTestSuite {
    /// Create default test suite
    pub fn new() -> Self {
-        let tests = vec![
-            TestDefinition {
-                name: "test_ftps_core_operations".to_string(),
-            },
-            TestDefinition {
-                name: "test_webdav_core_operations".to_string(),
-            },
-            TestDefinition {
-                name: "test_sftp_core_operations".to_string(),
-            },
-            TestDefinition {
-                name: "test_sftp_compliance_suite".to_string(),
-            },
-            TestDefinition {
-                name: "test_sftp_compliance_readonly".to_string(),
-            },
-            TestDefinition {
-                name: "test_sftp_idle_timeout_disconnects".to_string(),
-            },
-            TestDefinition {
-                name: "test_sftp_compliance_standalone".to_string(),
-            },
-        ];
+        let requested_features = requested_rustfs_build_features();
+        Self::with_requested_features(requested_features.as_deref())
+    }

+    fn with_requested_features(requested_features: Option<&str>) -> Self {
+        let tests = all_protocol_tests()
+            .into_iter()
+            .filter(|test| rustfs_build_feature_enabled(requested_features, test.required_feature))
+            .collect();
        Self { tests }
    }

@@ -104,7 +91,7 @@ impl ProtocolTestSuite {

        // Run tests
        for (i, test_def) in self.tests.iter().enumerate() {
-            let test_description = match test_def.name.as_str() {
+            let test_description = match test_def.name {
                "test_ftps_core_operations" => {
                    info!("=== Starting FTPS Module Test ===");
                    "FTPS core operations (put, ls, mkdir, rmdir, delete)"
@@ -147,11 +134,11 @@ impl ProtocolTestSuite {
            match result {
                Ok(_) => {
                    info!("Test passed: {} ({:.2}s)", test_def.name, test_duration.as_secs_f64());
-                    results.push(TestResult::success(test_def.name.clone()));
+                    results.push(TestResult::success(test_def.name.to_string()));
                }
                Err(e) => {
                    error!("Test failed: {} ({:.2}s): {}", test_def.name, test_duration.as_secs_f64(), e);
-                    results.push(TestResult::failure(test_def.name.clone(), e.to_string()));
+                    results.push(TestResult::failure(test_def.name.to_string(), e.to_string()));
                }
            }

@@ -169,7 +156,7 @@ impl ProtocolTestSuite {

    /// Run a single test
    async fn run_single_test(&self, test_def: &TestDefinition) -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
-        match test_def.name.as_str() {
+        match test_def.name {
            "test_ftps_core_operations" => test_ftps_core_operations().await.map_err(|e| e.into()),
            "test_webdav_core_operations" => test_webdav_core_operations().await.map_err(|e| e.into()),
            "test_sftp_core_operations" => test_sftp_core_operations().await.map_err(|e| e.into()),
@@ -186,6 +173,10 @@ impl ProtocolTestSuite {
        info!("=== Test Suite Summary ===");
        info!("Total duration: {:.2}s", total_duration.as_secs_f64());
        info!("Total tests: {}", results.len());
+        if results.is_empty() {
+            info!("No protocol tests scheduled for the requested feature set");
+            return;
+        }

        let passed = results.iter().filter(|r| r.success).count();
        let failed = results.len() - passed;
@@ -203,6 +194,39 @@ impl ProtocolTestSuite {
    }
 }

+fn all_protocol_tests() -> Vec<TestDefinition> {
+    vec![
+        TestDefinition {
+            name: "test_ftps_core_operations",
+            required_feature: "ftps",
+        },
+        TestDefinition {
+            name: "test_webdav_core_operations",
+            required_feature: "webdav",
+        },
+        TestDefinition {
+            name: "test_sftp_core_operations",
+            required_feature: "sftp",
+        },
+        TestDefinition {
+            name: "test_sftp_compliance_suite",
+            required_feature: "sftp",
+        },
+        TestDefinition {
+            name: "test_sftp_compliance_readonly",
+            required_feature: "sftp",
+        },
+        TestDefinition {
+            name: "test_sftp_idle_timeout_disconnects",
+            required_feature: "sftp",
+        },
+        TestDefinition {
+            name: "test_sftp_compliance_standalone",
+            required_feature: "sftp",
+        },
+    ]
+}
+
 /// Test suite
 #[tokio::test]
 #[serial]
@@ -218,3 +242,72 @@ async fn test_protocol_core_suite() -> Result<(), Box<dyn std::error::Error + Se
    info!("All protocol tests passed");
    Ok(())
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    fn scheduled_names(suite: ProtocolTestSuite) -> Vec<&'static str> {
+        suite.tests.into_iter().map(|test| test.name).collect()
+    }
+
+    #[test]
+    fn schedules_all_protocol_tests_without_feature_filter() {
+        let names = scheduled_names(ProtocolTestSuite::with_requested_features(None));
+
+        assert_eq!(names.len(), 7);
+        assert!(names.contains(&"test_ftps_core_operations"));
+        assert!(names.contains(&"test_webdav_core_operations"));
+        assert!(names.contains(&"test_sftp_core_operations"));
+        assert!(names.contains(&"test_sftp_compliance_standalone"));
+    }
+
+    #[test]
+    fn schedules_only_requested_non_sftp_protocols() {
+        let names = scheduled_names(ProtocolTestSuite::with_requested_features(Some("ftps, webdav")));
+
+        assert_eq!(names, vec!["test_ftps_core_operations", "test_webdav_core_operations"]);
+    }
+
+    #[test]
+    fn schedules_all_sftp_entries_for_sftp_feature() {
+        let names = scheduled_names(ProtocolTestSuite::with_requested_features(Some("sftp")));
+
+        assert_eq!(
+            names,
+            vec![
+                "test_sftp_core_operations",
+                "test_sftp_compliance_suite",
+                "test_sftp_compliance_readonly",
+                "test_sftp_idle_timeout_disconnects",
+                "test_sftp_compliance_standalone",
+            ]
+        );
+    }
+
+    #[test]
+    fn feature_filter_is_case_insensitive() {
+        let names = scheduled_names(ProtocolTestSuite::with_requested_features(Some("SFTP")));
+
+        assert_eq!(names.len(), 5);
+        assert!(names.iter().all(|name| name.contains("sftp")));
+    }
+
+    #[test]
+    fn full_feature_schedules_all_protocol_tests() {
+        let names = scheduled_names(ProtocolTestSuite::with_requested_features(Some("full")));
+
+        assert_eq!(names.len(), 7);
+        assert!(names.contains(&"test_ftps_core_operations"));
+        assert!(names.contains(&"test_webdav_core_operations"));
+        assert!(names.contains(&"test_sftp_core_operations"));
+        assert!(names.contains(&"test_sftp_compliance_standalone"));
+    }
+
+    #[test]
+    fn schedules_no_tests_when_requested_features_have_no_protocols() {
+        let names = scheduled_names(ProtocolTestSuite::with_requested_features(Some("diagnostics")));
+
+        assert!(names.is_empty());
+    }
+}