Skip to content

Add unit tests #1669

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.

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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 change: 1 addition & 0 deletions jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,6 @@ module.exports = {
moduleNameMapper: {
"^vscode$": "<rootDir>/src/test/mock/vscode.ts",
"^@lib$": "<rootDir>/src/test/mock/lib.ts",
"^@extension$": "<rootDir>/src/modules.ts",
},
};
146 changes: 146 additions & 0 deletions src/test/suite/altimate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,12 @@
import { DBTTerminal } from "../../dbt_client/dbtTerminal";
import { PythonEnvironment } from "../../manifest/pythonEnvironment";
import { window, workspace, ConfigurationTarget } from "vscode";
import { Readable } from "stream";
import * as fs from "fs";
import * as path from "path";
import * as os from "os";
import { AltimateRequest } from "../../altimate";
import { RateLimitException } from "../../exceptions/rateLimitException";

type FetchFn = (
input: string | URL | Request,
Expand Down Expand Up @@ -170,4 +175,145 @@
expect(onProgress).toHaveBeenCalledTimes(1);
expect(onProgress).toHaveBeenCalledWith(expect.stringContaining("success"));
});

it("getCredentialsMessage varies by config", () => {
mockPythonEnv.getResolvedConfigValue
.mockReturnValueOnce("")
.mockReturnValueOnce("");
expect(request.getCredentialsMessage()).toContain(
"API Key and an instance name",
);

mockPythonEnv.getResolvedConfigValue.mockImplementation((key: string) =>
key === "altimateAiKey" ? "k" : "",
);
expect(request.getCredentialsMessage()).toContain("instance name");

mockPythonEnv.getResolvedConfigValue.mockImplementation((key: string) =>
key === "altimateInstanceName" ? "i" : "",
);
expect(request.getCredentialsMessage()).toContain("API key");

mockPythonEnv.getResolvedConfigValue
.mockReturnValueOnce("key")
.mockReturnValueOnce("instance");
expect(request.getCredentialsMessage()).toBeUndefined();
Copy link
Preview

Copilot AI May 20, 2025

Choose a reason for hiding this comment

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

Consider adding a comment to clarify why undefined is the expected return value when both credentials are provided, so future maintainers understand this business logic.

Copilot uses AI. Check for mistakes.

});

it("handlePreviewFeatures shows message when missing", () => {
mockPythonEnv.getResolvedConfigValue
.mockReturnValueOnce("")
.mockReturnValueOnce("");
const spy = jest
.spyOn(request as any, "showAPIKeyMessage")
.mockResolvedValue(undefined);
expect(request.handlePreviewFeatures()).toBe(false);
expect(spy).toHaveBeenCalled();
});

it("handlePreviewFeatures returns true when credentials exist", () => {
mockPythonEnv.getResolvedConfigValue
.mockReturnValueOnce("k")
.mockReturnValueOnce("i");
const spy = jest.spyOn(request as any, "showAPIKeyMessage");
expect(request.handlePreviewFeatures()).toBe(true);
expect(spy).not.toHaveBeenCalled();
});

it("readStreamToBlob collects stream data", async () => {
const stream = Readable.from(["a", "b"]);
const blob: any = await (request as any).readStreamToBlob(stream as any);
Copy link
Contributor

Choose a reason for hiding this comment

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

Avoid casting to any for readStreamToBlob; consider adding an explicit return type in its definition to ease refactoring.

Suggested change
const blob: any = await (request as any).readStreamToBlob(stream as any);
const blob: Blob = await (request as any).readStreamToBlob(stream as any);

This comment was generated because it violated a code review rule: mrule_lKhrJQCKUgYAxwcS.

const text = await blob.text();
expect(text).toBe("ab");
});

it("uploadToS3 uploads file", async () => {
mockPythonEnv.getResolvedConfigValue
.mockReturnValueOnce("key")
.mockReturnValueOnce("instance");
const file = path.join(os.tmpdir(), "up.txt");
fs.writeFileSync(file, "x");

Check failure

Code scanning / CodeQL

Insecure temporary file High test

Insecure creation of file in
the os temp dir
.
const mockRes = new Response("ok", { status: 200, statusText: "OK" });
fetchMock.mockResolvedValue(mockRes);
const res = await request.uploadToS3("http://s3", {}, file);
expect(res.status).toBe(200);
fs.rmSync(file);
Comment on lines +236 to +240
Copy link
Preview

Copilot AI May 20, 2025

Choose a reason for hiding this comment

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

Consider encapsulating temporary file creation and removal within a try/finally block to ensure cleanup even if an assertion fails.

Suggested change
const mockRes = new Response("ok", { status: 200, statusText: "OK" });
fetchMock.mockResolvedValue(mockRes);
const res = await request.uploadToS3("http://s3", {}, file);
expect(res.status).toBe(200);
fs.rmSync(file);
try {
const mockRes = new Response("ok", { status: 200, statusText: "OK" });
fetchMock.mockResolvedValue(mockRes);
const res = await request.uploadToS3("http://s3", {}, file);
expect(res.status).toBe(200);
} finally {
fs.rmSync(file);
}

Copilot uses AI. Check for mistakes.

});

