Skip to content

ci: refactor test_helper.sh #4209

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.

Sign up for GitHub

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

Merged
vvbandeira merged 1 commit into from
May 5, 2026
+86 −30
Merged

ci: refactor test_helper.sh #4209

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
116 changes: 86 additions & 30 deletions flow/test/test_helper.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,33 +4,93 @@ set -eoux pipefail

cd "$(dirname "$(readlink -f "$0")")/../"

# Setting args (and setting default values for testing)
DESIGN_NAME=${1:-gcd}
PLATFORM=${2:-nangate45}
CONFIG_MK=${3:-config.mk}
if [ $# -ge 4 ]; then
FLOW_VARIANT=$4
fi
TARGET=${5:-'finish metadata'}
DESIGN_CONFIG=./designs/$PLATFORM/$DESIGN_NAME/$CONFIG_MK
if [ -z "${WORK_HOME+x}" ]; then
WORK_HOME=.
usage() {
cat <<'EOF'
Usage:
test_helper.sh [options]
test_helper.sh <design> <platform> [config_mk] [variant] [target] (legacy positional)

Options:
--design NAME Design name (default: gcd)
--platform NAME Platform (default: nangate45)
--config FILE Design config file name (default: config.mk)
--design-path DIR Root path containing 'designs/' (default: ./)
--variant NAME Flow variant (default: unset)
--target STR Make target(s), space-separated (default: 'finish metadata')
--work-home DIR Work home (default: .)
--private-dir DIR Private tool scripts dir (default: ../../private_tool_scripts)
--save-to-db Save metrics to DB (requires private.mk)
--run-calibre Run Calibre DRC (requires utils.mk)
--check-drc-db Check DRC DB (use with --run-calibre + --save-to-db)
--make-issue Run final_report_issue at end
-h, --help Show this help

Environment variables override defaults; flags override env. Flags also accept
values via the corresponding env var (e.g., DESIGN_NAME, PLATFORM, CONFIG_MK,
FLOW_VARIANT, TARGET, WORK_HOME, PRIVATE_DIR, DESIGN_PATH, SAVE_TO_DB,
RUN_CALIBRE, CHECK_DRC_DB, MAKE_ISSUE).
EOF
}

DESIGN_NAME="${DESIGN_NAME:-gcd}"
PLATFORM="${PLATFORM:-nangate45}"
CONFIG_MK="${CONFIG_MK:-config.mk}"
TARGET="${TARGET:-finish metadata}"
WORK_HOME="${WORK_HOME:-.}"
PRIVATE_DIR="${PRIVATE_DIR:-../../private_tool_scripts}"
DESIGN_PATH="${DESIGN_PATH:-./}"

if [ $# -gt 0 ]; then
if [ "${1#-}" = "$1" ]; then
# Legacy positional: <design> <platform> [config_mk] [variant] [target]
DESIGN_NAME=${1:-$DESIGN_NAME}
PLATFORM=${2:-$PLATFORM}
CONFIG_MK=${3:-$CONFIG_MK}
if [ $# -ge 4 ] && [ -n "$4" ]; then
FLOW_VARIANT=$4
fi
if [ $# -ge 5 ] && [ -n "$5" ]; then
TARGET=$5
fi
else
while [ $# -gt 0 ]; do
case "$1" in
--design) DESIGN_NAME=$2; shift 2 ;;
--platform) PLATFORM=$2; shift 2 ;;
--config) CONFIG_MK=$2; shift 2 ;;
--design-path) DESIGN_PATH=$2; shift 2 ;;
--variant) FLOW_VARIANT=$2; shift 2 ;;
--target) TARGET=$2; shift 2 ;;
--work-home) WORK_HOME=$2; shift 2 ;;
--private-dir) PRIVATE_DIR=$2; shift 2 ;;
Comment on lines +58 to +65
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The current flag parsing logic uses shift 2 for options that require an argument (e.g., --design, --platform). If an option is provided as the last argument without a value, shift 2 will fail because the shift count exceeds the number of remaining arguments. Since set -e is active, this will cause the script to crash with a shift count out of range error. Additionally, if another flag follows an option that expects a value (e.g., --design --platform), the second flag will be incorrectly assigned as the value for the first option. Consider adding a check to ensure an argument is present before shifting.

--save-to-db) SAVE_TO_DB=1; shift ;;
--run-calibre) RUN_CALIBRE=1; shift ;;
--check-drc-db) CHECK_DRC_DB=1; shift ;;
--make-issue) MAKE_ISSUE=1; shift ;;
-h|--help) usage; exit 0 ;;
*) echo "Unknown option: $1" >&2; usage >&2; exit 2 ;;
esac
done
fi
fi

