Merge pull request 'chore: prettify cleanup action logs' (#6) from feat/ai_code_review into develop
Reviewed-on: #6
This commit is contained in:
@@ -26,6 +26,11 @@
|
|||||||
"title": "log function definition unclear",
|
"title": "log function definition unclear",
|
||||||
"reason": "腳本中廣泛使用了 `log` 函式,但此 diff 並未包含 `log` 的定義,表示 `log` 可能來自基礎映像或另一個未包含的腳本。建議:在 entrypoint.sh 明確定義 `log` 或在文件/腳本中註明其來源,提升可維護性與自包含性。"
|
"reason": "腳本中廣泛使用了 `log` 函式,但此 diff 並未包含 `log` 的定義,表示 `log` 可能來自基礎映像或另一個未包含的腳本。建議:在 entrypoint.sh 明確定義 `log` 或在文件/腳本中註明其來源,提升可維護性與自包含性。"
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
"location": "tests/entrypoint.sh",
|
||||||
|
"title": "jq mock tightly coupled to entrypoint.sh expressions",
|
||||||
|
"reason": "測試中的 `jq` 模擬實作與 `entrypoint.sh` 中使用的特定 `jq` 表達式緊密耦合。若 `entrypoint.sh` 中的 jq 邏輯變更,Mock 也必須同步更新。這可能增加測試維護成本。建議:使用更通用的 jq 模擬方式,或在測試中直接使用真正的 `jq` 工具(若測試環境允許),以降低 Mock 與實際行為不同步的風險。"
|
||||||
|
},
|
||||||
{
|
{
|
||||||
"location": "entrypoint.sh:46",
|
"location": "entrypoint.sh:46",
|
||||||
"title": "resolve_token empty-token test suggestion",
|
"title": "resolve_token empty-token test suggestion",
|
||||||
|
|||||||
@@ -1,72 +1 @@
|
|||||||
[
|
[]
|
||||||
{
|
|
||||||
"level": "critical",
|
|
||||||
"role": "Aria",
|
|
||||||
"location": "entrypoint.sh:183",
|
|
||||||
"suggestion": "在 `collect_package_candidates` 函數中,變數 `kept_versions` 似乎是個拼寫錯誤,應修正為 `kept_count` 以保持與函數參數及其他變數命名的一致性。這不僅是風格問題,更可能導致邏輯錯誤。",
|
|
||||||
"is_new": false
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"level": "critical",
|
|
||||||
"role": "Maya",
|
|
||||||
"location": "entrypoint.sh:129",
|
|
||||||
"suggestion": "在 `fetch_package_versions` 函數中,`PAGE_LIMIT` 的驗證邏輯 `if [[ ! \"${limit}\" =~ ^[0-9]+$ ]] || (( limit <= 0 )); then fail \"Invalid PAGE_LIMIT: ${limit}\"; fi` 應增加測試案例。目前測試套件中缺少當 `PAGE_LIMIT` 為非正整數或無效格式時,腳本能正確失敗並輸出錯誤訊息的驗證。",
|
|
||||||
"is_new": false
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"level": "critical",
|
|
||||||
"role": "Maya",
|
|
||||||
"location": "entrypoint.sh:248",
|
|
||||||
"suggestion": "在 `main` 函數中,當 `resolve_token` 無法解析到 Gitea token 時(例如 `RUNNER_TOKEN` 未設定),腳本應正確失敗。目前的 `test_main_integration` 僅測試了成功解析 token 的情況,應增加測試案例以驗證此失敗路徑,確保在無 token 的情況下腳本能及早終止。",
|
|
||||||
"is_new": false
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"level": "critical",
|
|
||||||
"role": "Zara",
|
|
||||||
"location": "entrypoint.sh:370",
|
|
||||||
"suggestion": "在 `process_candidates` 函式中,`url_encode` 函式在迴圈內被呼叫,每次呼叫都會啟動一個新的 `jq` 外部程序來進行 URL 編碼。如果需要刪除的套件版本數量很多,這會導致大量的程序啟動和上下文切換開銷,嚴重影響效能。建議在 Bash 中直接實現 URL 編碼邏輯(例如使用 `printf %s \"$value\" | xxd -p | sed 's/\\(..\\)/%\\1/g'` 並處理安全字元),或者考慮 Gitea API 是否支援未編碼的套件名稱/版本,以避免頻繁的外部程序呼叫。",
|
|
||||||
"is_new": true
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"level": "warning",
|
|
||||||
"role": "Maya",
|
|
||||||
"location": "entrypoint.sh:109",
|
|
||||||
"suggestion": "在 `api_request` 函數中,日誌輸出會根據 API 回應是否包含 `x-gitea-request-id` 或 `x-request-id` 而有所不同。目前的測試案例僅涵蓋了有 `request_id` 的情況,建議增加一個測試案例來驗證當 API 回應沒有提供 `request_id` 時的日誌行為,確保所有日誌路徑都被覆蓋。",
|
|
||||||
"is_new": false
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"level": "warning",
|
|
||||||
"role": "Maya",
|
|
||||||
"location": "entrypoint.sh:267",
|
|
||||||
"suggestion": "在 `main` 函數中,`trap cleanup EXIT` 用於確保臨時文件 `candidate_file` 在腳本結束時被刪除。雖然 `test_main_integration` 執行了 `main` 函數,但並未明確驗證 `candidate_file` 在 `main` 執行結束後是否確實被移除。建議在測試中增加檢查,以確保資源清理機制正常運作,避免臨時文件洩漏。",
|
|
||||||
"is_new": false
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"level": "warning",
|
|
||||||
"role": "Zara",
|
|
||||||
"location": "entrypoint.sh:246",
|
|
||||||
"suggestion": "在 `fetch_package_versions` 函式的分頁迴圈中,每次迭代都會呼叫 `jq -s '.[0] + .[1]'` 來合併 JSON 陣列。這意味著每次迭代都會啟動一個新的 `jq` 程序,並讀寫多個臨時檔案。對於包含大量版本的套件(需要多個分頁請求),這種重複的程序啟動和檔案 I/O 會累積成顯著的效能開銷。建議將每個分頁的 JSON 響應追加到一個臨時檔案中(例如,使用 `printf '%s\\n' \"${API_RESPONSE_BODY}\" >> \"${aggregate_file}\"`),然後在迴圈結束後,只執行一次 `jq -s '.'` 來將所有 JSON 物件合併成一個最終的陣列,以減少程序啟動和檔案操作次數。",
|
|
||||||
"is_new": true
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"level": "warning",
|
|
||||||
"role": "Maya",
|
|
||||||
"location": "entrypoint.sh:207-208",
|
|
||||||
"suggestion": "`process_candidates` 函數在構建刪除請求的 URL 時,使用了 `url_encode` 來處理 `package_name` 和 `version`。雖然 `url_encode` 函數本身有測試,但 `process_candidates` 的測試案例中使用的套件名稱和版本(例如 `pkg-a`, `1.0.0`)不包含需要特殊 URL 編碼的字元。建議新增測試案例,使用包含特殊字元(例如 `/`, `?`, `+`, ` `)的套件名稱或版本,以確保 URL 編碼在實際刪除請求中能正確運作。",
|
|
||||||
"is_new": true
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"level": "info",
|
|
||||||
"role": "Leo",
|
|
||||||
"location": "tests/entrypoint.sh",
|
|
||||||
"suggestion": "測試中的 `jq` 模擬實作與 `entrypoint.sh` 中使用的特定 `jq` 表達式緊密耦合。若 `entrypoint.sh` 中的 `jq` 邏輯發生變化,此模擬也必須同步更新,這可能增加測試維護的成本。建議考慮使用更通用的 `jq` 模擬方式,或在測試中直接使用真實的 `jq` 工具(若測試環境允許且效能可接受),以減少模擬與實際行為不同步的風險。",
|
|
||||||
"is_new": false
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"level": "info",
|
|
||||||
"role": "Rex",
|
|
||||||
"location": "entrypoint.sh",
|
|
||||||
"suggestion": "雖然 `GITEA_SERVER_URL` 預期是來自受信任的環境變數,但為了增強韌性,可以考慮在腳本中加入對此 URL 格式的明確驗證,以確保其為有效的 HTTP/HTTPS URL,避免因格式錯誤導致的非預期行為。",
|
|
||||||
"is_new": false
|
|
||||||
}
|
|
||||||
]
|
|
||||||
|
|||||||
@@ -8,7 +8,7 @@
|
|||||||
- 預設每個套件保留最新 `2` 個版本。
|
- 預設每個套件保留最新 `2` 個版本。
|
||||||
- 直接刪除超出保留數量的舊版本。
|
- 直接刪除超出保留數量的舊版本。
|
||||||
- 只處理你指定的 NuGet 套件名稱,可一次指定多個。
|
- 只處理你指定的 NuGet 套件名稱,可一次指定多個。
|
||||||
- 輸出精簡 log,只保留必要的錯誤與 summary。
|
- 輸出整理過的 log,分成 summary、delete queue、delete results 幾個區塊。
|
||||||
- 每頁預設抓取 100 筆版本,可用 `PAGE_LIMIT` 調整。
|
- 每頁預設抓取 100 筆版本,可用 `PAGE_LIMIT` 調整。
|
||||||
|
|
||||||
## Token 來源順序
|
## Token 來源順序
|
||||||
@@ -33,9 +33,11 @@ Action 會依序嘗試以下來源:
|
|||||||
|
|
||||||
執行時只會輸出必要資訊:
|
執行時只會輸出必要資訊:
|
||||||
|
|
||||||
- `ERROR: ...`
|
- `Cleanup summary`
|
||||||
|
- `Delete queue`
|
||||||
|
- `Delete results`
|
||||||
|
- `Final summary`
|
||||||
- `Summary: packages=... versions=... kept=... candidates=... deleted=... errors=...`
|
- `Summary: packages=... versions=... kept=... candidates=... deleted=... errors=...`
|
||||||
- `No matching packages found for requested package_names` 之類的少量狀態訊息
|
|
||||||
|
|
||||||
## Workflow 範例
|
## Workflow 範例
|
||||||
|
|
||||||
|
|||||||
+46
-2
@@ -10,6 +10,15 @@ fail() {
|
|||||||
exit 1
|
exit 1
|
||||||
}
|
}
|
||||||
|
|
||||||
|
section() {
|
||||||
|
log ""
|
||||||
|
log "$1"
|
||||||
|
}
|
||||||
|
|
||||||
|
kv() {
|
||||||
|
printf ' %-11s: %s\n' "$1" "$2" >&2
|
||||||
|
}
|
||||||
|
|
||||||
cleanup_candidate_file=""
|
cleanup_candidate_file=""
|
||||||
|
|
||||||
cleanup() {
|
cleanup() {
|
||||||
@@ -283,8 +292,16 @@ process_candidates() {
|
|||||||
local package_name version _created_at
|
local package_name version _created_at
|
||||||
local encoded_owner encoded_package_name encoded_version
|
local encoded_owner encoded_package_name encoded_version
|
||||||
local body_file headers_file meta http_code status_text request_id
|
local body_file headers_file meta http_code status_text request_id
|
||||||
|
local current_package=""
|
||||||
|
|
||||||
if [[ ! -s "${candidate_file}" ]]; then
|
if [[ ! -s "${candidate_file}" ]]; then
|
||||||
|
section "Cleanup summary"
|
||||||
|
kv "packages" "${package_count}"
|
||||||
|
kv "versions" "${total_version_count}"
|
||||||
|
kv "kept" "${kept_count}"
|
||||||
|
kv "candidates" "0"
|
||||||
|
kv "deleted" "0"
|
||||||
|
kv "errors" "0"
|
||||||
log "Summary: packages=${package_count} versions=${total_version_count} kept=${kept_count} candidates=0 deleted=0 errors=0"
|
log "Summary: packages=${package_count} versions=${total_version_count} kept=${kept_count} candidates=0 deleted=0 errors=0"
|
||||||
return 0
|
return 0
|
||||||
fi
|
fi
|
||||||
@@ -292,6 +309,25 @@ process_candidates() {
|
|||||||
body_file="$(mktemp)"
|
body_file="$(mktemp)"
|
||||||
headers_file="$(mktemp)"
|
headers_file="$(mktemp)"
|
||||||
encoded_owner="$(url_encode "${owner}")"
|
encoded_owner="$(url_encode "${owner}")"
|
||||||
|
|
||||||
|
section "Cleanup summary"
|
||||||
|
kv "packages" "${package_count}"
|
||||||
|
kv "versions" "${total_version_count}"
|
||||||
|
kv "kept" "${kept_count}"
|
||||||
|
kv "candidates" "${candidate_count}"
|
||||||
|
|
||||||
|
section "Delete queue"
|
||||||
|
while IFS=$'\t' read -r package_name version _created_at; do
|
||||||
|
[[ -z "${package_name}" ]] && continue
|
||||||
|
|
||||||
|
if [[ "${package_name}" != "${current_package:-}" ]]; then
|
||||||
|
current_package="${package_name}"
|
||||||
|
log " ${current_package}"
|
||||||
|
fi
|
||||||
|
log " - ${version} (created_at=${_created_at})"
|
||||||
|
done < "${candidate_file}"
|
||||||
|
|
||||||
|
section "Delete results"
|
||||||
while IFS=$'\t' read -r package_name version _created_at; do
|
while IFS=$'\t' read -r package_name version _created_at; do
|
||||||
[[ -z "${package_name}" ]] && continue
|
[[ -z "${package_name}" ]] && continue
|
||||||
|
|
||||||
@@ -304,16 +340,24 @@ process_candidates() {
|
|||||||
|
|
||||||
if [[ "${http_code}" =~ ^2 ]]; then
|
if [[ "${http_code}" =~ ^2 ]]; then
|
||||||
deleted_count=$((deleted_count + 1))
|
deleted_count=$((deleted_count + 1))
|
||||||
|
log " - deleted ${package_name}@${version} [${status_text}]"
|
||||||
else
|
else
|
||||||
if [[ -n "${request_id}" ]]; then
|
if [[ -n "${request_id}" ]]; then
|
||||||
log "ERROR: DELETE package ${package_name} version ${version} -> ${status_text} request_id=${request_id}"
|
log " - failed ${package_name}@${version} [${status_text}] request_id=${request_id}"
|
||||||
else
|
else
|
||||||
log "ERROR: DELETE package ${package_name} version ${version} -> ${status_text}"
|
log " - failed ${package_name}@${version} [${status_text}]"
|
||||||
fi
|
fi
|
||||||
error_count=$((error_count + 1))
|
error_count=$((error_count + 1))
|
||||||
fi
|
fi
|
||||||
done < "${candidate_file}"
|
done < "${candidate_file}"
|
||||||
|
|
||||||
|
section "Final summary"
|
||||||
|
kv "packages" "${package_count}"
|
||||||
|
kv "versions" "${total_version_count}"
|
||||||
|
kv "kept" "${kept_count}"
|
||||||
|
kv "candidates" "${candidate_count}"
|
||||||
|
kv "deleted" "${deleted_count}"
|
||||||
|
kv "errors" "${error_count}"
|
||||||
log "Summary: packages=${package_count} versions=${total_version_count} kept=${kept_count} candidates=${candidate_count} deleted=${deleted_count} errors=${error_count}"
|
log "Summary: packages=${package_count} versions=${total_version_count} kept=${kept_count} candidates=${candidate_count} deleted=${deleted_count} errors=${error_count}"
|
||||||
rm -f "${body_file}" "${headers_file}"
|
rm -f "${body_file}" "${headers_file}"
|
||||||
}
|
}
|
||||||
|
|||||||
+8
-1
@@ -396,6 +396,7 @@ test_process_candidates_empty() {
|
|||||||
candidate_file="$(mktemp)"
|
candidate_file="$(mktemp)"
|
||||||
|
|
||||||
capture_call process_candidates acme "${candidate_file}" 0 0 0 0 token
|
capture_call process_candidates acme "${candidate_file}" 0 0 0 0 token
|
||||||
|
assert_contains "${CAPTURE_STDERR}" "Cleanup summary" "process_candidates empty section"
|
||||||
assert_contains "${CAPTURE_STDERR}" "Summary: packages=0 versions=0 kept=0 candidates=0 deleted=0 errors=0" "process_candidates empty summary"
|
assert_contains "${CAPTURE_STDERR}" "Summary: packages=0 versions=0 kept=0 candidates=0 deleted=0 errors=0" "process_candidates empty summary"
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -410,7 +411,10 @@ test_process_candidates() {
|
|||||||
add_route DELETE "https://gitea.example/api/v1/packages/acme/nuget/pkg-b/2.0.0" 500 "Internal Server Error" del-2 '{"error":"boom"}'
|
add_route DELETE "https://gitea.example/api/v1/packages/acme/nuget/pkg-b/2.0.0" 500 "Internal Server Error" del-2 '{"error":"boom"}'
|
||||||
|
|
||||||
capture_call process_candidates acme "${candidate_file}" 2 4 3 2 token
|
capture_call process_candidates acme "${candidate_file}" 2 4 3 2 token
|
||||||
assert_contains "${CAPTURE_STDERR}" "ERROR: DELETE package pkg-b version 2.0.0 -> 500 Internal Server Error request_id=del-2" "process_candidates failure path"
|
assert_contains "${CAPTURE_STDERR}" "Delete queue" "process_candidates queue section"
|
||||||
|
assert_contains "${CAPTURE_STDERR}" "Delete results" "process_candidates results section"
|
||||||
|
assert_contains "${CAPTURE_STDERR}" " - failed pkg-b@2.0.0 [500 Internal Server Error] request_id=del-2" "process_candidates failure path"
|
||||||
|
assert_contains "${CAPTURE_STDERR}" "Final summary" "process_candidates final summary section"
|
||||||
assert_contains "${CAPTURE_STDERR}" "Summary: packages=2 versions=4 kept=3 candidates=2 deleted=1 errors=1" "process_candidates summary"
|
assert_contains "${CAPTURE_STDERR}" "Summary: packages=2 versions=4 kept=3 candidates=2 deleted=1 errors=1" "process_candidates summary"
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -427,6 +431,9 @@ test_main_integration() {
|
|||||||
add_route DELETE "https://gitea.example/api/v1/packages/acme/nuget/pkg-a/1.0.0" 204 "No Content" del-a ''
|
add_route DELETE "https://gitea.example/api/v1/packages/acme/nuget/pkg-a/1.0.0" 204 "No Content" del-a ''
|
||||||
|
|
||||||
capture_call main
|
capture_call main
|
||||||
|
assert_contains "${CAPTURE_STDERR}" "Cleanup summary" "main summary section"
|
||||||
|
assert_contains "${CAPTURE_STDERR}" "Delete queue" "main queue section"
|
||||||
|
assert_contains "${CAPTURE_STDERR}" "Delete results" "main results section"
|
||||||
assert_contains "${CAPTURE_STDERR}" "Summary: packages=2 versions=4 kept=3 candidates=1 deleted=1 errors=0" "main summary"
|
assert_contains "${CAPTURE_STDERR}" "Summary: packages=2 versions=4 kept=3 candidates=1 deleted=1 errors=0" "main summary"
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user