it("uploadToS3 throws on failure", async () => {
mockPythonEnv.getResolvedConfigValue
.mockReturnValueOnce("key")
.mockReturnValueOnce("instance");
const file = path.join(os.tmpdir(), "up.txt");
fs.writeFileSync(file, "x");

Check failure

Code scanning / CodeQL

Insecure temporary file High test

Insecure creation of file in
the os temp dir
.

Copilot Autofix

AI 23 days ago

To fix the issue, we will replace the insecure temporary file creation logic with the tmp library, which securely creates temporary files. The tmp library ensures that the file is unique and inaccessible to other users. Specifically, we will:

  1. Import the tmp library.
  2. Replace the path.join(os.tmpdir(), "up.txt") logic with tmp.fileSync() to securely create a temporary file.
  3. Use the name property of the returned object from tmp.fileSync() to get the file path.
  4. Ensure the temporary file is removed after use by calling the removeCallback method provided by tmp.

Suggested changeset 2
src/test/suite/altimate.test.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/test/suite/altimate.test.ts b/src/test/suite/altimate.test.ts
--- a/src/test/suite/altimate.test.ts
+++ b/src/test/suite/altimate.test.ts
@@ -14,4 +14,4 @@
 import * as fs from "fs";
-import * as path from "path";
-import * as os from "os";
+import * as tmp from "tmp";
+// Removed unused imports: path, os
 import { AltimateRequest } from "../../altimate";
@@ -233,9 +233,9 @@
       .mockReturnValueOnce("instance");
-    const file = path.join(os.tmpdir(), "up.txt");
-    fs.writeFileSync(file, "x");
+    const tmpFile = tmp.fileSync();
+    fs.writeFileSync(tmpFile.name, "x");
     const mockRes = new Response("ok", { status: 200, statusText: "OK" });
     fetchMock.mockResolvedValue(mockRes);
-    const res = await request.uploadToS3("http://s3", {}, file);
+    const res = await request.uploadToS3("http://s3", {}, tmpFile.name);
     expect(res.status).toBe(200);
-    fs.rmSync(file);
+    tmpFile.removeCallback();
   });
@@ -246,10 +246,10 @@
       .mockReturnValueOnce("instance");
-    const file = path.join(os.tmpdir(), "up.txt");
-    fs.writeFileSync(file, "x");
+    const tmpFile = tmp.fileSync();
+    fs.writeFileSync(tmpFile.name, "x");
     const mockRes = new Response("bad", { status: 500, statusText: "ERR" });
     fetchMock.mockResolvedValue(mockRes);
-    await expect(request.uploadToS3("http://s3", {}, file)).rejects.toThrow(
+    await expect(request.uploadToS3("http://s3", {}, tmpFile.name)).rejects.toThrow(
       "Failed to upload data",
     );
-    fs.rmSync(file);
+    tmpFile.removeCallback();
   });
EOF
@@ -14,4 +14,4 @@
import * as fs from "fs";
import * as path from "path";
import * as os from "os";
import * as tmp from "tmp";
// Removed unused imports: path, os
import { AltimateRequest } from "../../altimate";
@@ -233,9 +233,9 @@
.mockReturnValueOnce("instance");
const file = path.join(os.tmpdir(), "up.txt");
fs.writeFileSync(file, "x");
const tmpFile = tmp.fileSync();
fs.writeFileSync(tmpFile.name, "x");
const mockRes = new Response("ok", { status: 200, statusText: "OK" });
fetchMock.mockResolvedValue(mockRes);
const res = await request.uploadToS3("http://s3", {}, file);
const res = await request.uploadToS3("http://s3", {}, tmpFile.name);
expect(res.status).toBe(200);
fs.rmSync(file);
tmpFile.removeCallback();
});
@@ -246,10 +246,10 @@
.mockReturnValueOnce("instance");
const file = path.join(os.tmpdir(), "up.txt");
fs.writeFileSync(file, "x");
const tmpFile = tmp.fileSync();
fs.writeFileSync(tmpFile.name, "x");
const mockRes = new Response("bad", { status: 500, statusText: "ERR" });
fetchMock.mockResolvedValue(mockRes);
await expect(request.uploadToS3("http://s3", {}, file)).rejects.toThrow(
await expect(request.uploadToS3("http://s3", {}, tmpFile.name)).rejects.toThrow(
"Failed to upload data",
);
fs.rmSync(file);
tmpFile.removeCallback();
});
package.json
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/package.json b/package.json
--- a/package.json
+++ b/package.json
@@ -1401,3 +1401,4 @@
     "zeromq": "^6.1.0",
