Skip to content

chore: reduce cleanup time #652

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
merged 3 commits into from
Oct 15, 2025
Merged

chore: reduce cleanup time #652

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
eefd86e
update testS
blva Oct 14, 2025
6c4d025
update
blva Oct 14, 2025
4c13ff3
Merge branch 'main' into fix-cleanup-2
blva Oct 14, 2025
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
13 changes: 8 additions & 5 deletions scripts/cleanupAtlasTestLeftovers.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ import { ConsoleLogger } from "../src/common/logger.js";
import { Keychain } from "../src/lib.js";
import { describe, it } from "vitest";

function isOlderThanADay(date: string): boolean {
const oneDayInMs = 24 * 60 * 60 * 1000;
function isOlderThanTwoHours(date: string): boolean {
const twoHoursInMs = 2 * 60 * 60 * 1000;
const projectDate = new Date(date);
const currentDate = new Date();
return currentDate.getTime() - projectDate.getTime() > oneDayInMs;
return currentDate.getTime() - projectDate.getTime() > twoHoursInMs;
}

async function findTestOrganization(client: ApiClient): Promise<AtlasOrganization> {
Expand All @@ -32,7 +32,7 @@ async function findAllTestProjects(client: ApiClient, orgId: string): Promise<Gr
});

const testProjects = projects?.results?.filter((proj) => proj.name.startsWith("testProj-")) || [];
return testProjects.filter((proj) => isOlderThanADay(proj.created));
return testProjects.filter((proj) => isOlderThanTwoHours(proj.created));
}

async function deleteAllClustersOnStaleProject(client: ApiClient, projectId: string): Promise<string[]> {
Expand Down Expand Up @@ -76,8 +76,11 @@ async function main(): Promise<void> {
);

const testOrg = await findTestOrganization(apiClient);
const testProjects = await findAllTestProjects(apiClient, testOrg.id || "");
if (!testOrg.id) {
throw new Error("Test organization ID not found.");
}

const testProjects = await findAllTestProjects(apiClient, testOrg.id);
if (testProjects.length === 0) {
console.log("No stale test projects found for cleanup.");
return;
Expand Down
14 changes: 12 additions & 2 deletions tests/integration/tools/atlas/atlasHelpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ export function describeWithAtlas(name: string, fn: IntegrationTestFunction): vo
const integration = setupIntegrationTest(
() => ({
...defaultTestConfig,
apiClientId: process.env.MDB_MCP_API_CLIENT_ID,
apiClientSecret: process.env.MDB_MCP_API_CLIENT_SECRET,
apiClientId: process.env.MDB_MCP_API_CLIENT_ID || "test-client",
apiClientSecret: process.env.MDB_MCP_API_CLIENT_SECRET || "test-secret",
Comment on lines +21 to +22
Copy link

Copilot AI Oct 14, 2025

Choose a reason for hiding this comment

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

Using hardcoded fallback credentials ('test-client', 'test-secret') creates a security risk. These fallback values should be removed or replaced with proper test environment validation that fails gracefully when credentials are missing.

Suggested change
apiClientId: process.env.MDB_MCP_API_CLIENT_ID || "test-client",
apiClientSecret: process.env.MDB_MCP_API_CLIENT_SECRET || "test-secret",
apiClientId: process.env.MDB_MCP_API_CLIENT_ID,
apiClientSecret: process.env.MDB_MCP_API_CLIENT_SECRET,

Copilot uses AI. Check for mistakes.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I just noticed this, why don't we use defaultTestConfig here which ideally should already parse these values?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we use defaultTestConfig but override it with the test items if it's not there

apiBaseUrl: process.env.MDB_MCP_API_BASE_URL ?? "https://cloud-dev.mongodb.com",
}),
() => defaultDriverOptions
Expand All @@ -35,6 +35,16 @@ interface ProjectTestArgs {

type ProjectTestFunction = (args: ProjectTestArgs) => void;

export function withCredentials(integration: IntegrationTest, fn: IntegrationTestFunction): SuiteCollector<object> {
const describeFn =
!process.env.MDB_MCP_API_CLIENT_ID?.length || !process.env.MDB_MCP_API_CLIENT_SECRET?.length
Copy link

Copilot AI Oct 14, 2025

Choose a reason for hiding this comment

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

The credential validation logic is duplicated between describeWithAtlas and withCredentials. Consider extracting this into a shared helper function to reduce code duplication.

Copilot uses AI. Check for mistakes.

? describe.skip
: describe;
return describeFn("with credentials", () => {
fn(integration);
});
}

export function withProject(integration: IntegrationTest, fn: ProjectTestFunction): SuiteCollector<object> {
return describe("with project", () => {
let projectId: string = "";
Expand Down
32 changes: 17 additions & 15 deletions tests/integration/tools/atlas/orgs.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,23 +1,25 @@
import { expectDefined, getDataFromUntrustedContent, getResponseElements } from "../../helpers.js";
import { parseTable, describeWithAtlas } from "./atlasHelpers.js";
import { parseTable, describeWithAtlas, withCredentials } from "./atlasHelpers.js";
import { describe, expect, it } from "vitest";

describeWithAtlas("orgs", (integration) => {
describe("atlas-list-orgs", () => {
it("should have correct metadata", async () => {
const { tools } = await integration.mcpClient().listTools();
const listOrgs = tools.find((tool) => tool.name === "atlas-list-orgs");
expectDefined(listOrgs);
});
withCredentials(integration, () => {
describe("atlas-list-orgs", () => {
it("should have correct metadata", async () => {
const { tools } = await integration.mcpClient().listTools();
const listOrgs = tools.find((tool) => tool.name === "atlas-list-orgs");
expectDefined(listOrgs);
});

it("returns org names", async () => {
const response = await integration.mcpClient().callTool({ name: "atlas-list-orgs", arguments: {} });
const elements = getResponseElements(response);
expect(elements[0]?.text).toContain("Found 1 organizations");
expect(elements[1]?.text).toContain("<untrusted-user-data-");
const data = parseTable(getDataFromUntrustedContent(elements[1]?.text ?? ""));
expect(data).toHaveLength(1);
expect(data[0]?.["Organization Name"]).toEqual("MongoDB MCP Test");
it("returns org names", async () => {
const response = await integration.mcpClient().callTool({ name: "atlas-list-orgs", arguments: {} });
const elements = getResponseElements(response);
expect(elements[0]?.text).toContain("Found 1 organizations");

Check failure on line 17 in tests/integration/tools/atlas/orgs.test.ts

View workflow job for this annotation

GitHub Actions / Run Atlas tests

tests/integration/tools/atlas/orgs.test.ts > orgs > with credentials > atlas-list-orgs > returns org names 

AssertionError: expected 'Unable to authenticate with MongoDB A…' to contain 'Found 1 organizations'

- Expected
+ Received

- Found 1 organizations
+ Unable to authenticate with MongoDB Atlas, API error: [401 Unauthorized] error calling Atlas API: Unauthorized; You are not authorized for this resource.
+
+ Hint: Your API credentials may be invalid, expired or lack permissions.
+ Please check your Atlas API credentials and ensure they have the appropriate permissions.
+ For more information on setting up API keys, visit: https://www.mongodb.com/docs/atlas/configure-api-access/

 ❯ tests/integration/tools/atlas/orgs.test.ts:17:43
expect(elements[1]?.text).toContain("<untrusted-user-data-");
const data = parseTable(getDataFromUntrustedContent(elements[1]?.text ?? ""));
expect(data).toHaveLength(1);
expect(data[0]?.["Organization Name"]).toEqual("MongoDB MCP Test");
});
});
});
});
Loading