-
-
Notifications
You must be signed in to change notification settings - Fork 0
chore: post-Jules-triage followups #338
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: main
Are you sure you want to change the base?
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 | ||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,94 @@ | ||||||||||||||||||||||||||||||||||||||||||
| import './vscode_mock_setup'; | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| import { describe, it, beforeEach, afterEach } from 'node:test'; | ||||||||||||||||||||||||||||||||||||||||||
| import assert from 'node:assert'; | ||||||||||||||||||||||||||||||||||||||||||
| import { mockVscode } from './mocks/vscode'; | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| // SupportsWriteMode in databaseModel.ts is evaluated at module-load time from | ||||||||||||||||||||||||||||||||||||||||||
| // `vsc.env.remoteName` and `CurrentExtension?.extensionKind`. Set both so the | ||||||||||||||||||||||||||||||||||||||||||
| // read-write path resolves to true regardless of which test loads databaseModel first. | ||||||||||||||||||||||||||||||||||||||||||
| (mockVscode as any).ExtensionKind = { Workspace: 2, UI: 1 }; | ||||||||||||||||||||||||||||||||||||||||||
| mockVscode.env.remoteName = 'remote'; | ||||||||||||||||||||||||||||||||||||||||||
| (mockVscode as any).extensions = { | ||||||||||||||||||||||||||||||||||||||||||
| getExtension: () => ({ extensionKind: 2 }) | ||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+10
to
+14
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. 🧩 Analysis chain🏁 Script executed: # Check if the file exists and read its content
if [ -f "tests/unit/editorController.test.ts" ]; then
echo "=== File exists ==="
wc -l tests/unit/editorController.test.ts
echo ""
echo "=== First 30 lines (including imports and lines 10-14) ==="
head -n 30 tests/unit/editorController.test.ts
else
echo "File not found"
fiRepository: zknpr/SQLite-Explorer Length of output: 1441 🏁 Script executed: # Also check for vscode_mock_setup import
rg -n "vscode_mock_setup" tests/unit/editorController.test.ts | head -5Repository: zknpr/SQLite-Explorer Length of output: 97 🏁 Script executed: # Search for any other uses of ExtensionKind, env.remoteName, or extensions in the test file
rg -n "(ExtensionKind|remoteName|extensions)" tests/unit/editorController.test.tsRepository: zknpr/SQLite-Explorer Length of output: 289 Use Direct assignment to VS Code API fields like Suggested patch-(mockVscode as any).ExtensionKind = { Workspace: 2, UI: 1 };
-mockVscode.env.remoteName = 'remote';
-(mockVscode as any).extensions = {
- getExtension: () => ({ extensionKind: 2 })
-};
+Object.defineProperty(mockVscode as object, 'ExtensionKind', {
+ value: { Workspace: 2, UI: 1 },
+ writable: true,
+ configurable: true,
+});
+Object.defineProperty(mockVscode.env as object, 'remoteName', {
+ value: 'remote',
+ writable: true,
+ configurable: true,
+});
+Object.defineProperty(mockVscode as object, 'extensions', {
+ value: { getExtension: () => ({ extensionKind: 2 }) },
+ writable: true,
+ configurable: true,
+});📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| // workerFactory imports threadPool which crashes due to bare `import.meta.env` at | ||||||||||||||||||||||||||||||||||||||||||
| // module load. Mock it in require cache before editorController is required. | ||||||||||||||||||||||||||||||||||||||||||
| const moduleCache = require('module')._cache; | ||||||||||||||||||||||||||||||||||||||||||
| const workerFactoryPath = require.resolve('../../src/workerFactory'); | ||||||||||||||||||||||||||||||||||||||||||
| moduleCache[workerFactoryPath] = { | ||||||||||||||||||||||||||||||||||||||||||
| id: workerFactoryPath, | ||||||||||||||||||||||||||||||||||||||||||
| filename: workerFactoryPath, | ||||||||||||||||||||||||||||||||||||||||||
| loaded: true, | ||||||||||||||||||||||||||||||||||||||||||
| exports: { | ||||||||||||||||||||||||||||||||||||||||||
| createDatabaseConnection: () => {} | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| const editorControllerModule = require('../../src/editorController'); | ||||||||||||||||||||||||||||||||||||||||||
| const { registerEditorProvider, DatabaseViewerProvider, DatabaseEditorProvider } = editorControllerModule; | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| describe('registerEditorProvider', () => { | ||||||||||||||||||||||||||||||||||||||||||
| type RegisterCall = { viewType: string; provider: unknown; options: unknown }; | ||||||||||||||||||||||||||||||||||||||||||
| let calls: RegisterCall[]; | ||||||||||||||||||||||||||||||||||||||||||
| let originalRegister: any; | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| beforeEach(() => { | ||||||||||||||||||||||||||||||||||||||||||
| calls = []; | ||||||||||||||||||||||||||||||||||||||||||
| originalRegister = (mockVscode.window as any).registerCustomEditorProvider; | ||||||||||||||||||||||||||||||||||||||||||
| (mockVscode.window as any).registerCustomEditorProvider = ( | ||||||||||||||||||||||||||||||||||||||||||
| viewType: string, | ||||||||||||||||||||||||||||||||||||||||||
| provider: unknown, | ||||||||||||||||||||||||||||||||||||||||||
| options: unknown | ||||||||||||||||||||||||||||||||||||||||||
| ) => { | ||||||||||||||||||||||||||||||||||||||||||
| calls.push({ viewType, provider, options }); | ||||||||||||||||||||||||||||||||||||||||||
| return { dispose: () => {} }; | ||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| afterEach(() => { | ||||||||||||||||||||||||||||||||||||||||||
| (mockVscode.window as any).registerCustomEditorProvider = originalRegister; | ||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| const ctx = { extensionUri: mockVscode.Uri.file('/ext'), globalState: { get: () => undefined } } as any; | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| it('registers DatabaseViewerProvider when readOnly=true regardless of verified', () => { | ||||||||||||||||||||||||||||||||||||||||||
| registerEditorProvider('sqlite-explorer.view', ctx, undefined, null, { verified: true, readOnly: true }); | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| assert.strictEqual(calls.length, 1); | ||||||||||||||||||||||||||||||||||||||||||
| assert.strictEqual(calls[0].viewType, 'sqlite-explorer.view'); | ||||||||||||||||||||||||||||||||||||||||||
| assert.ok(calls[0].provider instanceof DatabaseViewerProvider); | ||||||||||||||||||||||||||||||||||||||||||
| assert.ok(!(calls[0].provider instanceof DatabaseEditorProvider)); | ||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| it('registers DatabaseViewerProvider when verified=false', () => { | ||||||||||||||||||||||||||||||||||||||||||
| registerEditorProvider('sqlite-explorer.view', ctx, undefined, null, { verified: false, readOnly: false }); | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| assert.strictEqual(calls.length, 1); | ||||||||||||||||||||||||||||||||||||||||||
| assert.ok(calls[0].provider instanceof DatabaseViewerProvider); | ||||||||||||||||||||||||||||||||||||||||||
| assert.ok(!(calls[0].provider instanceof DatabaseEditorProvider)); | ||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| it('registers DatabaseEditorProvider when verified=true and readOnly=false (write mode enabled)', () => { | ||||||||||||||||||||||||||||||||||||||||||
| registerEditorProvider('sqlite-explorer.edit', ctx, undefined, null, { verified: true, readOnly: false }); | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| assert.strictEqual(calls.length, 1); | ||||||||||||||||||||||||||||||||||||||||||
| assert.strictEqual(calls[0].viewType, 'sqlite-explorer.edit'); | ||||||||||||||||||||||||||||||||||||||||||
| // DatabaseEditorProvider extends DatabaseViewerProvider, so check the more specific type. | ||||||||||||||||||||||||||||||||||||||||||
| assert.ok(calls[0].provider instanceof DatabaseEditorProvider); | ||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| it('passes retainContextWhenHidden=false in webview options', () => { | ||||||||||||||||||||||||||||||||||||||||||
| registerEditorProvider('sqlite-explorer.view', ctx, undefined, null, { verified: true, readOnly: true }); | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| const options = calls[0].options as { webviewOptions: { retainContextWhenHidden: boolean } }; | ||||||||||||||||||||||||||||||||||||||||||
| assert.strictEqual(options.webviewOptions.retainContextWhenHidden, false); | ||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| it('returns a disposable from registerCustomEditorProvider', () => { | ||||||||||||||||||||||||||||||||||||||||||
| const result = registerEditorProvider('sqlite-explorer.view', ctx, undefined, null, { verified: true, readOnly: true }); | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| assert.ok(result && typeof result.dispose === 'function'); | ||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -11,8 +11,8 @@ describe('RPC', () => { | |||||||||||||
| }); | ||||||||||||||
|
|
||||||||||||||
| it('should handle invocations', (context) => { | ||||||||||||||
| const methods = { | ||||||||||||||
| add: (a: number, b: number) => a + b | ||||||||||||||
| const methods: Record<string, (...args: unknown[]) => unknown> = { | ||||||||||||||
| add: (...args: unknown[]) => (args[0] as number) + (args[1] as number) | ||||||||||||||
| }; | ||||||||||||||
|
Comment on lines
+14
to
16
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. This change reduces type safety by using I suggest restoring the safer function implementation while keeping the explicit type annotation:
Suggested change
|
||||||||||||||
|
|
||||||||||||||
| let response: any = null; | ||||||||||||||
|
|
||||||||||||||
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.
🧩 Analysis chain
🏁 Script executed:
Repository: zknpr/SQLite-Explorer
Length of output: 4829
🌐 Web query:
What parameter types does sql.js bind() method accept for prepared statement parameters?💡 Result:
The bind method in sql.js accepts two primary parameter types to bind values to a prepared statement [1][2]. Depending on the type of parameters defined in your SQL statement, you should provide either an array or an object [1][2]: 1. Array: Use an array when binding values to anonymous (positional) parameters (e.g.,?,?NNN) [1][2]. The order of values in the array corresponds to the order of parameters in the SQL statement [2]. 2. Object: Use an object when binding values to named parameters (e.g.,:VVV,$VVV) [1][2]. The keys of the object must match the parameter names defined in the SQL string [2]. Note that the prefixes (:, @, $ ) must be included in the object keys [1][2]. Mapping of JavaScript types to SQLite types: The library automatically handles the mapping of JavaScript data types to their corresponding SQLite types as follows [1][3]: JavaScript Type | SQLite Type --- | --- number | REAL, INTEGER boolean | INTEGER string | TEXT Array, Uint8Array | BLOB null | NULL The bind method resets the statement before applying new values [1][2]. If you pass null, the method does nothing and returns true [1][3].
@VVV,Citations:
Add
booleanto theCellValuetype definition to match sql.js parameter support.The interface declarations constrain sql.js parameters to
CellValue[](string | number | null | Uint8Array), but sql.js's official documentation shows the bind() method accepts boolean values, which it maps to SQLite INTEGER. TheCellValuetype should includebooleanto accurately reflect the library's capabilities and prevent type safety gaps when boolean data is used with prepared statements.🤖 Prompt for AI Agents