fix(FN-2321): standardize loopback-gated integration test labeling
- Refine the dashboard loopback integration helper with documented helper contracts and centralized skipped-name formatting - Keep loopback bind detection memoized so all gated integration suites share one environment probe - Update loopback-gated dashboard suites to use consistent, explicit scope labels in skip output - Refresh skipped-test inventory docs with the new audit commands and loopback helper usage details
This commit is contained in:
@@ -1,15 +1,16 @@
|
|||||||
# Skipped Test Inventory
|
# Skipped Test Inventory
|
||||||
|
|
||||||
_Last audited: 2026-04-20 (FN-2197)_
|
_Last audited: 2026-04-23 (FN-2321)_
|
||||||
|
|
||||||
This document tracks intentional skip usage in test suites so stale follow-up backlog items can be retired quickly.
|
This document tracks intentional skip usage in test suites so stale follow-up backlog items can be retired quickly.
|
||||||
|
|
||||||
## Current Inventory
|
## Current Inventory
|
||||||
|
|
||||||
Audit command:
|
Audit commands:
|
||||||
|
|
||||||
```bash
|
```bash
|
||||||
rg -n "\b(it|test|describe)\.skip\b|\bskipIf\b|\?\s*it\s*:\s*it\.skip" packages --glob "**/*.{test,spec}.{ts,tsx}"
|
rg -n "\b(it|test|describe)\.skip\b|\bskipIf\b|createLoopbackIntegrationTest\(" packages --glob "**/*.{test,spec}.{ts,tsx}"
|
||||||
|
rg -n "\?\s*it\s*:\s*it\.skip|detectLoopbackBinding" packages/dashboard/src --glob "**/*.{test,spec}.{ts,tsx}"
|
||||||
```
|
```
|
||||||
|
|
||||||
Current results:
|
Current results:
|
||||||
@@ -19,12 +20,15 @@ Current results:
|
|||||||
- `it.skip("step-session skill selection covered in step-session-executor.test.ts", ...)`
|
- `it.skip("step-session skill selection covered in step-session-executor.test.ts", ...)`
|
||||||
- Rationale: dedicated coverage exists in `step-session-executor.test.ts`; this marker documents ownership.
|
- Rationale: dedicated coverage exists in `step-session-executor.test.ts`; this marker documents ownership.
|
||||||
|
|
||||||
2. **Environment-gated integration aliases**
|
2. **Environment-gated integration aliases (loopback gated)**
|
||||||
- `packages/dashboard/src/server-static-assets.test.ts`
|
- Canonical helper: `packages/dashboard/src/__tests__/loopback-integration-test.ts`
|
||||||
- `packages/dashboard/src/__tests__/websocket.test.ts`
|
- Helper consumers:
|
||||||
- `packages/dashboard/src/__tests__/server-webhook.test.ts`
|
- `packages/dashboard/src/server-static-assets.test.ts`
|
||||||
- Pattern: `loopbackBindingAvailable ? it : it.skip`
|
- `packages/dashboard/src/__tests__/websocket.test.ts`
|
||||||
- Rationale: these integration tests require loopback binding support in the runtime environment.
|
- `packages/dashboard/src/__tests__/server-webhook.test.ts`
|
||||||
|
- Pattern: suites call `createLoopbackIntegrationTest(scope)` and register integration cases through the returned test function.
|
||||||
|
- Rationale: these suites require real loopback binding support (`127.0.0.1`) and are intentionally environment-gated.
|
||||||
|
- Auditability: when loopback binding is unavailable, skipped test names include a standardized reason and the suite scope label (`...; scope: <suite scope>`), making coverage gaps explicit in CI/test output.
|
||||||
|
|
||||||
3. **Build-output-gated checks**
|
3. **Build-output-gated checks**
|
||||||
- `packages/cli/src/__tests__/bundle-output.test.ts`
|
- `packages/cli/src/__tests__/bundle-output.test.ts`
|
||||||
|
|||||||
@@ -5,6 +5,15 @@ type IntegrationTestCase = (name: string, fn: () => unknown | Promise<unknown>,
|
|||||||
|
|
||||||
const LOOPBACK_SKIP_REASON = "loopback binding to 127.0.0.1 is unavailable in this environment";
|
const LOOPBACK_SKIP_REASON = "loopback binding to 127.0.0.1 is unavailable in this environment";
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Ensure skip output is auditable with both the standardized reason and the suite scope.
|
||||||
|
*
|
||||||
|
* Example: "... (skipped: loopback binding to 127.0.0.1 is unavailable in this environment; scope: websocket integration)"
|
||||||
|
*/
|
||||||
|
function formatLoopbackSkipName(testName: string, scope: string): string {
|
||||||
|
return `${testName} (skipped: ${LOOPBACK_SKIP_REASON}; scope: ${scope})`;
|
||||||
|
}
|
||||||
|
|
||||||
let loopbackBindingAvailablePromise: Promise<boolean> | null = null;
|
let loopbackBindingAvailablePromise: Promise<boolean> | null = null;
|
||||||
|
|
||||||
async function detectLoopbackBinding(): Promise<boolean> {
|
async function detectLoopbackBinding(): Promise<boolean> {
|
||||||
@@ -17,6 +26,10 @@ async function detectLoopbackBinding(): Promise<boolean> {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Memoize loopback probe results so all integration suites share a single bind check.
|
||||||
|
* This avoids creating redundant probe listeners during test startup.
|
||||||
|
*/
|
||||||
async function isLoopbackBindingAvailable(): Promise<boolean> {
|
async function isLoopbackBindingAvailable(): Promise<boolean> {
|
||||||
if (!loopbackBindingAvailablePromise) {
|
if (!loopbackBindingAvailablePromise) {
|
||||||
loopbackBindingAvailablePromise = detectLoopbackBinding();
|
loopbackBindingAvailablePromise = detectLoopbackBinding();
|
||||||
@@ -25,6 +38,12 @@ async function isLoopbackBindingAvailable(): Promise<boolean> {
|
|||||||
return await loopbackBindingAvailablePromise;
|
return await loopbackBindingAvailablePromise;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Canonical gate for dashboard integration tests that require loopback binding.
|
||||||
|
*
|
||||||
|
* When loopback is unavailable, every skipped test name includes a standardized
|
||||||
|
* reason string and the provided suite scope for auditability in test output.
|
||||||
|
*/
|
||||||
export async function createLoopbackIntegrationTest(scope: string): Promise<IntegrationTestCase> {
|
export async function createLoopbackIntegrationTest(scope: string): Promise<IntegrationTestCase> {
|
||||||
const loopbackBindingAvailable = await isLoopbackBindingAvailable();
|
const loopbackBindingAvailable = await isLoopbackBindingAvailable();
|
||||||
|
|
||||||
@@ -32,5 +51,5 @@ export async function createLoopbackIntegrationTest(scope: string): Promise<Inte
|
|||||||
return (name, fn, timeout) => it(name, fn, timeout);
|
return (name, fn, timeout) => it(name, fn, timeout);
|
||||||
}
|
}
|
||||||
|
|
||||||
return (name, fn, timeout) => it.skip(`${name} (skipped: ${LOOPBACK_SKIP_REASON}; scope: ${scope})`, fn, timeout);
|
return (name, fn, timeout) => it.skip(formatLoopbackSkipName(name, scope), fn, timeout);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -17,7 +17,7 @@ vi.mock("../github-webhooks.js", async () => {
|
|||||||
|
|
||||||
const mockGetGitHubAppConfig = vi.mocked(getGitHubAppConfig);
|
const mockGetGitHubAppConfig = vi.mocked(getGitHubAppConfig);
|
||||||
|
|
||||||
const webhookIntegrationTest = await createLoopbackIntegrationTest("GitHub webhook integration");
|
const webhookIntegrationTest = await createLoopbackIntegrationTest("server-webhook GitHub webhook integration");
|
||||||
|
|
||||||
class MockStore extends EventEmitter {
|
class MockStore extends EventEmitter {
|
||||||
private tasks = new Map<string, Task>();
|
private tasks = new Map<string, Task>();
|
||||||
|
|||||||
@@ -7,7 +7,7 @@ import { WebSocketManager } from "../websocket.js";
|
|||||||
import { InMemoryBadgePubSub, type BadgePubSub } from "../badge-pubsub.js";
|
import { InMemoryBadgePubSub, type BadgePubSub } from "../badge-pubsub.js";
|
||||||
import { createLoopbackIntegrationTest } from "./loopback-integration-test.js";
|
import { createLoopbackIntegrationTest } from "./loopback-integration-test.js";
|
||||||
|
|
||||||
const websocketIntegrationTest = await createLoopbackIntegrationTest("websocket integration");
|
const websocketIntegrationTest = await createLoopbackIntegrationTest("websocket /api/ws integration");
|
||||||
|
|
||||||
class MockSocket extends EventEmitter {
|
class MockSocket extends EventEmitter {
|
||||||
readyState: number = WebSocket.OPEN;
|
readyState: number = WebSocket.OPEN;
|
||||||
|
|||||||
@@ -2,7 +2,7 @@ import express from "express";
|
|||||||
import { describe, expect, it } from "vitest";
|
import { describe, expect, it } from "vitest";
|
||||||
import { createLoopbackIntegrationTest } from "./__tests__/loopback-integration-test.js";
|
import { createLoopbackIntegrationTest } from "./__tests__/loopback-integration-test.js";
|
||||||
|
|
||||||
const staticAssetIntegrationTest = await createLoopbackIntegrationTest("static asset serving");
|
const staticAssetIntegrationTest = await createLoopbackIntegrationTest("server-static-assets integration");
|
||||||
|
|
||||||
describe("static asset serving", () => {
|
describe("static asset serving", () => {
|
||||||
staticAssetIntegrationTest("returns 404 for missing asset paths instead of falling back to index.html", async () => {
|
staticAssetIntegrationTest("returns 404 for missing asset paths instead of falling back to index.html", async () => {
|
||||||
|
|||||||
Reference in New Issue
Block a user