refactor(test): improve assertion quality in errors.test.ts

[nullvariant]

Summary

• Explicitly assert V8 stack availability in getSafeStack tests (remove conditional branch skip)
• Strengthen recursion test: reject empty strings, verify getter delegates to getSafeStack()
• Add wrapError boundary value tests for null/undefined/object inputs
• Add autoLog=true (default) SecurityError construction tests
• Sanitize catch clause error output format

Test plan

• TypeScript compile: npx tsc --noEmit passes
• ESLint: no warnings
• All tests pass with 100% statement coverage

🤖 Generated with Claude Code

[qodo-code-review]

ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan

Review Summary by Qodo

Improve test assertion quality and coverage in errors.test.ts

🧪 Tests

Walkthroughs

Description

• Add explicit V8 stack availability assertions, removing conditional skips
• Strengthen recursion test with empty string rejection and delegation verification
• Add boundary value tests for wrapError() with null/undefined/object inputs
• Add autoLog=true (default) SecurityError construction test cases
• Improve error output formatting in catch clause error handling

Diagram

flowchart LR
  A["Test Suite"] --> B["SecurityError Constructor"]
  A --> C["getSafeStack Tests"]
  A --> D["wrapError Factory"]
  B --> B1["Add autoLog=true tests"]
  C --> C1["Remove conditional skips"]
  C --> C2["Assert V8 stack availability"]
  D --> D1["Add null/undefined/object cases"]
  D --> D2["Verify boundary values"]

Loading

File Changes

1. extensions/git-id-switcher/src/test/errors.test.ts 🧪 Tests +86/-13

Strengthen assertions and add boundary value test coverage

• Added two new test cases for SecurityError constructor with default autoLog=true behavior for
 both VALIDATION and SECURITY categories
• Replaced conditional if (safeStack) checks with explicit assertions requiring non-empty strings,
 ensuring V8 stack traces are always available
• Enhanced recursion test to verify error.stack getter delegates to getSafeStack() and added
 empty string validation
• Added three boundary value test cases for wrapError() covering null, undefined, and plain object
 inputs
• Improved error output formatting in catch clause to safely handle non-Error objects using `error
 instanceof Error ? error.message : String(error)`

extensions/git-id-switcher/src/test/errors.test.ts

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX Issues (0)

1. Non-portable stack assertions 🐞 Bug ☼ Reliability

Description

testGetSafeStack() now unconditionally requires getSafeStack()/error.stack to be non-empty,
but SecurityError explicitly supports returning undefined (and '' for .stack) when no raw
stack is available. This can cause test failures in non-V8 runtimes or environments where
Error.captureStackTrace/stack strings are unavailable, despite the production code handling that
case.

Code

extensions/git-id-switcher/src/test/errors.test.ts[R158-162]