DESIGN_CONFIG=${DESIGN_PATH%/}/designs/$PLATFORM/$DESIGN_NAME/$CONFIG_MK
LOG_FILE=${WORK_HOME}/logs/$PLATFORM/$DESIGN_NAME.log
mkdir -p "${WORK_HOME}/logs/$PLATFORM"

__make="make DESIGN_CONFIG=$DESIGN_CONFIG"
__make=(make "DESIGN_CONFIG=$DESIGN_CONFIG")
if [ -n "${FLOW_VARIANT+x}" ]; then
__make+=" FLOW_VARIANT=$FLOW_VARIANT"
__make+=("FLOW_VARIANT=$FLOW_VARIANT")
fi

mkdir -p "$(dirname "$LOG_FILE")"
$__make clean_all clean_metadata 2>&1 | tee "$LOG_FILE"
"${__make[@]}" clean_all clean_metadata 2>&1 | tee "$LOG_FILE"

# turn off abort on error so we can always capture the result
set +e

eval $__make "${TARGET}" 2>&1 | tee -a "$LOG_FILE"
read -r -a TARGET_ARR <<< "$TARGET"
"${__make[@]}" "${TARGET_ARR[@]}" 2>&1 | tee -a "$LOG_FILE"

# Save the return code to return as the overall status after we package
# the results
Expand All @@ -40,26 +100,22 @@ if [ "${TARGET}" != "finish metadata" ]; then
exit $ret
fi

if [ -z "${PRIVATE_DIR+x}" ]; then
PRIVATE_DIR="../../private_tool_scripts"
fi

if [ -f "$PRIVATE_DIR/openRoad/private.mk" ] && [ -n "${SAVE_TO_DB+x}" ]; then
$__make save_to_metrics_db
"${__make[@]}" save_to_metrics_db
ret=$(( ret + $? ))
fi

if [ -f "$PRIVATE_DIR/util/utils.mk" ] && [ -n "${RUN_CALIBRE+x}" ]; then
$__make calibre_drc
"${__make[@]}" calibre_drc
ret=$(( ret + $? ))
$__make convert_calibre
"${__make[@]}" convert_calibre
ret=$(( ret + $? ))
if [ -n "${SAVE_TO_DB+x}" ]; then
$__make save_to_drc_db
"${__make[@]}" save_to_drc_db
ret=$(( ret + $? ))
fi
if [ -n "${CHECK_DRC_DB+x}" ]; then
$__make check_drc_db
"${__make[@]}" check_drc_db
ret=$(( ret + $? ))
fi
fi
Expand All @@ -68,20 +124,20 @@ fi
set -e

if [ -n "${MAKE_ISSUE+x}" ]; then
$__make final_report_issue 2>&1 | tee -a "$LOG_FILE"
"${__make[@]}" final_report_issue 2>&1 | tee -a "$LOG_FILE"
fi

# Find make targets
set +x # These rules are noisy
TARGETS=$($__make -np | grep -e '^[^ ]*:')
if [ $ret -eq 0 ] && grep -q 'simulate:' <(echo $TARGETS); then
TARGETS=$("${__make[@]}" -np | grep -e '^[^ ]*:')
if [ $ret -eq 0 ] && grep -q 'simulate:' <(echo "$TARGETS"); then
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

For consistency with the use of here-strings elsewhere in the script (e.g., line 92), you can use <<< "$TARGETS" instead of <(echo "$TARGETS"). This is more idiomatic in Bash and avoids spawning a subshell for echo.

Suggested change
if [ $ret -eq 0 ] && grep -q 'simulate:' <(echo "$TARGETS"); then
if [ $ret -eq 0 ] && grep -q 'simulate:' <<< "$TARGETS"; then

echo "Start simulate"
$__make simulate 2>&1 | tee -a "$LOG_FILE"
"${__make[@]}" simulate 2>&1 | tee -a "$LOG_FILE"
ret=$(( ret + $? ))
fi
if [ $ret -eq 0 ] && grep -q 'power:' <(echo $TARGETS); then
if [ $ret -eq 0 ] && grep -q 'power:' <(echo "$TARGETS"); then
echo "Start power"
$__make power 2>&1 | tee -a "$LOG_FILE"
"${__make[@]}" power 2>&1 | tee -a "$LOG_FILE"
ret=$(( ret + $? ))
fi
set -x
Expand Down