MLE-25617 Two more client-level TS methods#1041
Conversation
rjrudin
requested review from
BillFarber,
anu3990 and
stevebio
as code owners
|
Copyright Validation Results ✅ Valid Files
✅ All files have valid copyright headers! |
There was a problem hiding this comment.
Pull request overview
This PR adds TypeScript support for two client-level methods: setLogger and setAuthToken. It includes both compile-time type checking tests and runtime validation tests to ensure the methods work correctly in TypeScript environments.
Key Changes
- Added compile-time type checking tests for
setLogger(with logger object and level string) andsetAuthToken - Added runtime validation tests for
setLoggerwith both logger object and level string configurations
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| test-typescript/dbclient-setLogger-setAuthToken-compile.test.ts | New file containing compile-time type checking tests for setLogger and setAuthToken methods |
| test-typescript/dbclient-convenience-runtime.test.ts | Added runtime validation tests for setLogger with level string and logger object |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| import type { DatabaseClient } from 'marklogic'; | ||
|
|
||
| const marklogic = require('../lib/marklogic.js'); |
There was a problem hiding this comment.
Mixing import type with require is inconsistent. Consider using import for the module on line 10 instead of require, or remove the type-only import and use type assertions instead. This ensures consistency in import style throughout the file.
| const marklogic = require('../lib/marklogic.js'); | |
| import * as marklogic from '../lib/marklogic.js'; |
| }); | ||
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
[nitpick] Remove excessive blank lines. There should be at most one blank line between test cases for consistency with the rest of the file.
| it('should use setLogger with a level string', async function() { | ||
| // Set logger to 'info' level | ||
| client.setLogger('info'); | ||
|
|
||
| // Verify client is still functional after setting logger | ||
| const result = await client.probe(testUri).result(); | ||
| result.should.have.property('exists'); | ||
| result.should.have.property('uri'); | ||
| }); |
There was a problem hiding this comment.
This test verifies that the client remains functional after calling setLogger, but doesn't validate that the logger is actually being used or that log messages are being produced. Consider capturing log output or verifying logger behavior to ensure setLogger actually configures logging correctly.
| it('should use setLogger with a logger object', async function() { | ||
| // Create a simple logger object | ||
| const testLogger = { | ||
| debug: (msg: string) => console.log('DEBUG:', msg), | ||
| info: (msg: string) => console.log('INFO:', msg), | ||
| warn: (msg: string) => console.log('WARN:', msg), | ||
| error: (msg: string) => console.log('ERROR:', msg) | ||
| }; | ||
|
|
||
| // Set logger with object | ||
| client.setLogger(testLogger, false); | ||
|
|
||
| // Verify client is still functional after setting logger | ||
| const result = await client.probe(testUri).result(); | ||
| result.should.have.property('exists'); | ||
| result.should.have.property('uri'); | ||
| }); |
There was a problem hiding this comment.
This test verifies that the client remains functional after calling setLogger with a custom logger object, but doesn't validate that the custom logger is actually being invoked. Consider tracking whether the logger methods are called to ensure setLogger properly integrates the custom logger.
rjrudin
force-pushed
the
feature/25617-ts-dbclient
branch
from
f9dfe16 to
3e40374
Compare
rjrudin
force-pushed
the
feature/25617-ts-dbclient
branch
from
3e40374 to
a5442e3
Compare
1 participant
No description provided.