ci: enforce test presence on new/modified filter modules
Add check-test-presence.sh script and CI job that fails if any *_cmd.rs file added or modified in a PR is missing a #[cfg(test)] block. - New CI job runs with no dependencies (parallel to all other jobs, <10s) - Uses --diff-filter=AM to catch both added files and test deletions - --self-test mode for local verification - Add missing tests to wget_cmd.rs (17 tests) and env_cmd.rs (12 tests) covering pure functions: compact_url, format_size, parse_error, extract_filename, mask_value, is_lang_var, is_cloud_var, etc. Fixes the enforcement gap: CONTRIBUTING.md required tests but CI did not check. Now 34/34 *_cmd.rs modules have #[cfg(test)]. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Florian BRUNIAUX <florian@bruniaux.com>
This commit is contained in:
@@ -14,6 +14,18 @@ env:
|
||||
jobs:
|
||||
# ─── Fast gates (fail early, save CI minutes) ───
|
||||
|
||||
check-test-presence:
|
||||
name: test presence
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
with:
|
||||
fetch-depth: 50
|
||||
- name: Check filter modules have tests
|
||||
run: |
|
||||
git fetch origin "${{ github.base_ref }}" --depth=1 || true
|
||||
bash scripts/check-test-presence.sh "origin/${{ github.base_ref }}"
|
||||
|
||||
fmt:
|
||||
name: fmt
|
||||
runs-on: ubuntu-latest
|
||||
|
||||
Executable
+69
@@ -0,0 +1,69 @@
|
||||
#!/usr/bin/env bash
|
||||
set -euo pipefail
|
||||
|
||||
# check-test-presence.sh — CI guard: new/modified *_cmd.rs files must have #[cfg(test)]
|
||||
#
|
||||
# Usage:
|
||||
# bash scripts/check-test-presence.sh [BASE_BRANCH]
|
||||
# bash scripts/check-test-presence.sh --self-test
|
||||
#
|
||||
# BASE_BRANCH defaults to origin/develop
|
||||
|
||||
if [ "${1:-}" = "--self-test" ]; then
|
||||
# Self-test: create a tempfile without tests and verify the check catches it
|
||||
TMPFILE="src/cmds/system/_rtk_check_self_test_cmd.rs"
|
||||
echo "pub fn run() {}" > "$TMPFILE"
|
||||
trap 'rm -f "$TMPFILE"' EXIT
|
||||
|
||||
if grep -q '#\[cfg(test)\]' "$TMPFILE"; then
|
||||
echo "FAIL: self-test broken (false negative)"
|
||||
exit 1
|
||||
fi
|
||||
rm "$TMPFILE"
|
||||
trap - EXIT
|
||||
echo "PASS: --self-test detection works correctly"
|
||||
exit 0
|
||||
fi
|
||||
|
||||
BASE_BRANCH="${1:-origin/develop}"
|
||||
EXIT_CODE=0
|
||||
|
||||
# Find *_cmd.rs files that were added or modified in this PR
|
||||
CHANGED_FILES=$(git diff --name-only --diff-filter=AM --no-renames "$BASE_BRANCH"...HEAD \
|
||||
2>/dev/null | grep -E 'src/cmds/.+_cmd\.rs$' || true)
|
||||
|
||||
if [ -z "$CHANGED_FILES" ]; then
|
||||
echo "check-test-presence: no *_cmd.rs changes detected — OK"
|
||||
exit 0
|
||||
fi
|
||||
|
||||
echo "check-test-presence: checking $(echo "$CHANGED_FILES" | wc -l | tr -d ' ') filter module(s)..."
|
||||
echo ""
|
||||
|
||||
while IFS= read -r file; do
|
||||
if [ ! -f "$file" ]; then
|
||||
continue
|
||||
fi
|
||||
|
||||
if grep -q '#\[cfg(test)\]' "$file"; then
|
||||
echo " PASS $file"
|
||||
else
|
||||
echo " FAIL $file"
|
||||
echo " Missing #[cfg(test)] module."
|
||||
echo " Every *_cmd.rs filter must include inline unit tests."
|
||||
echo " Reference: src/cmds/cloud/aws_cmd.rs"
|
||||
echo ""
|
||||
EXIT_CODE=1
|
||||
fi
|
||||
done <<< "$CHANGED_FILES"
|
||||
|
||||
echo ""
|
||||
|
||||
if [ "$EXIT_CODE" -ne 0 ]; then
|
||||
echo "check-test-presence: FAILED — add tests before merging."
|
||||
echo "See .claude/rules/cli-testing.md for the testing guide."
|
||||
else
|
||||
echo "check-test-presence: all filter modules have tests — OK"
|
||||
fi
|
||||
|
||||
exit "$EXIT_CODE"
|
||||
@@ -261,3 +261,114 @@ fn truncate_line(line: &str, max: usize) -> String {
|
||||
format!("{}...", t)
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn test_compact_url_strips_protocol() {
|
||||
assert_eq!(compact_url("https://example.com/file.zip"), "example.com/file.zip");
|
||||
assert_eq!(compact_url("http://example.com/file.zip"), "example.com/file.zip");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_compact_url_truncates_long_url() {
|
||||
let long = "https://example.com/very/long/path/that/exceeds/fifty/characters/file.zip";
|
||||
let result = compact_url(long);
|
||||
assert!(result.contains("..."), "Long URL should be truncated with ...");
|
||||
assert!(result.len() < long.len());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_compact_url_short_unchanged() {
|
||||
let short = "https://x.com/f";
|
||||
assert_eq!(compact_url(short), "x.com/f");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_format_size_zero() {
|
||||
assert_eq!(format_size(0), "?");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_format_size_bytes() {
|
||||
assert_eq!(format_size(512), "512B");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_format_size_kilobytes() {
|
||||
let result = format_size(2048);
|
||||
assert!(result.ends_with("KB"), "Expected KB, got {}", result);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_format_size_megabytes() {
|
||||
let result = format_size(2 * 1024 * 1024);
|
||||
assert!(result.ends_with("MB"), "Expected MB, got {}", result);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_parse_error_404() {
|
||||
assert_eq!(parse_error("HTTP request failed: 404", ""), "404 Not Found");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_parse_error_dns() {
|
||||
assert_eq!(
|
||||
parse_error("unable to resolve host example.com", ""),
|
||||
"DNS lookup failed"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_parse_error_ssl() {
|
||||
assert_eq!(
|
||||
parse_error("SSL certificate verification failed", ""),
|
||||
"SSL/TLS error"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_parse_error_unknown() {
|
||||
assert_eq!(parse_error("", ""), "Unknown error");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_truncate_line_short() {
|
||||
assert_eq!(truncate_line("hello", 10), "hello");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_truncate_line_exact() {
|
||||
assert_eq!(truncate_line("hello", 5), "hello");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_truncate_line_long() {
|
||||
let result = truncate_line("hello world this is long", 10);
|
||||
assert!(result.ends_with("..."));
|
||||
assert!(result.len() <= 10);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_extract_filename_from_output_flag() {
|
||||
let args = vec!["-O".to_string(), "myfile.zip".to_string()];
|
||||
assert_eq!(
|
||||
extract_filename_from_output("", "https://example.com/x", &args),
|
||||
"myfile.zip"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_extract_filename_from_url_fallback() {
|
||||
let result = extract_filename_from_output("", "https://example.com/file.tar.gz", &[]);
|
||||
assert_eq!(result, "file.tar.gz");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_extract_filename_empty_url_fallback() {
|
||||
let result = extract_filename_from_output("", "https://example.com/", &[]);
|
||||
assert_eq!(result, "index.html");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -204,3 +204,95 @@ fn is_interesting_var(key: &str) -> bool {
|
||||
let patterns = ["HOME", "USER", "LANG", "LC_", "TZ", "PWD", "OLDPWD"];
|
||||
patterns.iter().any(|p| key.to_uppercase().starts_with(p))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn test_mask_value_short() {
|
||||
assert_eq!(mask_value("abc"), "****");
|
||||
assert_eq!(mask_value(""), "****");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_mask_value_long() {
|
||||
let result = mask_value("supersecrettoken");
|
||||
assert!(result.contains("****"), "Masked value should contain ****");
|
||||
assert!(result.starts_with("su"), "Should preserve 2-char prefix");
|
||||
assert!(result.ends_with("en"), "Should preserve 2-char suffix");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_mask_value_exactly_four() {
|
||||
assert_eq!(mask_value("abcd"), "****");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_mask_value_five_chars() {
|
||||
let result = mask_value("abcde");
|
||||
assert!(result.starts_with("ab"));
|
||||
assert!(result.ends_with("de"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_is_lang_var_rust() {
|
||||
assert!(is_lang_var("RUST_LOG"));
|
||||
assert!(is_lang_var("CARGO_HOME"));
|
||||
assert!(is_lang_var("GOPATH"));
|
||||
assert!(is_lang_var("NODE_ENV"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_is_lang_var_negative() {
|
||||
assert!(!is_lang_var("HOME"));
|
||||
assert!(!is_lang_var("PATH"));
|
||||
assert!(!is_lang_var("USER"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_is_cloud_var() {
|
||||
assert!(is_cloud_var("AWS_ACCESS_KEY_ID"));
|
||||
assert!(is_cloud_var("AZURE_CLIENT_ID"));
|
||||
assert!(is_cloud_var("DOCKER_HOST"));
|
||||
assert!(is_cloud_var("KUBERNETES_SERVICE_HOST"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_is_cloud_var_negative() {
|
||||
assert!(!is_cloud_var("HOME"));
|
||||
assert!(!is_cloud_var("RUST_LOG"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_is_tool_var() {
|
||||
assert!(is_tool_var("EDITOR"));
|
||||
assert!(is_tool_var("GIT_AUTHOR_NAME"));
|
||||
assert!(is_tool_var("SSH_AUTH_SOCK"));
|
||||
assert!(is_tool_var("CLAUDE_API_KEY"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_is_interesting_var() {
|
||||
assert!(is_interesting_var("HOME"));
|
||||
assert!(is_interesting_var("USER"));
|
||||
assert!(is_interesting_var("LANG"));
|
||||
assert!(is_interesting_var("TZ"));
|
||||
assert!(is_interesting_var("PWD"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_is_interesting_var_negative() {
|
||||
assert!(!is_interesting_var("RANDOM_VAR"));
|
||||
assert!(!is_interesting_var("MY_CUSTOM_VAR"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_sensitive_patterns_contains_keys() {
|
||||
let patterns = get_sensitive_patterns();
|
||||
assert!(patterns.contains("key"));
|
||||
assert!(patterns.contains("secret"));
|
||||
assert!(patterns.contains("password"));
|
||||
assert!(patterns.contains("token"));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user