-
Notifications
You must be signed in to change notification settings - Fork 116
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
base: master
Are you sure you want to change the base?
Add unit tests #1669
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -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, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -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(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid casting to
Suggested change
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 failureCode scanning / CodeQL Insecure temporary file High test
Insecure creation of file in
the os temp dir Error loading related location Loading |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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); | |
| } |
Check failure
Code scanning / CodeQL
Insecure temporary file High test
the os temp dir
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 months 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:
- Import the
tmplibrary. - Replace the
path.join(os.tmpdir(), "up.txt")logic withtmp.fileSync()to securely create a temporary file. - Use the
nameproperty of the returned object fromtmp.fileSync()to get the file path. - Ensure the temporary file is removed after use by calling the
removeCallbackmethod provided bytmp.
-
Copy modified lines R15-R16 -
Copy modified lines R234-R235 -
Copy modified line R238 -
Copy modified line R240 -
Copy modified lines R247-R248 -
Copy modified line R251 -
Copy modified line R254
| @@ -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(); | ||
| }); |
-
Copy modified lines R1402-R1403
| @@ -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" | ||
| }, |
| Package | Version | Security advisories |
| tmp (npm) | 0.2.3 | None |
There was a problem hiding this comment.
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 tsLength 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
uploadToS3cases 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.
| 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.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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"); | ||
|
||
| }); | ||
| }); | ||
There was a problem hiding this comment.
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.