-    "zod-to-json-schema": "^3.24.3"
+    "zod-to-json-schema": "^3.24.3",
+    "tmp": "^0.2.3"
   },
EOF
@@ -1401,3 +1401,4 @@
"zeromq": "^6.1.0",
"zod-to-json-schema": "^3.24.3"
"zod-to-json-schema": "^3.24.3",
"tmp": "^0.2.3"
},
This fix introduces these dependencies
Package Version Security advisories
tmp (npm) 0.2.3 None
Copilot is powered by AI and may make mistakes. Always verify output.
const mockRes = new Response("bad", { status: 500, statusText: "ERR" });
fetchMock.mockResolvedValue(mockRes);
await expect(request.uploadToS3("http://s3", {}, file)).rejects.toThrow(
"Failed to upload data",
);
fs.rmSync(file);
});
Comment on lines +230 to +255
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Security issue in temporary file creation

The tests are creating temporary files with fixed names in the system temp directory, which could cause issues if multiple test instances run in parallel or allow potential information leakage.

Consider using a secure method to create temporary files:

-const file = path.join(os.tmpdir(), "up.txt");
-fs.writeFileSync(file, "x");
+const tmpdir = fs.mkdtempSync(path.join(os.tmpdir(), 'altimate-test-'));
+const file = path.join(tmpdir, "up.txt");
+fs.writeFileSync(file, "x");
 // Test code
 fs.rmSync(file);
+fs.rmdirSync(tmpdir);

Let's check how this is handled in other parts of the codebase:


🏁 Script executed:

#!/bin/bash
# Search for patterns of temporary file creation to see how it's handled elsewhere
rg -A 3 "os\.tmpdir\(\)" --type ts

Length of output: 1833



---

🏁 Script executed:

