-
Notifications
You must be signed in to change notification settings - Fork 43
Update URM Test Runner #568
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -71,13 +71,15 @@ if command -v flock >/dev/null 2>&1; then | |
| echo "$TESTNAME SKIP" >"$RES_FILE" | ||
| exit 0 | ||
| fi | ||
| lock_flock=1 | ||
| trap 'exec 9>&-' EXIT INT TERM | ||
| else | ||
| if ! mkdir "$LOCKDIR" 2>/dev/null; then | ||
| log_warn "Another ${TESTNAME} run is active or stale fallback lockdir exists: $LOCKDIR" | ||
| echo "$TESTNAME SKIP" >"$RES_FILE" | ||
| exit 0 | ||
| fi | ||
| lock_flock=0 | ||
| trap 'rmdir "$LOCKDIR" 2>/dev/null || true' EXIT INT TERM | ||
| fi | ||
|
|
||
|
|
@@ -275,10 +277,50 @@ else | |
| fi | ||
|
|
||
| # ---------- Config preflight (check both common/ and tests/) ---------- | ||
| URM_CONFIG_DIR="${URM_CONFIG_DIR:-/etc/urm}" | ||
| COMMON_CONFIGS_DIR="$URM_CONFIG_DIR/common" | ||
| TEST_CONFIGS_DIR="$URM_CONFIG_DIR/tests/configs" | ||
| TEST_NODES_DIR="$URM_CONFIG_DIR/tests/nodes" | ||
| # Resolution order for each root (first match wins): | ||
| # 1. Explicit per-root override (URM_COMMON_CONFIG_DIR / URM_TESTS_CONFIG_DIR) | ||
| # 2. Legacy URM_CONFIG_DIR (compatibility fallback for existing callers) | ||
| # 3. Built-in default | ||
| # | ||
| # For test-nodes the candidate order is: | ||
| # 1. Explicit URM_TEST_NODES_DIR override | ||
| # 2. Legacy URM_CONFIG_DIR (compatibility fallback for existing callers) | ||
| # 3. /var/lib/urm — runtime/writable location, only when non-empty | ||
| # 4. /usr/share/urm — package-installed location (default) | ||
|
|
||
| # Resolve common-config root | ||
| if [ -n "${URM_COMMON_CONFIG_DIR:-}" ]; then | ||
| common_root="$URM_COMMON_CONFIG_DIR" | ||
| elif [ -n "${URM_CONFIG_DIR:-}" ]; then | ||
| common_root="$URM_CONFIG_DIR" | ||
| else | ||
| common_root="/etc/urm" | ||
| fi | ||
|
|
||
| # Resolve tests-config root | ||
| if [ -n "${URM_TESTS_CONFIG_DIR:-}" ]; then | ||
| tests_root="$URM_TESTS_CONFIG_DIR" | ||
| elif [ -n "${URM_CONFIG_DIR:-}" ]; then | ||
| tests_root="$URM_CONFIG_DIR" | ||
| else | ||
| tests_root="/usr/share/urm" | ||
| fi | ||
|
|
||
| # Resolve test-nodes root: explicit override > legacy URM_CONFIG_DIR > populated runtime dir > package dir | ||
| if [ -n "${URM_TEST_NODES_DIR:-}" ]; then | ||
| nodes_root="$URM_TEST_NODES_DIR" | ||
| elif [ -n "${URM_CONFIG_DIR:-}" ]; then | ||
| nodes_root="$URM_CONFIG_DIR" | ||
| elif [ -d "/var/lib/urm/tests/nodes" ] && \ | ||
| [ "$(find "/var/lib/urm/tests/nodes" -mindepth 1 -maxdepth 1 -type f 2>/dev/null | wc -l | awk '{print $1}')" -gt 0 ]; then | ||
| nodes_root="/var/lib/urm" | ||
| else | ||
| nodes_root="/usr/share/urm" | ||
| fi | ||
|
|
||
| COMMON_CONFIGS_DIR="$common_root/common" | ||
| TEST_CONFIGS_DIR="$tests_root/tests/configs" | ||
| TEST_NODES_DIR="$nodes_root/tests/nodes" | ||
|
|
||
| COMMON_CONFIGS_OK=1 | ||
| TEST_CONFIGS_OK=1 | ||
|
|
@@ -397,6 +439,30 @@ if [ -z "$TESTS" ]; then | |
| exit 0 | ||
| fi | ||
|
|
||
| # ---------- Stage test nodes into the hardcoded runtime path ---------- | ||
| # UrmComponentTests and UrmIntegrationTests read nodes from the fixed path | ||
| # /run/urm/tests/nodes (hardcoded in the binaries). The package installs | ||
| # read-only node files under /usr/share/urm/tests/nodes (or /var/lib/urm), | ||
| # so we copy them to the writable runtime location before running the tests. | ||
| # Cleanup removes only the directory we created, and is registered with trap | ||
| # immediately so it runs on exit, interrupt, or termination. | ||
| RUNTIME_NODES_DIR="/run/urm/tests/nodes" | ||
| if [ "$TEST_NODES_OK" -eq 1 ]; then | ||
| mkdir -p "$RUNTIME_NODES_DIR" | ||
| if cp -r "$TEST_NODES_DIR/"* "$RUNTIME_NODES_DIR/"; then | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The copy occurs before the cleanup trap is installed and its destination is not cleared or made atomic. If cp partially copies and then fails, TEST_NODES_OK=0 skips the suites but leaves a partial /run/urm/tests/nodes; a later run can merge with and execute against that stale partial set. Register a cleanup handler immediately after establishing owned staging state, and stage atomically, for example, copy to a private temporary directory, validate it, then publish it only when the fixed runtime path is safely owned. |
||
| log_info "[NODES] Staged test nodes from $TEST_NODES_DIR to $RUNTIME_NODES_DIR" | ||
| # Extend the existing trap to also clean up the staged nodes. | ||
| if [ "$lock_flock" -eq 1 ]; then | ||
| trap 'rm -rf "$RUNTIME_NODES_DIR"; exec 9>&-' EXIT INT TERM | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The trap uses rm -rf "$RUNTIME_NODES_DIR" for every successful staging run, including when mkdir -p reused a pre-existing directory. This contradicts the comment that cleanup “removes only the directory we created.” Track ownership explicitly and fail/skip rather than deleting a path that was already present; otherwise an interrupted or concurrent system component can lose its runtime nodes. |
||
| else | ||
| trap 'rm -rf "$RUNTIME_NODES_DIR"; rmdir "$LOCKDIR" 2>/dev/null || true' EXIT INT TERM | ||
| fi | ||
| else | ||
| log_warn "[NODES] Failed to stage test nodes into $RUNTIME_NODES_DIR — suites requiring nodes will SKIP" | ||
| TEST_NODES_OK=0 | ||
| fi | ||
| fi | ||
|
|
||
| # ---------- Execute ---------- | ||
| PASS=0 | ||
| FAIL=0 | ||
|
|
@@ -508,7 +574,7 @@ log_info "Overall counts: PASS=$PASS FAIL=$FAIL SKIP=$SKIP" | |
| # - Else -> overall SKIP (everything skipped) | ||
| if [ "$FAIL" -gt 0 ]; then | ||
| echo "$TESTNAME FAIL" >"$RES_FILE" | ||
| exit 1 | ||
| exit 1 | ||
| fi | ||
| if [ "$PASS" -gt 0 ]; then | ||
| echo "$TESTNAME PASS" >"$RES_FILE" | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mkdir -p accepts an existing /run/urm/tests/nodes, but this test does not establish that it created or exclusively owns that directory. A previous interrupted run, another service, or a manual diagnostic can leave nodes there; the subsequent copy merges its files with stale content and the exit trap later deletes the entire directory.
That can produce invalid test inputs or remove state outside this run. Require an absent/test-owned destination before staging, or snapshot and restore pre-existing state. The cleanup must remove only assets this invocation created.