test(comments review): 抽出嚴重等級解析斷言 #39
@@ -1,42 +1,18 @@
|
||||
[
|
||||
{
|
||||
"level": "warning",
|
||||
"role": "Leo",
|
||||
"location": "app/comments.test.js:30",
|
||||
"problem": "常數 `REVIEW_SEVERITY_LABELS`、`REVIEW_SEVERITY_PATTERN` 以及輔助函式 `reviewSeverityLabel` 被定義在測試檔案的 `describe` 區塊內。如果實際的應用程式邏輯(例如產生或處理這些評論的程式碼)也需要用到這些資訊,那麼這會造成邏輯重複或知識分散。未來若評論格式或嚴重等級標籤有變動,將需要同時修改多處,增加維護成本與出錯風險。",
|
||||
"suggestion": "將 `REVIEW_SEVERITY_LABELS`、`REVIEW_SEVERITY_PATTERN` 和 `reviewSeverityLabel` 這些與評論格式相關的常數與函式,提取到一個獨立的共用模組中(例如 `app/utils/reviewComments.js`),並讓測試檔案和任何需要用到它們的應用程式邏輯都從該模組匯入。這樣能確保「評論格式」的定義只有一個來源,提升可維護性。",
|
||||
"is_new": true
|
||||
},
|
||||
{
|
||||
"level": "info",
|
||||
"role": "Bard",
|
||||
"location": "app/comments.test.js:211",
|
||||
"problem": "程式碼中直接嵌入了正規表達式 `/嚴重等級\\*\\*:(.+)/`,這是一個「魔術值」。它讓程式碼的意圖不夠清晰,且若未來需要修改此模式,會增加維護的困難度,也使得該行程式碼過於冗長,影響閱讀流暢性。",
|
||||
"suggestion": "建議將此正規表達式提取為一個具名常數,以提升可讀性與可維護性。例如:\n```javascript\nconst SEVERITY_PATTERN = /嚴重等級\\*\\*:(.+)/;\n// ...\nreviewCalls[0].comments.map(c => c.body.match(SEVERITY_PATTERN)?.[1]),\n```",
|
||||
"is_new": true
|
||||
},
|
||||
{
|
||||
"level": "warning",
|
||||
"role": "Leo",
|
||||
"location": "app/comments.test.js:212",
|
||||
"problem": "測試中直接硬編碼了嚴重等級的顯示文字 `['🔴 嚴重', '🟡 警告', '🔵 建議']`。這些字串很可能是應用程式中定義的常數,直接複製貼上會導致未來當這些等級的顯示文字需要調整時,必須同時修改應用程式程式碼和測試程式碼,增加維護負擔,也容易造成不一致。",
|
||||
"suggestion": "如果這些嚴重等級的顯示文字在應用程式中已有定義(例如:`SEVERITY_LEVELS.CRITICAL.DISPLAY_TEXT`),建議在測試中引入並使用這些常數,確保測試與實際邏輯保持同步,避免重複定義的技術債。",
|
||||
"is_new": true
|
||||
},
|
||||
{
|
||||
"level": "warning",
|
||||
"role": "Leo",
|
||||
"location": "app/comments.test.js:211",
|
||||
"problem": "測試中用來解析 `c.body` 的正規表達式 `(/嚴重等級\\*\\*:(.+)/)` 過於依賴特定的字串格式「嚴重等級**:」。如果未來這個前綴文字有任何微小的變動(例如:多一個空格、換個標點符號、或改用其他詞彙),即使語義不變,測試也會立刻失效,導致不必要的維護工作,增加了測試的脆弱性。",
|
||||
"suggestion": "考慮讓正規表達式更具彈性,例如使用 `\\s*` 匹配空白,或將這個字串格式的解析邏輯封裝成一個輔助函式,並在該函式中處理可能存在的格式變動彈性。如果 `c.body` 的內容是從結構化資料組裝而來,更理想的做法是測試該結構化資料本身,而非其最終的字串呈現。",
|
||||
"is_new": true
|
||||
},
|
||||
{
|
||||
"level": "warning",
|
||||
"role": "Mage",
|
||||
"location": "app/comments.test.js:211",
|
||||
"problem": "此處的正規表達式 `嚴重等級\\*\\*:(.+)` 使用了貪婪匹配 `.+`。這可能導致它捕獲到「嚴重等級**:」後方,直到字串結尾的所有內容,而非僅僅是預期的嚴重等級文字。若 `c.body` 中在嚴重等級後方還有其他文字(例如:「嚴重等級**:🔴 嚴重。請注意此問題。」),此測試將會因為提取到錯誤的內容而失敗,或在實際情況與預期不符時,無法正確驗證。",
|
||||
"suggestion": "為了確保只捕獲到預期的嚴重等級文字,應使用更精確的正規表達式。考量到測試的預期值是固定的 `['🔴 嚴重', '🟡 警告', '🔵 建議']`,最穩健的作法是直接匹配這些值,例如:\n`c.body.match(/嚴重等級\\*\\*:(🔴 嚴重|🟡 警告|🔵 建議)/)?.[1]`\n或者,如果嚴重等級後方可能跟隨其他文字但有明確分隔符(如句點、換行符或字串結尾),可以使用非貪婪匹配並指定邊界,例如:\n`c.body.match(/嚴重等級\\*\\*:(.+?)(?:\\s|$|\\.|\\n)/)?.[1]`",
|
||||
"is_new": true
|
||||
},
|
||||
{
|
||||
"level": "warning",
|
||||
"role": "Maya",
|
||||
"location": "app/comments.test.js:211",
|
||||
"problem": "這個新增的斷言只驗證了評論內文(`c.body`)中存在且格式正確的嚴重等級資訊。然而,它沒有涵蓋到評論內文可能沒有嚴重等級資訊,或是嚴重等級格式不符預期的情境,這讓程式碼的行為在這些邊界條件下缺乏驗證。",
|
||||
"suggestion": "建議新增測試案例,或擴充現有案例的測試資料,以涵蓋以下情境:\n1. 評論內文完全沒有「嚴重等級**:」這個模式時,應驗證程式碼的行為(例如,是否正確提取到 `undefined` 或預設值)。\n2. 評論內文有「嚴重等級**:」但後面沒有內容,或內容格式不符預期時,應驗證程式碼的行為(例如,是否正確處理為 `undefined` 或拋出錯誤)。",
|
||||
"location": "app/comments.test.js:192",
|
||||
"problem": "此處新增的 `reviewSeverityLabel` 函式,雖其意圖在上下文脈絡中尚稱清晰,但若能為其添上一筆簡潔的 JSDoc 註解,闡明其參數與回傳值的語義,將使這段樂章更臻完善,即便在測試檔案中,亦能提升未來維護者的閱讀體驗,使程式碼的旋律更加和諧。",
|
||||
"suggestion": "建議為 `reviewSeverityLabel` 函式加上 JSDoc 註解,例如:\n```javascript\n /**\n * 從評論物件中提取嚴重等級標籤。\n * @param {object | null | undefined} comment - 評論物件,預期包含 `body` 屬性。\n * @returns {string | undefined} 嚴重等級標籤字串(如 '🔴 嚴重'),若無匹配或輸入無效則回傳 undefined。\n */\n function reviewSeverityLabel(comment) {\n return comment?.body?.match(REVIEW_SEVERITY_PATTERN)?.[1];\n }\n```",
|
||||
"is_new": true
|
||||
}
|
||||
]
|
||||
|
||||
+27
-2
@@ -187,6 +187,31 @@ describe('postNewCriticalComments', () => {
|
||||
});
|
||||
|
||||
describe('postFindingsReview', () => {
|
||||
const REVIEW_SEVERITY_LABELS = ['🔴 嚴重', '🟡 警告', '🔵 建議'];
|
||||
const REVIEW_SEVERITY_PATTERN = new RegExp(`\\*\\*嚴重等級\\*\\*:(${REVIEW_SEVERITY_LABELS.join('|')})(?:\\n|$)`);
|
||||
|
||||
function reviewSeverityLabel(comment) {
|
||||
return comment?.body?.match(REVIEW_SEVERITY_PATTERN)?.[1];
|
||||
}
|
||||
|
||||
it('handles missing review severity bodies gracefully', () => {
|
||||
assert.equal(reviewSeverityLabel(null), undefined);
|
||||
assert.equal(reviewSeverityLabel(undefined), undefined);
|
||||
assert.equal(reviewSeverityLabel({}), undefined);
|
||||
assert.equal(reviewSeverityLabel({ body: null }), undefined);
|
||||
assert.equal(reviewSeverityLabel({ body: undefined }), undefined);
|
||||
});
|
||||
|
||||
it('extracts review severity labels only when the format is valid', () => {
|
||||
assert.equal(
|
||||
reviewSeverityLabel({ body: '**嚴重等級**:🔴 嚴重\n**審查員**:Rex' }),
|
||||
'🔴 嚴重',
|
||||
);
|
||||
assert.equal(reviewSeverityLabel({ body: '**審查員**:Rex' }), undefined);
|
||||
assert.equal(reviewSeverityLabel({ body: '**嚴重等級**:' }), undefined);
|
||||
assert.equal(reviewSeverityLabel({ body: '**嚴重等級**:高風險' }), undefined);
|
||||
});
|
||||
|
||||
it('posts one review with statistics and sorted line comments', async () => {
|
||||
const reviewCalls = [];
|
||||
const findings = [
|
||||
@@ -209,8 +234,8 @@ describe('postFindingsReview', () => {
|
||||
['app/a.js', 'app/b.js', 'app/c.js'],
|
||||
);
|
||||
assert.deepEqual(
|
||||
reviewCalls[0].comments.map(c => c.body.match(/嚴重等級\*\*:(.+)/)?.[1]),
|
||||
['🔴 嚴重', '🟡 警告', '🔵 建議'],
|
||||
reviewCalls[0].comments.map(reviewSeverityLabel),
|
||||
REVIEW_SEVERITY_LABELS,
|
||||
);
|
||||
assert.deepEqual(
|
||||
reviewCalls[0].comments.map(c => c.new_position),
|
||||
|
||||
Reference in New Issue
Block a user