```shell
#!/bin/bash
# Verify if any other tests use the same fixed temp filename "up.txt"
rg '"up.txt"' 

Length of output: 181


Ensure unique temporary directories in altimate tests

The altimate.test.ts specs currently write to a fixed file (up.txt) under os.tmpdir(), which can collide when tests run in parallel and may expose sensitive files. Other tests (e.g. utils.test.ts) already use fs.mkdtempSync to isolate temp folders—let’s do the same here.

Files to update:

  • src/test/suite/altimate.test.ts (both uploadToS3 cases around lines 230–255)

Recommended diff:

   it("uploadToS3 uploads file", async () => {
-    const file = path.join(os.tmpdir(), "up.txt");
-    fs.writeFileSync(file, "x");
+    const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-test-"));
+    const file = path.join(tmpDir, "up.txt");
+    fs.writeFileSync(file, "x");
     const mockRes = new Response("ok", { status: 200, statusText: "OK" });
     fetchMock.mockResolvedValue(mockRes);
     const res = await request.uploadToS3("http://s3", {}, file);
     expect(res.status).toBe(200);
-    fs.rmSync(file);
+    fs.rmSync(file);
+    fs.rmdirSync(tmpDir);
   });

   it("uploadToS3 throws on failure", async () => {
-    const file = path.join(os.tmpdir(), "up.txt");
-    fs.writeFileSync(file, "x");
+    const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-test-"));
+    const file = path.join(tmpDir, "up.txt");
+    fs.writeFileSync(file, "x");
     const mockRes = new Response("bad", { status: 500, statusText: "ERR" });
     fetchMock.mockResolvedValue(mockRes);
     await expect(request.uploadToS3("http://s3", {}, file)).rejects.toThrow(
       "Failed to upload data",
     );
-    fs.rmSync(file);
+    fs.rmSync(file);
+    fs.rmdirSync(tmpDir);
   });

This aligns with existing patterns (utils.test.ts) and avoids name collisions or leakage.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
it("uploadToS3 uploads file", async () => {
mockPythonEnv.getResolvedConfigValue
.mockReturnValueOnce("key")
.mockReturnValueOnce("instance");
const file = path.join(os.tmpdir(), "up.txt");
fs.writeFileSync(file, "x");
const mockRes = new Response("ok", { status: 200, statusText: "OK" });
fetchMock.mockResolvedValue(mockRes);
const res = await request.uploadToS3("http://s3", {}, file);
expect(res.status).toBe(200);
fs.rmSync(file);
});
it("uploadToS3 throws on failure", async () => {
mockPythonEnv.getResolvedConfigValue
.mockReturnValueOnce("key")
.mockReturnValueOnce("instance");
const file = path.join(os.tmpdir(), "up.txt");
fs.writeFileSync(file, "x");
const mockRes = new Response("bad", { status: 500, statusText: "ERR" });
fetchMock.mockResolvedValue(mockRes);
await expect(request.uploadToS3("http://s3", {}, file)).rejects.toThrow(
"Failed to upload data",
);
fs.rmSync(file);
});
it("uploadToS3 uploads file", async () => {
mockPythonEnv.getResolvedConfigValue
.mockReturnValueOnce("key")
.mockReturnValueOnce("instance");
- const file = path.join(os.tmpdir(), "up.txt");
- fs.writeFileSync(file, "x");
+ const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-test-"));
+ const file = path.join(tmpDir, "up.txt");
+ fs.writeFileSync(file, "x");
const mockRes = new Response("ok", { status: 200, statusText: "OK" });
fetchMock.mockResolvedValue(mockRes);
const res = await request.uploadToS3("http://s3", {}, file);
expect(res.status).toBe(200);
- fs.rmSync(file);
+ fs.rmSync(file);
+ fs.rmdirSync(tmpDir);
});
it("uploadToS3 throws on failure", async () => {
mockPythonEnv.getResolvedConfigValue
.mockReturnValueOnce("key")
.mockReturnValueOnce("instance");
- const file = path.join(os.tmpdir(), "up.txt");
- fs.writeFileSync(file, "x");
+ const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-test-"));
+ const file = path.join(tmpDir, "up.txt");
+ fs.writeFileSync(file, "x");
const mockRes = new Response("bad", { status: 500, statusText: "ERR" });
fetchMock.mockResolvedValue(mockRes);
await expect(request.uploadToS3("http://s3", {}, file)).rejects.toThrow(
"Failed to upload data",
);
- fs.rmSync(file);
+ fs.rmSync(file);
+ fs.rmdirSync(tmpDir);
});
🧰 Tools
🪛 GitHub Check: CodeQL

[failure] 235-235: Insecure temporary file
Insecure creation of file in the os temp dir.


[failure] 248-248: Insecure temporary file
Insecure creation of file in the os temp dir.

🤖 Prompt for AI Agents
In src/test/suite/altimate.test.ts around lines 230 to 255, the tests create
temporary files with a fixed name "up.txt" in the system temp directory, which
risks collisions and security issues during parallel test runs. To fix this,
replace the fixed filename with a unique temporary directory created using
fs.mkdtempSync, then create the file inside this directory. Ensure to clean up
by removing the entire temporary directory after the test completes.


it("fetchAsStream throws NotFoundError", async () => {
mockPythonEnv.getResolvedConfigValue
.mockReturnValueOnce("key")
.mockReturnValueOnce("instance");
fetchMock.mockResolvedValue(
new Response("", { status: 404, statusText: "NotFound" }),
);
await expect(request.fetchAsStream("foo", {}, jest.fn())).rejects.toThrow(
"Resource Not found",
);
});

it("fetchAsStream throws RateLimitException", async () => {
mockPythonEnv.getResolvedConfigValue
.mockReturnValueOnce("key")
.mockReturnValueOnce("instance");
fetchMock.mockResolvedValue(
new Response("wait", {
status: 429,
statusText: "Too Many",
headers: { "Retry-After": "1" },
}),
);
await expect(request.fetchAsStream("foo", {}, jest.fn())).rejects.toThrow(
RateLimitException,
);
});

it("fetchAsStream throws ForbiddenError", async () => {
mockPythonEnv.getResolvedConfigValue
.mockReturnValueOnce("key")
.mockReturnValueOnce("instance");
fetchMock.mockResolvedValue(
new Response("", { status: 403, statusText: "Forbidden" }),
);
await expect(request.fetchAsStream("foo", {}, jest.fn())).rejects.toThrow(
"Invalid credentials",
);
});

it("fetchAsStream returns null on empty body", async () => {
mockPythonEnv.getResolvedConfigValue
.mockReturnValueOnce("key")
.mockReturnValueOnce("instance");
fetchMock.mockResolvedValue(new Response(null, { status: 200 }));
const cb = jest.fn();
const res = await request.fetchAsStream("foo", {}, cb);
expect(res).toBeNull();
expect(cb).not.toHaveBeenCalled();
});

it("fetchAsStream throws ExecutionsExhaustedException", async () => {
mockPythonEnv.getResolvedConfigValue
.mockReturnValueOnce("key")
.mockReturnValueOnce("instance");
fetchMock.mockResolvedValue(
new Response('{"detail":"stop"}', { status: 402, statusText: "Limit" }),
);
await expect(request.fetchAsStream("foo", {}, jest.fn())).rejects.toThrow(
"stop",
);
});
});
20 changes: 20 additions & 0 deletions src/test/suite/commandProcessExecution.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,4 +119,24 @@ describe("CommandProcessExecution Tests", () => {
const result = await execution.complete();
expect(result.stderr.trim()).toBe("error");
});

it("should stream output to terminal", async () => {
const execution = factory.createCommandProcessExecution({
command: process.platform === "win32" ? "cmd" : "echo",
args: process.platform === "win32" ? ["/c", "echo stream"] : ["stream"],
});
when(mockTerminal.log(anything())).thenReturn();
const result = await execution.completeWithTerminalOutput();
expect(result.stdout.trim()).toBe("stream");
verify(mockTerminal.log(anything())).atLeast(1);
});

it("should format text by replacing newlines", () => {
const execution = new CommandProcessExecution(
instance(mockTerminal),
"",
[],
);
expect(execution.formatText("a\n\nb")).toBe("a\r\n\rb");
Copy link
Preview

Copilot AI May 20, 2025

Choose a reason for hiding this comment

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

Document the expected transformation logic of the formatText function for newline characters, ensuring its behavior is clear in the test.

Copilot uses AI. Check for mistakes.

});
});
35 changes: 34 additions & 1 deletion src/test/suite/dbtTerminal.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,19 @@ describe("DBTTerminal Test Suite", () => {
sendTelemetryError: jest.fn(),
};

jest.spyOn(console, "log").mockImplementation(() => {});
jest.spyOn(console, "debug").mockImplementation(() => {});
jest.spyOn(console, "info").mockImplementation(() => {});
jest.spyOn(console, "warn").mockImplementation(() => {});
jest.spyOn(console, "error").mockImplementation(() => {});

terminal = new DBTTerminal(mockTelemetry);
// @ts-ignore - Manually set the output channel
terminal.outputChannel = mockOutputChannel;
});

afterEach(() => {
jest.clearAllMocks();
jest.restoreAllMocks();
});

it("should log messages with proper formatting", () => {
Expand All @@ -42,6 +48,28 @@ describe("DBTTerminal Test Suite", () => {
expect(mockOutputChannel.info).toHaveBeenCalledWith(message, []);
});

it("logLine writes message and newline", () => {
const spy = jest.spyOn(terminal, "log");
terminal.logLine("abc");
expect(spy).toHaveBeenCalledWith("abc");
expect(spy).toHaveBeenCalledWith("\r\n");
});

it("logNewLine logs CRLF", () => {
const spy = jest.spyOn(terminal, "log");
terminal.logNewLine();
expect(spy).toHaveBeenCalledWith("\r\n");
});

it("should write to terminal when active", () => {
const message = "hello";
// @ts-ignore - access private
terminal.terminal = { show: jest.fn(), dispose: jest.fn() } as any;
const fireSpy = jest.spyOn((terminal as any).writeEmitter, "fire");
terminal.log(message);
expect(fireSpy).toHaveBeenCalledWith(message);
});

it("should send telemetry on info messages", () => {
const name = "test_event";
const message = "Test info message";
Expand Down Expand Up @@ -105,6 +133,11 @@ describe("DBTTerminal Test Suite", () => {
);
});

it("should handle string errors", () => {
terminal.error("name", "msg", "oops");
expect(mockOutputChannel.error).toHaveBeenCalledWith("name:msg:oops", []);
});

it("should format and log blocks with horizontal rules", () => {
const block = ["Line 1", "Line 2", "Line 3"];
terminal.logBlock(block);
Expand Down
Loading