zedc(fix): code coverage path mismatch, look for ZOWE_OPT_PASSWORD (#3697)

* fix(zedc): Coverage not always identified in multi-line diffs

Signed-off-by: Trae Yelovich <[email protected]>

* demo: add uncovered lines and some tests

Signed-off-by: Trae Yelovich <[email protected]>

* fix(zedc/status): censor user & pass in env. vars

Signed-off-by: Trae Yelovich <[email protected]>

* chore: zedc changelog

Signed-off-by: Trae Yelovich <[email protected]>

* chore: bump zedc version

Signed-off-by: Trae Yelovich <[email protected]>

* Revert "demo: add uncovered lines and some tests"

This reverts commit e78e4cd.

Signed-off-by: Trae Yelovich <[email protected]>

* chore: address changelog feedback

Signed-off-by: Trae Yelovich <[email protected]>

---------

Signed-off-by: Trae Yelovich <[email protected]>
Co-authored-by: Fernando Rijo Cedeno <[email protected]>

Author: traeok

zedc/CHANGELOG.md

@@ -1,4 +1,10 @@
-# TBD Release
+# 0.2.1
+
+- **BugFix:** Fixed an issue where the `zedc test coverage` command incorrectly reported lines as uncovered due to OS-specific path inconsistencies between `git diff` and the coverage report.
+- **BugFix:** Fixed an issue where the `zedc status` command did not censor the `ZOWE_OPT_USER` environment variable.
+- **BugFix:** Fixed an issue where the `zedc status` command looked for the `ZOWE_OPT_PASS` environment variable instead of the variable `ZOWE_OPT_PASSWORD` when printing environment status.
+
+# 0.2.0
 
 - **Enhancement:** Added `zedc status` command to display working tree status, diagnostics and system info in a developer's environment.
 - **Enhancement:** Added `zedc test coverage` command to show patch coverage and uncovered lines with hyperlinks (for [compatible terminals only](https://github.com/Alhadis/OSC8-Adoption))

zedc/Cargo.lock

@@ -1,6 +1,6 @@
 [package]
 name = "zedc"
-version = "0.2.0"
+version = "0.2.1"
 edition = "2021"
 
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

zedc/Cargo.toml

@@ -145,17 +145,18 @@ impl EnvironmentStatus {
             "VSCODE_EXTENSIONS",
             "ZOWE_CLI_HOME",
             "ZOWE_OPT_USER",
-            "ZOWE_OPT_PASS",
+            "ZOWE_OPT_PASSWORD",
         ];
 
         for var in relevant_vars {
             if let Ok(value) = std::env::var(var) {
                 // Mask sensitive values
-                let display_value = if var.contains("PASS") || var.contains("PAT") {
-                    "********".to_string()
-                } else {
-                    value
-                };
+                let display_value =
+                    if var.contains("USER") || var.contains("PASS") || var.contains("PAT") {
+                        "********".to_string()
+                    } else {
+                        value
+                    };
                 self.env_vars.push((var.to_string(), display_value));
             }
         }

zedc/src/status.rs

@@ -792,18 +792,20 @@ fn check_file_coverage(
     verbose: bool,
 ) {
     let changed_file_rel_path_from_repo_root = file_from_diff;
-    let paths_to_check_in_coverage_map =
-        generate_paths_to_check(changed_file_rel_path_from_repo_root, repo_root_pathbuf);
 
     if verbose {
         println!(
-            "Debug - Paths to check in coverage map for {}: {:?}",
-            changed_file_rel_path_from_repo_root, paths_to_check_in_coverage_map
+            "Debug - Attempting to find coverage for {}",
+            changed_file_rel_path_from_repo_root
         );
     }
 
-    let (file_coverage_data, matched_coverage_path) =
-        find_coverage_data(package_coverage_data, &paths_to_check_in_coverage_map);
+    let (file_coverage_data, matched_coverage_path) = find_coverage_data(
+        package_coverage_data,
+        changed_file_rel_path_from_repo_root,
+        repo_root_pathbuf,
+        verbose,
+    );
 
     match file_coverage_data {
         Some(file_cov) => {
@@ -830,6 +832,14 @@ fn check_file_coverage(
                     "Warning: No coverage data found for changed file {}. Lines from this file considered uncovered.",
                     changed_file_rel_path_from_repo_root
                 );
+                if let Some(map) = package_coverage_data.as_object() {
+                    let keys: Vec<&String> = map.keys().collect();
+                    println!("Debug - Coverage map top-level keys: {:?}", keys);
+                    if let Some(coverage_map) = map.get("coverageMap").and_then(|m| m.as_object()) {
+                        let nested_keys: Vec<&String> = coverage_map.keys().collect();
+                        println!("Debug - 'coverageMap' nested keys: {:?}", nested_keys);
+                    }
+                }
             }
             // All lines in this diff for this file are considered uncovered
             for line_num in lines_in_diff {
@@ -848,67 +858,46 @@ fn check_file_coverage(
     }
 }
 
-/// Generate all possible paths to check in coverage map
-fn generate_paths_to_check(path: &str, repo_root_pathbuf: &PathBuf) -> Vec<String> {
-    let mut paths_to_check = Vec::new();
-    let abs_path_from_repo_root = repo_root_pathbuf.join(path);
-
-    if let Some(abs_path_str) = abs_path_from_repo_root.to_str() {
-        paths_to_check.push(abs_path_str.to_string());
-        paths_to_check.push(abs_path_str.replace('\\', "/"));
-
-        #[cfg(windows)]
-        if abs_path_str.len() > 1 && abs_path_str.chars().nth(1) == Some(':') {
-            if let Some(first_char) = abs_path_str.chars().next() {
-                if first_char.is_ascii_uppercase() {
-                    let mut lower_drive_abs_path_chars = abs_path_str.chars();
-                    let drive_letter_lower = lower_drive_abs_path_chars
-                        .next()
-                        .unwrap()
-                        .to_lowercase()
-                        .to_string();
-                    let rest_of_path: String = lower_drive_abs_path_chars.collect();
-                    let path_with_lower_drive = format!("{}{}", drive_letter_lower, rest_of_path);
-
-                    paths_to_check.push(path_with_lower_drive.clone());
-                    paths_to_check.push(path_with_lower_drive.replace('\\', "/"));
-                }
-            }
-        }
-    }
-
-    paths_to_check
-}
-
 /// Find the coverage data for a file
 fn find_coverage_data<'a>(
     package_coverage_data: &'a serde_json::Value,
-    paths_to_check: &[String],
+    changed_file_rel_path: &str,
+    repo_root: &PathBuf,
+    verbose: bool,
 ) -> (Option<&'a serde_json::Value>, String) {
-    let mut file_coverage_data = None;
-    let mut matched_coverage_path = String::new();
-
-    if let Some(coverage_map_obj) = package_coverage_data.as_object() {
-        // Istanbul often nests coverageMap
-        for path_to_check in paths_to_check {
-            if let Some(data) = coverage_map_obj.get(path_to_check) {
-                file_coverage_data = Some(data);
-                matched_coverage_path = path_to_check.clone();
-                break;
-            }
-        }
-    } else {
-        // Fallback if the top-level is the coverage map (less common for Istanbul)
-        for path_to_check in paths_to_check {
-            if let Some(data) = package_coverage_data.get(path_to_check) {
-                file_coverage_data = Some(data);
-                matched_coverage_path = path_to_check.clone();
-                break;
+    let coverage_map = package_coverage_data
+        .get("coverageMap")
+        .and_then(|v| v.as_object())
+        .or_else(|| package_coverage_data.as_object());
+
+    // Construct a normalized, absolute path for the file from the git diff.
+    let mut changed_file_abs_path = repo_root.clone();
+    for component in changed_file_rel_path.split('/') {
+        changed_file_abs_path.push(component);
+    }
+
+    if let Some(map) = coverage_map {
+        for file_cov_data in map.values() {
+            if let Some(path_from_report_str) = file_cov_data.get("path").and_then(|p| p.as_str()) {
+                let report_path = PathBuf::from(path_from_report_str);
+
+                // Compare the path from the report with the one we constructed.
+                // PathBuf's equality handles OS-specific details.
+                if report_path == changed_file_abs_path {
+                    return (Some(file_cov_data), path_from_report_str.to_string());
+                }
             }
         }
     }
 
-    (file_coverage_data, matched_coverage_path)
+    if verbose {
+        println!(
+            "Debug - Failed to find coverage. Constructed path for comparison: {:?}",
+            changed_file_abs_path
+        );
+    }
+
+    (None, String::new())
 }
 
 /// Process statements in a file's coverage data
@@ -923,78 +912,105 @@ fn process_file_statements(
 ) {
     let statement_map = file_cov.get("statementMap").and_then(|sm| sm.as_object());
     let s_map = file_cov.get("s").and_then(|s| s.as_object());
+    let branch_map = file_cov.get("branchMap").and_then(|bm| bm.as_object());
+    let b_map = file_cov.get("b").and_then(|b| b.as_object());
+
+    let mut executable_lines = std::collections::HashSet::new();
+    if let Some(stmt_map) = statement_map {
+        for stmt_data in stmt_map.values() {
+            if let (Some(start_line), Some(end_line)) = (
+                stmt_data
+                    .get("start")
+                    .and_then(|l| l.get("line"))
+                    .and_then(|l| l.as_u64()),
+                stmt_data
+                    .get("end")
+                    .and_then(|l| l.get("line"))
+                    .and_then(|l| l.as_u64()),
+            ) {
+                for line in start_line..=end_line {
+                    executable_lines.insert(line as usize);
+                }
+            }
+        }
+    }
+
+    let mut uncovered_lines = std::collections::HashSet::new();
 
     if let (Some(stmt_map), Some(s)) = (statement_map, s_map) {
-        for &line_num in lines_in_diff {
-            let mut line_covered = false;
-
-            for (stmt_idx, stmt_data) in stmt_map {
-                // Check start line
-                if let Some(loc) = stmt_data.get("start").and_then(|loc| loc.get("line")) {
-                    if loc.as_u64() == Some(line_num as u64) {
-                        // Check if this statement index is covered in 's'
-                        if let Some(count) = s.get(stmt_idx).and_then(|c| c.as_u64()) {
-                            if count > 0 {
-                                line_covered = true;
-                                if verbose {
-                                    println!(
-                                        "Debug - Line {} in {} is COVERED (statement {})",
-                                        line_num, changed_file_rel_path, stmt_idx
-                                    );
-                                }
-                                break; // Line is covered by this statement
-                            }
+        for (stmt_idx, stmt_data) in stmt_map {
+            if let Some(count) = s.get(stmt_idx).and_then(|c| c.as_u64()) {
+                if count == 0 {
+                    if let (Some(start_line), Some(end_line)) = (
+                        stmt_data
+                            .get("start")
+                            .and_then(|l| l.get("line"))
+                            .and_then(|l| l.as_u64()),
+                        stmt_data
+                            .get("end")
+                            .and_then(|l| l.get("line"))
+                            .and_then(|l| l.as_u64()),
+                    ) {
+                        for line in start_line..=end_line {
+                            uncovered_lines.insert(line as usize);
                         }
                     }
                 }
+            }
+        }
+    }
 
-                // Also check "end" line for multi-line statements
-                if let Some(loc) = stmt_data.get("end").and_then(|loc| loc.get("line")) {
-                    if loc.as_u64() == Some(line_num as u64) {
-                        if let Some(count) = s.get(stmt_idx).and_then(|c| c.as_u64()) {
-                            if count > 0 {
-                                line_covered = true;
-                                if verbose {
-                                    println!(
-                                        "Debug - Line {} in {} is COVERED (by end of statement {})",
-                                        line_num, changed_file_rel_path, stmt_idx
-                                    );
-                                }
-                                break;
-                            }
-                        }
+    if let (Some(br_map), Some(b)) = (branch_map, b_map) {
+        for (branch_idx, branch_data) in br_map {
+            // Check if branch is intentionally skipped
+            if branch_data.get("skip").and_then(|s| s.as_bool()) == Some(true) {
+                continue;
+            }
+
+            if let Some(branch_counts) = b.get(branch_idx).and_then(|bc| bc.as_array()) {
+                // If any branch path has a count of 0, the branch is not fully covered.
+                if branch_counts.iter().any(|count| count.as_u64() == Some(0)) {
+                    // The line where the branch is defined should be marked as uncovered.
+                    if let Some(line) = branch_data.get("line").and_then(|l| l.as_u64()) {
+                        uncovered_lines.insert(line as usize);
                     }
                 }
             }
+        }
+    }
 
-            if line_covered {
-                *covered_lines_in_patch += 1;
-            } else {
+    for &line_num in lines_in_diff {
+        let is_executable = executable_lines.contains(&line_num);
+        let is_uncovered = uncovered_lines.contains(&line_num);
+
+        if is_executable {
+            if is_uncovered {
                 if verbose {
                     println!(
-                        "Debug - Line {} in {} is NOT COVERED",
+                        "Debug - Line {} in {} is NOT COVERED (uncovered executable code)",
                         line_num, changed_file_rel_path
                     );
                 }
                 uncovered_lines_details
                     .entry(file_from_diff.to_string())
                     .or_default()
                     .push(line_num);
+            } else {
+                if verbose {
+                    println!(
+                        "Debug - Line {} in {} is COVERED",
+                        line_num, changed_file_rel_path
+                    );
+                }
+                *covered_lines_in_patch += 1;
             }
-        }
-    } else {
-        if verbose {
+        } else if verbose && is_uncovered {
+            // This is for verbose logging of non-executable but uncovered lines like 'else {'
             println!(
-                "Debug - Missing 'statementMap' or 's'. All lines for this file in diff considered uncovered."
+                "Debug - Line {} in {} is part of an uncovered block, but not executable. Ignoring.",
+                line_num, changed_file_rel_path
             );
         }
-        // All lines for this file in the diff are considered uncovered if maps are missing
-        for &line_num in lines_in_diff {
-            uncovered_lines_details
-                .entry(file_from_diff.to_string())
-                .or_default()
-                .push(line_num);
-        }
     }
 }