+    // V8 runtime must provide stack traces
+    assert.ok(safeStack !== undefined, 'getSafeStack() must return a string on V8');
+    assert.ok(safeStack.length > 0, 'getSafeStack() must not be empty');
+    // Should not contain /Users/username pattern (if macOS)
+    assert.ok(!/\/Users\/[a-zA-Z0-9_-]+\//.test(safeStack));

Evidence

The updated test asserts stack availability/non-emptiness, but the implementation explicitly returns
undefined when rawStack is missing and maps .stack to an empty string in that case, so the
stricter assertions are not universally valid.

extensions/git-id-switcher/src/test/errors.test.ts[157-179]
extensions/git-id-switcher/src/core/errors.ts[99-112]
extensions/git-id-switcher/src/core/errors.ts[156-191]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`errors.test.ts` now hard-requires non-empty stack strings, but `SecurityError.getSafeStack()` is designed to return `undefined` when no raw stack is available, and the `.stack` getter intentionally falls back to `''`. This makes the test suite fail in non-V8/no-stack environments even though the implementation handles them.

### Issue Context
`SecurityError` stores `rawStack` and `getSafeStack()` returns `undefined` when `rawStack` is falsy; the `stack` getter returns `getSafeStack() || ''`.

### Fix Focus Areas
- extensions/git-id-switcher/src/test/errors.test.ts[157-199]
- extensions/git-id-switcher/src/core/errors.ts[99-112]
- extensions/git-id-switcher/src/core/errors.ts[156-191]

### Suggested change
Adjust the assertions to match supported behavior, e.g.:
- Gate the “must be present” assertions behind a runtime capability check (`Error.captureStackTrace` and/or `error.getSafeStack() !== undefined`), OR
- Make the test deterministic by injecting a fake `rawStack` (similar to `getSafeStack.test.ts`) and asserting sanitization on that value.
- For `.stack`, assert it is a string and either (a) matches `getSafeStack()` when defined or (b) equals `''` when `getSafeStack()` is `undefined`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. AutoLog test adds warn noise 🐞 Bug ⚙ Maintainability

Description

New constructor tests rely on the default autoLog=true, which triggers securityLogger logging
and results in console.warn output even in unit-test mode. This makes test output noisier and can
obscure real failures.

Code

extensions/git-id-switcher/src/test/errors.test.ts[R95-121]

+  // autoLog defaults to true — SecurityError should be created without throwing
+  // (logError() is called internally; verify it does not throw)
+  {
+    const error = new SecurityError({
+      category: ErrorCategory.VALIDATION,
+      userMessage: 'Auto-logged error',
+      internalDetails: {
+        field: 'testField',
+        value: 'testValue',
+      },
+    });
+
+    assert.strictEqual(error.category, ErrorCategory.VALIDATION);
+    assert.strictEqual(error.userMessage, 'Auto-logged error');
+    assert.strictEqual(error.getInternalDetails().field, 'testField');
+  }
+
+  // autoLog defaults to true with SECURITY category
+  {
+    const error = new SecurityError({
+      category: ErrorCategory.SECURITY,
+      userMessage: 'Security auto-logged',
+    });
+
+    assert.strictEqual(error.category, ErrorCategory.SECURITY);
+    assert.strictEqual(error.userMessage, 'Security auto-logged');
+  }

Evidence

When autoLog is omitted, SecurityError calls logError(), which delegates to
securityLogger.logValidationFailure(). The logger’s log() path emits console.warn(...)
regardless of VS Code OutputChannel availability, so these tests will always produce warning output.

extensions/git-id-switcher/src/test/errors.test.ts[95-121]
extensions/git-id-switcher/src/core/errors.ts[109-112]
extensions/git-id-switcher/src/security/securityLogger.ts[360-374]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The new `autoLog`-default constructor tests cause `console.warn` output via `securityLogger`, which makes the unit test logs noisier.

### Issue Context
The intent of the test is to verify that `autoLog` default behavior does not throw, not to validate logging output.

### Fix Focus Areas
- extensions/git-id-switcher/src/test/errors.test.ts[95-121]
- extensions/git-id-switcher/src/security/securityLogger.ts[360-374]

### Suggested change
Wrap these test blocks with a temporary `console.warn` stub to keep output clean while still exercising the default autoLog path, e.g.:
```ts
const origWarn = console.warn;
console.warn = () => {};
try {
 // construct SecurityError without autoLog:false
} finally {
 console.warn = origWarn;
}
```
(Alternatively, if you *do* want to assert logging, capture the calls and assert on them rather than printing to stdout/stderr.)

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more

[nullvariant-slow]

🦥 Slow's Code Review 😩

...yawn... Do I really have to review this?

⚠️ TOO LONG... I can barely keep my eyes open reading these:

File	Lines

| extensions/git-id-switcher/src/test/errors.test.ts | 534 |

Split it up... reading long files is exhausting.

---

働きたくないでござる

This review was reluctantly filed by nullvariant-slow[bot]

[nullvariant-mimi]

🐰 Mimi's Validation Report ✅

All checks are looking good! Great job! 🎉

⏳ Some checks are still running. I will keep watching!

---

バリデーターを通してくださいね

This report was carefully prepared by nullvariant-mimi[bot]

[github-actions]

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Snapshot Warnings

⚠️: No snapshots were found for the head SHA 7069a4c.

Ensure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice.

Scanned Files

None

[nullvariant-ciel]

🕊️ Ciel's Mediation 🌤️

*~~ floating down from the clouds ~~ The zoo seems a bit noisy today...*

2 zoo members have reviewed this PR.

Zoo Member	Status
🦥 Slow	Commented
🐰 Mimi	Commented

⚖️ The zoo has mixed opinions. Some are concerned, some are fine with it. Please review each comment carefully and make the final call.

---

まあまあ、ほどほどに。

This mediation was peacefully delivered by nullvariant-ciel[bot]

[nullvariant-ciel]

🕊️ Ciel's Mediation 🌤️

*~~ floating down from the clouds ~~ The zoo seems a bit noisy today...*

2 zoo members have reviewed this PR.

Zoo Member	Status
🦥 Slow	Commented
🐰 Mimi	Commented

⚖️ The zoo has mixed opinions. Some are concerned, some are fine with it. Please review each comment carefully and make the final call.

---

まあまあ、ほどほどに。

This mediation was peacefully delivered by nullvariant-ciel[bot]

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[nullvariant-justice]

⚖️ Justice grants passage. CI checks passed — this code meets the garden's standards.