Skip to content

test(oauth): clean up post-IPC migration test debt from #30257 #30260

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
noanflaherty merged 1 commit into from
May 11, 2026
Merged

test(oauth): clean up post-IPC migration test debt from #30257 #30260

Show file tree
Hide file tree
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
1,232 changes: 0 additions & 1,232 deletions assistant/src/__tests__/credentials-cli.test.ts
View file
Open in desktop

This file was deleted.

2,045 changes: 35 additions & 2,010 deletions assistant/src/__tests__/oauth-cli.test.ts
View file
Open in desktop

Large diffs are not rendered by default.

711 changes: 711 additions & 0 deletions assistant/src/__tests__/oauth-commands-routes.test.ts
View file
Open in desktop

Large diffs are not rendered by default.

93 changes: 11 additions & 82 deletions assistant/src/__tests__/usage-cli.test.ts
View file
Open in desktop
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot May 11, 2026

Choose a reason for hiding this comment

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

🟡 Comment references removed code, violating assistant/AGENTS.md code comments rule

The new file-level JSDoc comment narrates history of removed code: "Tests that previously seeded the DB and asserted on table output / JSON shape now belong on the daemon route side — they were skipped pending IPC mock rewrite (#30257)" (lines 20-22). Per assistant/AGENTS.md § "Code comments": "do not reference code that has been removed" and "Avoid phrases like... 'previously used Y', or 'was removed in PR Z'".

(Refers to lines 20-22)

Prompt for agents 
The file-level JSDoc comment in usage-cli.test.ts (lines 15-27) narrates history of removed code at lines 20-22 ('Tests that previously seeded the DB and asserted on table output / JSON shape now belong on the daemon route side — they were skipped pending IPC mock rewrite (#30257)'). Per assistant/AGENTS.md code comments rule, remove the history-narrating portions. The comment should simply state that this file tests CLI argument validation for the usage command, and note the follow-up opportunity to add IPC mock tests.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Original file line number Diff line number Diff line change
@@ -1,3 +1,14 @@
/**
* CLI plumbing tests for `assistant usage` (cli/commands/usage.ts).
*
* The `usage breakdown` subcommand is daemon-mediated via `cliIpcCall`; only
* argument validation runs in the CLI process. Table formatting and
* aggregation behavior are covered daemon-side in `usage-routes.test.ts`.
*
* Follow-up opportunity: mock `../ipc/cli-client.js` and assert CLI plumbing
* (table formatting, --json output shape) against canned IPC responses.
*/

import { beforeEach, describe, expect, mock, test } from "bun:test";

import { Command } from "commander";
Expand All @@ -19,13 +30,8 @@ mock.module("../util/logger.js", () => ({
}),
}));

const { initializeDb } = await import("../memory/db-init.js");
const { getDb } = await import("../memory/db-connection.js");
const { recordUsageEvent } = await import("../memory/llm-usage-store.js");
const { registerUsageCommand } = await import("../cli/commands/usage.js");

initializeDb();

async function runCommand(args: string[]): Promise<{
exitCode: number;
output: string;
Expand Down Expand Up @@ -54,86 +60,9 @@ async function runCommand(args: string[]): Promise<{
return { exitCode, output: logLines.join("\n") };
}

function insertUsage(
overrides: Partial<Parameters<typeof recordUsageEvent>[0]>,
estimatedCostUsd = 0.01,
): void {
recordUsageEvent(
{
conversationId: null,
runId: null,
requestId: null,
actor: "main_agent",
provider: "anthropic",
model: "claude-sonnet-4-20250514",
inputTokens: 100,
outputTokens: 50,
cacheCreationInputTokens: 0,
cacheReadInputTokens: 0,
...overrides,
},
{ estimatedCostUsd, pricingStatus: "priced" },
);
}

describe("assistant usage CLI", () => {
beforeEach(() => {
logLines.length = 0;
getDb().run("DELETE FROM llm_usage_events");
});

// TODO(IPC test rewrite): this test depends on usage-cli's old
// direct-DB path; CLI now goes through cliIpcCall for breakdown.
// Re-enable when test mocks the IPC layer with breakdown response.
test.skip("breakdown JSON includes call-site display labels and groupKey", async () => {
insertUsage({ callSite: "mainAgent" });
insertUsage({ callSite: null, inputTokens: 200 }, 0.005);

const result = await runCommand([
"usage",
"breakdown",
"--range",
"all",
"--group-by",
"call_site",
"--json",
]);

expect(result.exitCode).toBe(0);
const parsed = JSON.parse(result.output) as {
breakdown: Array<{ group: string; groupKey: string | null }>;
};
expect(parsed.breakdown.map((row) => row.group)).toEqual([
"Main Agent",
"Unknown Task",
]);
expect(parsed.breakdown.map((row) => row.groupKey)).toEqual([
"mainAgent",
null,
]);
});

// TODO(IPC test rewrite): usage breakdown CLI now goes through
// cliIpcCall to the daemon for data fetching. Without an IPC mock
// this test hits the real exitFromIpcResult and exits with code 10
// in CI (no daemon). Re-enable after the test mocks
// '../ipc/cli-client.js' or the test is rewritten to assert pure
// CLI plumbing (arg parsing, table formatting on canned data).
test.skip("breakdown table prints friendly profile fallback labels", async () => {
insertUsage({ inferenceProfile: null });

const result = await runCommand([
"usage",
"breakdown",
"--range",
"all",
"--group-by",
"inference_profile",
]);

expect(result.exitCode).toBe(0);
expect(result.output).toContain("PROFILE");
expect(result.output).toContain("Default / Unset");
});

test("rejects invalid breakdown dimensions", async () => {
Expand Down
Loading
Loading