Add unit tests #1669
Add unit tests #1669
Conversation
WalkthroughThis update adds a new module alias to the Jest configuration and introduces or extends several Jest test suites to improve coverage for modules handling credentials, streaming, file uploads, command execution, terminal output, and utility functions. No changes were made to exported or public entity declarations. Changes
Sequence Diagram(s)sequenceDiagram
participant TestSuite as Jest Test Suite
participant AltimateRequest as AltimateRequest
participant CommandProcessExecution as CommandProcessExecution
participant DBTTerminal as DBTTerminal
participant Utils as Utility Functions
TestSuite->>AltimateRequest: Test credential message, preview features, stream reading, file upload, error handling
TestSuite->>CommandProcessExecution: Test output streaming and text formatting
TestSuite->>DBTTerminal: Test logging, terminal output, error handling
TestSuite->>Utils: Test utility functions, custom exceptions, and helpers
Possibly related PRs
Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm warn config production Use Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| .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
| .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
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 16 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:
- 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.
| @@ -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(); | ||
| }); |
| @@ -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 |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed everything up to 8f98a71 in 2 minutes and 38 seconds. Click for details.
- Reviewed
525lines of code in5files - Skipped
0files when reviewing. - Skipped posting
2draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. jest.config.js:18
- Draft comment:
Verify that the '@extension' alias uses a clear, non‐abbreviated name consistent with project conventions. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
2. src/test/suite/utils.test.ts:225
- Draft comment:
Monkey-patching global.Date can be brittle; consider using a dedicated date-mocking library and ensure global Date is reliably restored. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% While the suggestion to use a dedicated date-mocking library is technically valid, the current implementation is actually quite reasonable. It properly saves and restores the original Date, is scoped to a single test, and is clear in its intent. The current approach is not causing any actual problems and the suggested change would be more of an opinion than a necessary fix. The comment does raise a valid point about brittleness - monkey-patching globals can be dangerous. The test could potentially interfere with other tests if the restore fails. However, the current implementation follows good practices by properly restoring the global state and keeping the change isolated. The suggested alternative wouldn't necessarily be better, just different. This comment is more of a style preference than a clear issue that needs fixing. The current implementation is reasonable and follows good practices.
Workflow ID: wflow_3TwxdJxenWQQeBzv
You can customize 
|
|
||
| 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.
Avoid casting to any for readStreamToBlob; consider adding an explicit return type in its definition to ease refactoring.
| 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.
| selection: { active: { line: 2, character: 5 } }, | ||
| }; | ||
| expect(getCurrentlySelectedModelNameInYamlConfig()).toBe("model_b"); | ||
| }); |
There was a problem hiding this comment.
After setting window.activeTextEditor in the YAML config test, consider cleaning it up to avoid state leakage across tests.
There was a problem hiding this comment.
Pull Request Overview
This PR significantly expands the unit test coverage by introducing comprehensive tests for utility functions, AltimateRequest, DBTTerminal, and CommandProcessExecution. Key changes include:
- Adding tests for stream processing, deep equality, file handling, and configuration utilities.
- Enhancing DBTTerminal tests with extended logging and telemetry verifications.
- Incorporating detailed AltimateRequest tests for credential messaging, preview feature handling, stream-to-blob conversion, file uploads, and error handling.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/test/suite/utils.test.ts | Tests for various utility functions related to streams, objects, and configuration. |
| src/test/suite/dbtTerminal.test.ts | Expanded tests covering terminal logging methods and telemetry. |
| src/test/suite/commandProcessExecution.test.ts | New tests for command output streaming and newline formatting. |
| src/test/suite/altimate.test.ts | Comprehensive tests for AltimateRequest including credential messages, preview features, and S3 uploads. |
| mockPythonEnv.getResolvedConfigValue | ||
| .mockReturnValueOnce("key") | ||
| .mockReturnValueOnce("instance"); | ||
| expect(request.getCredentialsMessage()).toBeUndefined(); |
There was a problem hiding this comment.
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.
| "", | ||
| [], | ||
| ); | ||
| expect(execution.formatText("a\n\nb")).toBe("a\r\n\rb"); |
There was a problem hiding this comment.
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.
| 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); |
There was a problem hiding this comment.
Consider encapsulating temporary file creation and removal within a try/finally block to ensure cleanup even if an assertion fails.
| 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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/test/suite/utils.test.ts (4)
45-67: Consider adding error handling tests for streams.While the current tests verify successful processing of both Node.js and Web streams, there's no testing for error cases. For complete coverage, add a test that verifies error propagation when a stream emits an error.
it("processStreamResponse propagates errors from Node streams", async () => { const stream = new Readable({ read() { this.emit('error', new Error('Test error')); } }); const cb = jest.fn(); await expect(processStreamResponse(stream, cb)).rejects.toThrow('Test error'); expect(cb).not.toHaveBeenCalled(); });
69-75: Expand deepEqual tests with more edge cases.The current tests cover basic nested objects, but consider adding tests for:
- Arrays with same elements in different order
- Objects with null/undefined values
- Comparison with non-object values
it("deepEqual handles edge cases correctly", () => { // Arrays with different order expect(deepEqual([1, 2], [2, 1])).toBe(false); // Objects with null/undefined expect(deepEqual({a: null}, {a: null})).toBe(true); expect(deepEqual({a: undefined}, {a: null})).toBe(false); // Primitive comparisons expect(deepEqual(null, null)).toBe(true); expect(deepEqual(undefined, undefined)).toBe(true); expect(deepEqual(null, undefined)).toBe(false); });
103-107: Consider testing the primary path for getFirstWorkspacePath.While the test covers the fallback behavior when no workspace is present, it would be valuable to also test the primary path where a workspace is available.
it("getFirstWorkspacePath returns first workspace path when available", () => { (workspace.workspaceFolders as any) = [ { uri: { fsPath: "/workspace/path" } } ]; expect(getFirstWorkspacePath()).toBe("/workspace/path"); });
109-123: Add tests for error handling in YAML parsing.The getCurrentlySelectedModelNameInYamlConfig test only covers the happy path. Consider adding tests for error conditions like invalid YAML, cursor position outside any model, or when no models are defined.
it("getCurrentlySelectedModelNameInYamlConfig handles invalid YAML", () => { const invalidYaml = `models:\n - name: model_a\n invalid: - name:`; const document = { languageId: "yaml", getText: () => invalidYaml, offsetAt: jest.fn() } as any; (window as any).activeTextEditor = { document, selection: { active: { line: 2, character: 5 } } }; // Should return empty string on parsing error expect(getCurrentlySelectedModelNameInYamlConfig()).toBe(""); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
jest.config.js(1 hunks)src/test/suite/altimate.test.ts(2 hunks)src/test/suite/commandProcessExecution.test.ts(1 hunks)src/test/suite/dbtTerminal.test.ts(3 hunks)src/test/suite/utils.test.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/test/suite/utils.test.ts (3)
src/utils.ts (18)
processStreamResponse(138-172)deepEqual(174-202)getStringSizeInMb(382-398)isQuotedIdentifier(237-256)getColumnNameByCase(204-212)isColumnNameEqual(214-235)getCurrentlySelectedModelNameInYamlConfig(413-466)isRelationship(286-296)isAcceptedValues(298-305)getColumnTestConfigFromYml(307-369)getExternalProjectNamesFromDbtLoomConfig(258-284)setupWatcherHandler(93-100)notEmpty(77-79)arrayEquals(81-83)debounce(85-91)stripANSI(113-118)getFormattedDateTime(371-380)isEnclosedWithinCodeBlock(20-75)src/test/mock/vscode.ts (2)
workspace(74-91)Position(21-26)src/exceptions/rateLimitException.ts (1)
RateLimitException(1-9)
🪛 GitHub Check: CodeQL
src/test/suite/altimate.test.ts
[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.
🔇 Additional comments (24)
jest.config.js (1)
20-20: LGTM: Good addition to module mappingThe addition of the
@extensionmodule alias correctly maps to the src/modules.ts file, which will simplify imports in test files.src/test/suite/commandProcessExecution.test.ts (2)
123-132: Good test for terminal output streamingThis test properly verifies that command output is correctly streamed to the terminal and confirms both the returned output and that the terminal's log method is called.
134-141: Good test for formatText implementationThis test verifies the text formatting functionality that replaces newlines with carriage return + newline sequences for proper terminal display.
src/test/suite/dbtTerminal.test.ts (6)
30-35: Good practice: Mocking console methodsMocking these console methods prevents test output from being cluttered with console logs, making test results cleaner and easier to read.
42-42: Better test isolation with restoreAllMocksChanging from
clearAllMocks()torestoreAllMocks()is an improvement as it fully restores the original implementation of mocked functions, preventing test interference.
51-56: Good test for logLine methodThis test verifies that logLine correctly handles both the message and the newline sequence.
58-62: Good test for logNewLine methodThis test ensures the logNewLine method properly logs the carriage return + line feed sequence.
64-71: Good test for terminal writing functionalityThis test verifies that the terminal's write emitter is correctly triggered when the terminal is active.
136-139: Good test for string error handlingThis test verifies that the error method correctly handles string errors, ensuring proper formatting.
src/test/suite/altimate.test.ts (5)
13-18: Good addition of required importsThe new imports properly support the expanded test cases for streaming, file operations, and error handling.
179-201: Good test coverage for credentials message logicThese tests thoroughly verify the
getCredentialsMessagemethod's behavior across different configuration scenarios, ensuring appropriate messages are returned based on available credentials.
203-221: Good test coverage for preview features handlingThese tests verify both scenarios for the
handlePreviewFeaturesmethod - when credentials are missing and when they exist, ensuring the message is only shown when needed.
223-228: Good test for stream data collectionThis test verifies that the
readStreamToBlobmethod correctly accumulates data from a readable stream.
257-318: Good test coverage for error handling in fetchAsStreamThese tests thoroughly verify the error handling logic in the
fetchAsStreammethod for different HTTP status codes and error conditions, including:
- 404 NotFound errors
- 429 RateLimiting with retry header
- 403 Forbidden/invalid credentials
- Empty response body handling
- 402 Payment required / executions exhausted
This comprehensive coverage will help ensure robust error handling.
src/test/suite/utils.test.ts (10)
1-39: Comprehensive imports and organization structure.The test file includes all necessary imports from Jest, Node.js modules, VSCode API, utility functions, and custom exceptions. Good organization helps set up proper test coverage for the extensive utility functions in the codebase.
77-82: Good test for multibyte character handling in size calculation.The test properly verifies the size calculation for both ASCII and multibyte characters, using toBeCloseTo for floating-point comparison. This is important for correctly measuring string sizes in memory.
84-101: Well-structured tests for configuration-dependent functions.These tests effectively mock the workspace configuration and verify both the isQuotedIdentifier and getColumnNameByCase/isColumnNameEqual functions with different configurations. Good coverage of the configuration-dependent behavior.
125-157: Good coverage of type guards and test config extraction.The tests for isRelationship, isAcceptedValues, and getColumnTestConfigFromYml are well-structured and cover multiple scenarios, including different test types and array value matching regardless of order.
159-171: Excellent file system testing approach.Using temporary files and directories for testing file system operations is a good practice. The test correctly cleans up resources and tests both successful file reading and handling of missing files.
173-183: Effective event handler testing.The test for setupWatcherHandler properly verifies that event handlers are wired correctly by checking that callbacks are invoked and disposables are returned.
185-190: Complete custom exceptions validation.The test verifies that both custom exceptions (RateLimitException and NoCredentialsError) correctly expose their properties, which is crucial for error handling throughout the application.
192-212: Well-implemented utility function tests.The tests for notEmpty, arrayEquals, and debounce are thorough and validate the expected behavior, including edge cases. The debounce test particularly makes good use of Jest's fake timers.
214-234: Good testing of string manipulation and date formatting.The tests for extendErrorWithSupportLinks, stripANSI, and getFormattedDateTime properly verify the behavior of these utility functions. The date formatting test correctly mocks the global Date object and restores it afterward.
236-249: Thorough code block detection testing.The test for isEnclosedWithinCodeBlock effectively verifies the detection of code blocks within a document by testing both positive and negative cases with well-mocked document and position data.
| 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); | ||
| }); |
There was a problem hiding this comment.
💡 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.
Summary
Testing
npm test --silent -- --coverageImportant
Add extensive unit tests for
AltimateRequest,DBTTerminal,CommandProcessExecution, and utility functions, and updatejest.config.jsfor module mapping.AltimateRequestinaltimate.test.tsto cover credential messages, preview features, stream handling, and S3 uploads.DBTTerminalindbtTerminal.test.tsto cover logging, telemetry, and terminal management.CommandProcessExecutionincommandProcessExecution.test.tsto cover command execution, environment variables, and output streaming.utils.test.tsto cover stream processing, object comparison, and configuration handling.jest.config.jsto map@extensionto<rootDir>/src/modules.ts.This description was created by
for 8f98a71. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
Tests
Chores