Conversation
![flatfile-nullify[bot]](https://avatars.githubusercontent.com/u/31830338?s=60&v=4){kind=small}
Nullify Code Vulnerabilities5 findings found in this pull request
You can find a list of all findings here |
carlbrugger
force-pushed
the
feat/vitest
branch
from
0989538 to
4184908
Compare
|
|
||
| let env = {} | ||
|
|
||
| if (fs.existsSync(envPath)) { |
There was a problem hiding this comment.
Nullify Code Language: JavaScript 🟡 HIGH Severity CWE-22
Improper limitation of a pathname to a restricted directory ('Path Traversal')
The application dynamically constructs file or path information. If the path
information comes from user-supplied input, it could be abused to read sensitive files,
access other users' data, or aid in exploitation to gain further system access.
User input should never be used in constructing paths or files for interacting
with the filesystem. This includes filenames supplied by user uploads or downloads.
If possible, consider hashing user input or using unique values and
use path.normalize to resolve and validate the path information
prior to processing any file functionality.
Example using path.normalize and not allowing direct user input:
// User input, saved only as a reference
// id is a randomly generated UUID to be used as the filename
const userData = {userFilename: userSuppliedFilename, id: crypto.randomUUID()};
// Restrict all file processing to this directory only
const basePath = '/app/restricted/';
// Create the full path, but only use our random generated id as the filename
const joinedPath = path.join(basePath, userData.id);
// Normalize path, removing any '..'
const fullPath = path.normalize(joinedPath);
// Verify the fullPath is contained within our basePath
if (!fullPath.startsWith(basePath)) {
console.log("Invalid path specified!");
}
// Process / work with file
// ...
For more information on path traversal issues see OWASP:
https://owasp.org/www-community/attacks/Path_Traversal
Read more:
https://cwe.mitre.org/data/definitions/22.html
⚡ Here's how you might fix this potential vulnerability
The initial code was vulnerable to path traversal attacks because it used user-supplied input to construct a file path without properly sanitizing it. The modified code mitigates this vulnerability by sanitizing the 'mode' parameter before using it to construct a file path. It does this by using the 'path.basename' function, which ensures that the 'mode' parameter does not contain any directory traversal characters. This ensures that the constructed file path always points to a file in the intended directory, preventing path traversal attacks.
autoFixesExperimental
Sanitize the 'mode' parameter before using it to construct a file path
Lines 6-7
const sanitizedMode = path.basename(mode)
const envPath = path.resolve('../../', `.env.${sanitizedMode}`)
const defaultEnvPath = path.resolve('../../', '.env')poweredByNullify
Reply with /nullify to interact with me like another developer
(you will need to refresh the page for updates)
| let env = {} | ||
|
|
||
| if (fs.existsSync(envPath)) { | ||
| env = { ...env, ...dotenv.parse(fs.readFileSync(envPath)) } |
There was a problem hiding this comment.
Nullify Code Language: JavaScript 🟡 HIGH Severity CWE-22
Improper limitation of a pathname to a restricted directory ('Path Traversal')
The application dynamically constructs file or path information. If the path
information comes from user-supplied input, it could be abused to read sensitive files,
access other users' data, or aid in exploitation to gain further system access.
User input should never be used in constructing paths or files for interacting
with the filesystem. This includes filenames supplied by user uploads or downloads.
If possible, consider hashing user input or using unique values and
use path.normalize to resolve and validate the path information
prior to processing any file functionality.
Example using path.normalize and not allowing direct user input:
// User input, saved only as a reference
// id is a randomly generated UUID to be used as the filename
const userData = {userFilename: userSuppliedFilename, id: crypto.randomUUID()};
// Restrict all file processing to this directory only
const basePath = '/app/restricted/';
// Create the full path, but only use our random generated id as the filename
const joinedPath = path.join(basePath, userData.id);
// Normalize path, removing any '..'
const fullPath = path.normalize(joinedPath);
// Verify the fullPath is contained within our basePath
if (!fullPath.startsWith(basePath)) {
console.log("Invalid path specified!");
}
// Process / work with file
// ...
For more information on path traversal issues see OWASP:
https://owasp.org/www-community/attacks/Path_Traversal
Read more:
https://cwe.mitre.org/data/definitions/22.html
Reply with /nullify to interact with me like another developer
(you will need to refresh the page for updates)
| env = { ...env, ...dotenv.parse(fs.readFileSync(envPath)) } | ||
| } | ||
|
|
||
| if (fs.existsSync(defaultEnvPath)) { |
There was a problem hiding this comment.
Nullify Code Language: JavaScript 🟡 HIGH Severity CWE-22
Improper limitation of a pathname to a restricted directory ('Path Traversal')
The application dynamically constructs file or path information. If the path
information comes from user-supplied input, it could be abused to read sensitive files,
access other users' data, or aid in exploitation to gain further system access.
User input should never be used in constructing paths or files for interacting
with the filesystem. This includes filenames supplied by user uploads or downloads.
If possible, consider hashing user input or using unique values and
use path.normalize to resolve and validate the path information
prior to processing any file functionality.
Example using path.normalize and not allowing direct user input:
// User input, saved only as a reference
// id is a randomly generated UUID to be used as the filename
const userData = {userFilename: userSuppliedFilename, id: crypto.randomUUID()};
// Restrict all file processing to this directory only
const basePath = '/app/restricted/';
// Create the full path, but only use our random generated id as the filename
const joinedPath = path.join(basePath, userData.id);
// Normalize path, removing any '..'
const fullPath = path.normalize(joinedPath);
// Verify the fullPath is contained within our basePath
if (!fullPath.startsWith(basePath)) {
console.log("Invalid path specified!");
}
// Process / work with file
// ...
For more information on path traversal issues see OWASP:
https://owasp.org/www-community/attacks/Path_Traversal
Read more:
https://cwe.mitre.org/data/definitions/22.html
Reply with /nullify to interact with me like another developer
(you will need to refresh the page for updates)
| } | ||
|
|
||
| if (fs.existsSync(defaultEnvPath)) { | ||
| env = { ...env, ...dotenv.parse(fs.readFileSync(defaultEnvPath)) } |
There was a problem hiding this comment.
Nullify Code Language: JavaScript 🟡 HIGH Severity CWE-22
Improper limitation of a pathname to a restricted directory ('Path Traversal')
The application dynamically constructs file or path information. If the path
information comes from user-supplied input, it could be abused to read sensitive files,
access other users' data, or aid in exploitation to gain further system access.
User input should never be used in constructing paths or files for interacting
with the filesystem. This includes filenames supplied by user uploads or downloads.
If possible, consider hashing user input or using unique values and
use path.normalize to resolve and validate the path information
prior to processing any file functionality.
Example using path.normalize and not allowing direct user input:
// User input, saved only as a reference
// id is a randomly generated UUID to be used as the filename
const userData = {userFilename: userSuppliedFilename, id: crypto.randomUUID()};
// Restrict all file processing to this directory only
const basePath = '/app/restricted/';
// Create the full path, but only use our random generated id as the filename
const joinedPath = path.join(basePath, userData.id);
// Normalize path, removing any '..'
const fullPath = path.normalize(joinedPath);
// Verify the fullPath is contained within our basePath
if (!fullPath.startsWith(basePath)) {
console.log("Invalid path specified!");
}
// Process / work with file
// ...
For more information on path traversal issues see OWASP:
https://owasp.org/www-community/attacks/Path_Traversal
Read more:
https://cwe.mitre.org/data/definitions/22.html
Reply with /nullify to interact with me like another developer
(you will need to refresh the page for updates)
carlbrugger
force-pushed
the
feat/vitest
branch
3 times, most recently
from
1bd8200 to
b58a6fc
Compare
carlbrugger
force-pushed
the
feat/bundlers
branch
2 times, most recently
from
d27f1c0 to
449301d
Compare
carlbrugger
force-pushed
the
feat/vitest
branch
2 times, most recently
from
0e26000 to
c4949d2
Compare
WalkthroughThis pull request introduces a series of minor version updates for over 70 Flatfile plugins and utilities, indicating backward-compatible changes. Key updates include the transition from Jest to Vitest as the testing framework, along with the introduction of new configuration files for Vitest across various plugins. The build process has also been modified to use Changes
Possibly related PRs
Suggested reviewers
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 21
🧹 Outside diff range and nitpick comments (47)
plugins/dedupe/src/keep.first.logic.spec.ts (1)
Line range hint 5-10: Enhance test clarity and coverage.
Several improvements could make this test more maintainable and comprehensive:
- The test description should be more specific about what behavior is being tested
- The
uniquesSet can useconstinstead ofletas it's not reassigned - Consider adding test cases for edge cases (empty records, missing email field)
- Document the test data structure from
fake.data
Here's a suggested improvement:
describe('keepFirst()', () => {
- it('keepFirst()', () => {
- let uniques = new Set()
+ it('should remove duplicate records keeping the first occurrence based on email field', () => {
+ const uniques = new Set()
const removeThese = keepFirst(records, 'email', uniques)
expect(removeThese).toEqual(['recordId:5'])
})
+
+ it('should handle empty record array', () => {
+ const uniques = new Set()
+ const removeThese = keepFirst([], 'email', uniques)
+ expect(removeThese).toEqual([])
+ })
+
+ it('should handle records with missing email field', () => {
+ const uniques = new Set()
+ const incompleteRecords = [
+ { recordId: '1', email: 'test@example.com' },
+ { recordId: '2' }
+ ]
+ const removeThese = keepFirst(incompleteRecords, 'email', uniques)
+ expect(removeThese).toEqual([])
+ })
})config/vitest/src/index.js (2)
3-16: Consider adding JSDoc documentation for the configuration function.
The configuration function would benefit from documentation explaining:
- The purpose and expected values of the
modeparameter - The significance of the 30-second timeout
- Why plugins array is empty
Example documentation:
+/**
+ * Creates a Vitest configuration object
+ * @param {Object} options - Configuration options
+ * @param {string} options.mode - The environment mode (e.g., 'test', 'development')
+ * @returns {import('vitest').ConfigEnv} Vitest configuration object
+ */
const config = ({ mode }) => {1-18: Consider documenting migration impact and extending configuration.
As part of the Jest to Vitest migration:
- Consider adding a README.md in the config directory documenting:
- Key differences between Jest and Vitest configurations
- Migration guide for teams using this configuration
- Common gotchas and troubleshooting tips
- The configuration structure allows for easy extension. Consider adding:
- Coverage reporting configuration
- Custom matchers support
- Test file patterns and ignore patterns
plugins/dedupe/src/keep.last.logic.spec.ts (1)
Line range hint 5-10: Enhance test coverage with additional test cases.
While the basic functionality is tested, consider adding the following test cases for better coverage:
- Empty records array
- Records with missing email field
- Records with null/undefined email values
- Multiple records with the same email
- Case sensitivity handling
Here's a suggested expansion of the test suite:
describe('keepLast()', () => {
it('keeps the last record when duplicates exist', () => {
let uniques = new Set()
const removeThese = keepLast(records, 'email', uniques)
expect(removeThese).toEqual(['recordId:1'])
})
it('handles empty records array', () => {
let uniques = new Set()
const removeThese = keepLast([], 'email', uniques)
expect(removeThese).toEqual([])
})
it('handles records with missing email field', () => {
let uniques = new Set()
const recordsWithMissing = [
{ recordId: '1', data: { name: 'Test' } },
{ recordId: '2', data: { email: 'test@example.com' } }
]
const removeThese = keepLast(recordsWithMissing, 'email', uniques)
expect(removeThese).toEqual([])
})
it('handles records with null/undefined email values', () => {
let uniques = new Set()
const recordsWithNull = [
{ recordId: '1', data: { email: null } },
{ recordId: '2', data: { email: undefined } },
{ recordId: '3', data: { email: 'test@example.com' } }
]
const removeThese = keepLast(recordsWithNull, 'email', uniques)
expect(removeThese).toEqual([])
})
})plugins/dxp-configure/src/shim.target.spec.ts (1)
Line range hint 4-49: Consider adding test cases for edge cases and error scenarios.
While the current test cases cover the main functionality well, consider adding tests for:
- Error handling when context or payload is undefined
- Edge cases with empty or malformed sheet slugs
- Boundary conditions for action names with multiple colons
Would you like me to help generate additional test cases for these scenarios?
plugins/openapi-schema/src/setup.factory.openapi.spec.ts (2)
Line range hint 4-33: Consider extracting test fixtures to a separate file.
The expectedConfig object is quite large and could be moved to a separate test fixtures file to improve maintainability and reusability across other test cases.
+ // fixtures/pet-schema.fixture.ts
+ export const expectedPetConfig = {
+ workbooks: [
+ {
+ name: 'Webhook Example',
+ sheets: [
+ {
+ name: 'Pet',
+ slug: 'Pet',
+ fields: [
+ {
+ label: 'id',
+ constraints: [{ type: 'required' }],
+ key: 'id',
+ type: 'number',
+ },
+ // ... rest of the fields
+ ],
+ },
+ ],
+ },
+ ],
+ }Based on the gathered information, I can now provide the final response:
Error handling exists but test coverage needs improvement
The implementation in setup.factory.ts already includes proper error handling:
- HTTP error handling with status code checks
- Schema validation checks
- Try-catch blocks with error propagation
- Proper error messages for failed requests
However, the test file lacks:
- Error scenario coverage
- Local schema fixtures
- Timeout configuration
Required changes:
- Create a local fixture at
plugins/openapi-schema/src/__fixtures__/webhook-example.json - Add test cases for HTTP errors, invalid schemas, and missing models
- Set appropriate timeout for the async test
🔗 Analysis chain
Line range hint 34-47: Verify external dependency and add error handling test cases.
The test relies on an external OpenAPI schema from GitHub. This introduces potential test reliability issues and security concerns.
Consider:
- Downloading and storing the schema locally to prevent test failures due to external dependencies
- Adding test cases for error scenarios (invalid schema, network issues)
- Adding timeout handling for the async operation
Let's verify the external schema's availability and contents:
Let me gather more information about error handling and timeout handling in the test file and related files.
Let me check one more thing to see how the external schema is fetched and if there's any error handling in place.
Let me check the exact error handling implementation in the setup factory.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the external OpenAPI schema
# Expected: 200 status code and valid JSON response
curl -I https://raw.githubusercontent.com/OAI/OpenAPI-Specification/main/examples/v3.1/webhook-example.json
# Search for similar external URL usage that might need the same treatment
rg -l "raw.githubusercontent.com" --type ts
Length of output: 1437
Script:
#!/bin/bash
# Check the test file for error handling patterns
ast-grep --pattern 'it($_, async $_)' plugins/openapi-schema/src/setup.factory.openapi.spec.ts
# Check for error handling in the implementation
ast-grep --pattern 'generateSetup($_)'
# Check for timeout configuration in test setup
rg "timeout|jest.setTimeout" plugins/openapi-schema/src/setup.factory.openapi.spec.ts
# Check the implementation of generateSetup for error handling
rg -A 5 "export.*generateSetup"
Length of output: 5556
Script:
#!/bin/bash
# Check the implementation for fetch/axios usage and error handling
rg -A 10 "source.*githubusercontent" plugins/openapi-schema/src/
# Look for any fetch or HTTP client implementation
ast-grep --pattern 'fetch($$$)' plugins/openapi-schema/src/
ast-grep --pattern 'axios.$_($$$)' plugins/openapi-schema/src/
# Check for any error handling utilities
rg -l "try|catch|throw" plugins/openapi-schema/src/
Length of output: 392
Script:
#!/bin/bash
# Check the complete implementation of setup.factory.ts
cat plugins/openapi-schema/src/setup.factory.ts
# Look for test fixtures or local schema files
fd -e json -e yaml webhook example schema --exec file {} \;
Length of output: 6463
utils/common/src/slugify.spec.ts (1)
Line range hint 4-42: Consider adding more test cases for enhanced coverage.
The current test suite provides good coverage of basic scenarios and edge cases. Consider adding these additional test cases to make it more comprehensive:
describe('slugify', () => {
// ... existing tests ...
+
+ it('should handle case sensitivity', () => {
+ expect(slugify('Hello World')).toBe('hello-world')
+ })
+
+ it('should preserve numbers', () => {
+ expect(slugify('hello 123 world')).toBe('hello-123-world')
+ })
+
+ it('should handle non-ASCII characters', () => {
+ expect(slugify('héllo wörld')).toBe('hello-world')
+ })
+
+ it('should handle very long strings', () => {
+ const longString = 'a'.repeat(256)
+ expect(slugify(longString)).toBe('a'.repeat(256))
+ })
})validate/email/src/validate.email.plugin.ts (2)
32-39: Consider extracting the error message string to a constant.
While the reformatting improves readability, the hard-coded error message string could make maintenance more difficult. Consider extracting it to a named constant at the module level.
+const DISPOSABLE_EMAIL_ERROR = 'Disposable email addresses are not allowed'
+
export function validateEmail(config: EmailValidationConfig) {
return recordHook(config.sheetSlug || '**', (record: FlatfileRecord) => {
// ... existing code ...
if (
- validationResult.error ===
- 'Disposable email addresses are not allowed'
+ validationResult.error === DISPOSABLE_EMAIL_ERROR
) {
errorMessage = errorMessages.disposable || validationResult.error
}Plugin functionality requires test coverage
The codebase shows:
- Comprehensive unit tests exist for email validation utilities (
validate.email.utils.spec.ts) - No tests found for the plugin functionality (
validate.email.plugin.ts) - Missing test coverage for:
- Record hook validation logic
- Error message configuration
- Field validation iteration
- Integration with FlatfileRecord
🔗 Analysis chain
Line range hint 13-46: Verify test coverage during Jest to Vitest transition.
Since this is part of a testing framework transition, ensure that all existing test cases have been migrated and that the error handling paths are properly covered in the new Vitest tests.
Let me gather more specific information about the email validation tests.
Let me check for the plugin test coverage specifically.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for corresponding test files and their content
# Expect: Test file exists with proper Vitest syntax and comprehensive test coverage
# Look for test files
fd -e test.ts -e spec.ts validate.email.plugin
# Check test content for proper Vitest usage
rg -l "describe|test|expect|vi\." $(fd -e test.ts -e spec.ts validate.email.plugin)
Length of output: 7991
Script:
#!/bin/bash
# Search for email validation test files and their content
rg -l "validate.email" validate/email/
# Show content of any test files found
rg "describe|test|expect|vi\." validate/email/ -A 5
Length of output: 8216
Script:
#!/bin/bash
# Search for tests specific to the email validation plugin functionality
rg "validate.email.plugin" validate/email/ -A 5
# Also check for any e2e tests
fd -e e2e.spec.ts . validate/email/
Length of output: 169
convert/translate/src/convert.translate.plugin.spec.ts (1)
Line range hint 1-50: Consider adding more comprehensive test coverage
While the migration to Vitest is clean, the test suite could benefit from additional test cases:
- Error handling scenarios
- Different language combinations
- Empty field handling
- Rate limiting scenarios
Would you like me to provide examples of these additional test cases?
support/common-utils/src/async.map.spec.ts (2)
Line range hint 38-47: Consider improving timeout test robustness
The timeout test could be enhanced in several ways:
- Extract the timeout values to named constants for better readability
- Add a larger buffer between the timeout and the delayed resolution to prevent flaky tests
- Consider testing parallel execution timeout behavior
Here's a suggested improvement:
- const LONG_TIMEOUT = 100
+ const TEST_TIMEOUT_MS = 100
+ const DELAYED_RESOLUTION_MS = TEST_TIMEOUT_MS + 50 // Larger buffer
const mockSlowCallback = vi
.fn()
.mockImplementation(
- () => new Promise((resolve) => setTimeout(resolve, LONG_TIMEOUT + 10))
+ () => new Promise((resolve) => setTimeout(resolve, DELAYED_RESOLUTION_MS))
)
await expect(
- asyncMap(mockArray, mockSlowCallback, { timeout: LONG_TIMEOUT })
+ asyncMap(mockArray, mockSlowCallback, { timeout: TEST_TIMEOUT_MS })
).rejects.toThrow('timed out')Line range hint 14-47: Consider adding tests for additional edge cases
While the current test coverage is good, consider adding tests for:
- Error handling during parallel execution
- Partial success scenarios where some items succeed and others fail
- Array items that resolve in different orders when running in parallel
Example test case structure:
it('handles errors in parallel execution', async () => {
const mockCallbackWithMixedResults = vi.fn()
.mockImplementationOnce(() => Promise.resolve('SUCCESS'))
.mockImplementationOnce(() => Promise.reject(new Error('FAIL')))
.mockImplementationOnce(() => Promise.resolve('SUCCESS'));
await expect(
asyncMap(['a', 'b', 'c'], mockCallbackWithMixedResults, { parallel: true })
).rejects.toThrow('FAIL');
});plugins/xlsx-extractor/src/utils.spec.ts (1)
Line range hint 6-38: Consider adding tests for edge cases
While the current test coverage is good, consider adding these scenarios to make it more robust:
- Handling of null/undefined inputs
- Processing of special characters beyond asterisks
- Performance verification with larger datasets
- Documentation of the function's contract/behavior
Example test case:
it('should handle null/undefined inputs', () => {
const input = { a: undefined, b: null, c: 'value' }
const expected = { a: 'empty', b: 'empty', c: 'value' }
expect(prependNonUniqueHeaderColumns(input)).toEqual(expected)
})plugins/autocast/src/autocast.plugin.spec.ts (3)
12-23: Consider adding edge cases for string casting
While the current test cases cover the basic scenarios well, consider adding tests for edge cases:
- Empty string
- null/undefined values
- Special characters
- Very long strings
+ it('should handle edge cases', () => {
+ expect(castString('')).toBe('')
+ expect(castString(null)).toBe('')
+ expect(castString(undefined)).toBe('')
+ expect(castString('🚀')).toBe('🚀')
+ })24-32: Consider additional number format test cases
The current tests cover basic number formats well. Consider adding tests for:
- Negative decimals
- Scientific notation
- Currency symbols
- Percentage values
+ test.each([
+ ['-1.1', -1.1],
+ ['1e5', 100000],
+ ['$1,234.56', 1234.56],
+ ['50%', 0.5]
+ ])('should handle various number formats', (input, expected) => {
+ expect(castNumber(input)).toBe(expected)
+ })39-50: Consider additional date format test cases
While the current date format coverage is good, consider adding tests for:
- Invalid dates
- Different timezone formats
- Leap year edge cases
- International date formats (DD-MM-YYYY)
+ test.each([
+ ['2023-02-31', 'Invalid date'],
+ ['2024-02-29', 'Thu, 29 Feb 2024 00:00:00 GMT'],
+ ['2023-08-16T00:00:00+05:30', 'Tue, 15 Aug 2023 18:30:00 GMT'],
+ ['16-08-2023', 'Wed, 16 Aug 2023 00:00:00 GMT']
+ ])('should handle edge case dates', (input, expected) => {
+ if (expected === 'Invalid date') {
+ expect(() => castDate(input)).toThrow(expected)
+ } else {
+ expect(castDate(input)).toBe(expected)
+ }
+ })enrich/summarize/src/summarize.plugin.spec.ts (1)
Line range hint 41-52: Remove debug console.log statement.
The test cases look good, but there's a debug console.log statement that should be removed.
Apply this diff:
it('should extract key phrases from content', () => {
const content = 'The quick brown fox jumps over the lazy dog.'
const keyPhrases = extractKeyPhrases(content)
- console.log('keyPhrases', keyPhrases)
expect(keyPhrases[0]).toContain('quick brown fox')
expect(keyPhrases[1]).toContain('lazy dog')
})extract/markdown/src/parser.spec.ts (3)
Line range hint 17-19: Consider making test data paths more robust
The relative path construction could be fragile if the project structure changes.
Consider creating a test utilities file with a helper function:
// testUtils.ts
export const getTestFilePath = (filename: string) =>
path.join(__dirname, '..', 'samples', filename);
// In your test:
const buffer = fs.readFileSync(getTestFilePath(file));Line range hint 44-57: Enhance error handling test coverage
While the basic error handling cases are covered, consider adding more specific assertions.
Consider enhancing the error handling tests:
test('should handle errorHandling option', () => {
const filePath = path.join(__dirname, '..', 'samples', 'lenient_tables.md')
const buffer = fs.readFileSync(filePath)
const strictOptions: MarkdownExtractorOptions = { errorHandling: 'strict' }
expect(() => parseBuffer(buffer, strictOptions)).toThrow()
// Add specific error message assertion
expect(() => parseBuffer(buffer, strictOptions)).toThrow(/specific error message/)
const lenientOptions: MarkdownExtractorOptions = {
errorHandling: 'lenient',
}
const result = parseBuffer(buffer, lenientOptions)
// Add assertions for the lenient parsing result
expect(result).toBeDefined()
expect(Object.keys(result).length).toBeGreaterThan(0)
})
// Add test for invalid errorHandling option
test('should throw on invalid errorHandling option', () => {
const buffer = Buffer.from('# test')
expect(() =>
parseBuffer(buffer, { errorHandling: 'invalid' as any })
).toThrow(/Invalid errorHandling option/)
})Line range hint 8-57: Consider adding snapshot tests for complex tables
For complex table structures, snapshot tests could help catch unexpected changes in parsing behavior.
Add a snapshot test for complex tables:
test('should maintain consistent parsing for complex tables', () => {
const buffer = fs.readFileSync(
path.join(__dirname, '..', 'samples', 'complex_table.md')
)
const result = parseBuffer(buffer, { maxTables: Infinity })
expect(result).toMatchSnapshot()
})utils/common/src/logging.helper.spec.ts (1)
Line range hint 5-69: Consider refactoring test setup and cleanup
While the tests are working correctly, consider these improvements for better maintainability:
- Use
beforeEach/afterEachhooks to handle spy setup and cleanup - Add more descriptive test names that explain the expected behavior
Here's how you could refactor the first test case:
describe('logging', () => {
let logSpy: SpyInstance;
let warnSpy: SpyInstance;
let errorSpy: SpyInstance;
beforeEach(() => {
logSpy = vi.spyOn(global.console, 'log');
warnSpy = vi.spyOn(global.console, 'warn');
errorSpy = vi.spyOn(global.console, 'error');
});
afterEach(() => {
logSpy.mockRestore();
warnSpy.mockRestore();
errorSpy.mockRestore();
});
test('log() should use console.log for INFO level messages', () => {
log('@flatfile/util-common', 'Log an INFO message');
expect(logSpy).toHaveBeenCalledWith(
'[@flatfile/util-common]:[INFO] Log an INFO message'
);
});
// Similar refactoring for other test cases...
});plugins/constraints/src/external.sheet.constraint.e2e.spec.ts (1)
Line range hint 82-93: Consider expanding test coverage
While the current test case covers the basic error scenario, consider adding tests for:
- Valid cases where both name and email are present
- Edge cases with empty strings or whitespace
- Different combinations of name/email presence
Example additional test case:
it('accepts records with both name and email', async () => {
await createRecords(sheetId, [{
name: 'John Doe',
age: 30,
email: 'john@example.com'
}])
await listener.waitFor('commit:created')
const records = await getRecords(sheetId)
expect(records[0].values['name'].messages).toBeUndefined()
expect(records[0].values['email'].messages).toBeUndefined()
})plugins/job-handler/src/job.handler.e2e.spec.ts (1)
83-84: Consider enhancing error handling test coverage.
While the basic error case is covered, consider:
- Removing the console.log in the job:failed listener as it's not necessary for testing
- Adding more specific assertions about the error object structure
Replace:
listener.on('job:failed', (event) => {
console.log(event.topic, event.payload.job)
})with:
listener.on('job:failed', (event) => {
expect(event.payload.job).toBeDefined()
expect(event.topic).toBe('space:configure')
})enrich/sentiment/src/enrich.sentiment.plugin.ts (2)
Line range hint 4-14: Consider enhancing config validation and type safety.
The EnrichSentimentConfig interface could benefit from stronger validation and more specific types:
export interface EnrichSentimentConfig {
sheetSlug: string
- textFields: string[]
+ textFields: readonly string[] // Prevent accidental modification
automaticValidation: boolean
errorMessages?: {
- required?: string
- invalid?: string
- disposable?: string
- domain?: string
+ // Remove unused error message types
+ analysis?: string
+ validation?: string
}
}Line range hint 67-77: Consider optimizing multiple field processing.
The current implementation processes each field sequentially. For better performance with multiple fields, consider:
- Batch processing fields
- Caching sentiment analysis results for repeated values
Example optimization:
// Cache for sentiment results
const sentimentCache = new Map<string, SentimentResult>();
// Batch process fields
const processFields = async (fields: string[], record: Record) => {
const uniqueValues = new Set(fields.map(f => String(record.get(f))));
// Process unique values
for (const value of uniqueValues) {
if (!sentimentCache.has(value)) {
const result = performEnrichSentiment(value, '');
sentimentCache.set(value, result);
}
}
// Apply cached results to fields
for (const field of fields) {
const value = String(record.get(field));
const result = sentimentCache.get(value);
// Apply result to record...
}
};plugins/constraints/src/external.constraint.e2e.spec.ts (1)
Line range hint 102-124: Consider moving test data to a separate file
The test schema and data at the bottom of the file could be moved to a separate test fixtures file to improve maintainability and reusability.
Consider creating a new file __fixtures__/simple-value.ts:
import { Flatfile } from '@flatfile/api'
export const defaultSimpleValueSchema: Array<Flatfile.Property> = [
{
key: 'name',
type: 'string',
constraints: [
{ type: 'external', validator: 'test', config: { flag: true } },
],
},
{
key: 'age',
type: 'number',
},
]
export const defaultSimpleValueData = [
{
name: 'John Doe',
age: 1,
},
{
name: 'Jane Doe',
age: 1,
},
]plugins/json-schema/src/setup.factory.spec.ts (3)
Line range hint 4-39: Consider expanding test coverage with additional JSON Schema scenarios.
The current schema covers basic types well, but consider adding test cases for:
- Additional data types (boolean, null)
- Validation constraints (pattern, minimum, maximum)
- Required properties
- Array constraints (minItems, maxItems)
Example addition:
const extendedJsonSchema = {
...simpleJsonSchema,
required: ['stringColumn'],
properties: {
...simpleJsonSchema.properties,
booleanColumn: {
type: 'boolean',
description: 'A column for booleans!'
},
constrainedString: {
type: 'string',
pattern: '^[A-Z]{2}$',
description: 'Two uppercase letters'
},
constrainedNumber: {
type: 'number',
minimum: 0,
maximum: 100,
description: 'Number between 0 and 100'
}
}
}Line range hint 41-89: Document schema transformation rules.
The expected result shows specific transformation rules that would benefit from documentation:
- Arrays are converted to enums with empty options
- Nested objects are flattened using underscore notation
- Integer type is mapped to number type
Consider adding comments or updating the test description to explain these transformation rules.
Example documentation:
/**
* Test cases for JSON Schema to Flatfile transformations:
* 1. Arrays are converted to enums (e.g., string[] → enum)
* 2. Nested objects are flattened (e.g., object.nested → object_nested)
* 3. JSON Schema types are mapped to Flatfile types:
* - integer → number
* - array → enum
*/Line range hint 91-107: Enhance test robustness with additional test cases.
The current test only covers the happy path. Consider adding:
- Error cases (invalid schema, missing required fields)
- Edge cases (empty schema, schema with only required fields)
- Use exact matching instead of partial matching to catch unexpected properties
Example additional test cases:
it('should handle empty schema', async () => {
const results = await generateSetup({
workbooks: [{
name: 'Empty Schema',
sheets: [{ source: { type: 'object', properties: {} } }]
}]
})
expect(results.workbooks[0].sheets[0].fields).toHaveLength(0)
})
it('should throw error for invalid schema', async () => {
await expect(generateSetup({
workbooks: [{
name: 'Invalid Schema',
sheets: [{ source: { type: 'invalid' } }]
}]
})).rejects.toThrow()
})plugins/automap/src/automap.plugin.e2e.spec.ts (1)
55-55: Consider test optimization opportunities
A few suggestions for improving the test implementation:
- Consider extracting the 90_000ms timeout to a constant to follow DRY principles
- The long timeout (90s) might indicate potential performance bottlenecks in the test. Consider investigating if this can be optimized.
Example improvement:
const TEST_TIMEOUT = 90_000;
// Then use it in the tests
it('correctly modifies a value', async () => {
// ... test code ...
}, TEST_TIMEOUT)Also applies to: 94-94
plugins/webhook-event-forwarder/src/forward-webhook.e2e.spec.ts (1)
67-69: Add type annotation for the callback parameter.
The callback function's data parameter lacks type information, which could lead to type-related issues.
Consider adding proper type annotations:
- callback = vi.fn((data: any, event: FlatfileEvent) => {
+ type WebhookResponse = Record<string, unknown>;
+ callback = vi.fn((data: WebhookResponse, event: FlatfileEvent) => {enrich/geocode/src/enrich.geocode.plugin.spec.ts (1)
2-2: LGTM! Consider adding type safety to mock responses.
The migration to Vitest is implemented correctly. The imports, mocking setup, and test cleanup are all properly configured.
Consider adding type safety to the mock responses by creating an interface for the expected response structure:
interface GeocodeResponse {
status: 'OK' | 'ZERO_RESULTS';
results: Array<{
geometry: {
location: { lat: number; lng: number };
};
formatted_address: string;
address_components: Array<{
types: string[];
long_name: string;
}>;
}>;
}Also applies to: 5-5, 11-11
plugins/xml-extractor/src/parser.spec.ts (3)
Line range hint 90-132: Consider adding more edge cases to xmlToJson tests.
While the current tests cover basic functionality, consider adding test cases for:
- Empty XML document
- Malformed XML handling
- Special characters in element values and attributes
- Deep nested structures
- XML with CDATA sections
Example test case:
test('handles special characters and empty elements', () => {
const xml = `
<root>
<item>
<name><Special & Chars></name>
<empty></empty>
<![CDATA[<text>with special characters</text>]]>
</item>
</root>
`
expect(xmlToJson(xml)).toEqual([{
name: '<Special & Chars>',
empty: '',
text: 'with special characters'
}])
})Line range hint 134-157: Enhance headersFromObjectList test coverage.
The current test verifies basic functionality, but consider adding test cases for:
- Empty object list
- Objects with different sets of keys
- Nested objects with conflicting paths
Example test cases:
test('handles objects with different keys', () => {
const json = [
{ a: '1', b: '2' },
{ b: '3', c: '4' },
{ a: '5', c: '6' }
]
expect(headersFromObjectList(json)).toEqual(['a', 'b', 'c'])
})
test('handles empty object list', () => {
expect(headersFromObjectList([])).toEqual([])
})Add missing error handling test cases for XML parsing
The implementation shows error handling for invalid XML (throws 'No root xml object found'), but there are no corresponding test cases. The review comment's suggestion is valid as the test suite lacks coverage for error scenarios.
Consider adding these test cases:
- Empty buffer handling
- Invalid XML structure validation
- Missing root element detection
- XML parsing errors
Example test case structure remains valid as suggested in the review comment.
🔗 Analysis chain
Line range hint 159-192: Verify error handling in parseBuffer function.
While the duplicate header test is thorough, we should verify error handling and edge cases:
Consider adding test cases for:
- Empty buffer
- Invalid XML buffer
- Custom sheet names
- Buffer encoding issues
Example test case:
test('handles invalid XML buffer', () => {
const invalidXml = Buffer.from('<root><unclosed>')
expect(() => parseBuffer(invalidXml)).toThrow('XML parsing error')
})
test('handles empty buffer', () => {
expect(() => parseBuffer(Buffer.from(''))).toThrow('Empty buffer')
})Let me gather more specific information about error handling in the XML extractor.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if parseBuffer has error handling tests elsewhere
rg -l "parseBuffer.*throw|catch|error" --type ts
Length of output: 3946
Script:
#!/bin/bash
# Check for existing error handling tests in parser.spec.ts
rg "test\(.*error|test\(.*invalid|test\(.*empty" plugins/xml-extractor/src/parser.spec.ts -A 5
# Look for error handling implementation in parser.ts
rg "throw|catch|error" plugins/xml-extractor/src/parser.ts -A 2
Length of output: 274
plugins/xlsx-extractor/src/parser.spec.ts (1)
Line range hint 9-13: Consider centralizing test file paths
The test file paths are currently hardcoded in multiple places. Consider extracting these paths to a central configuration or constants file to improve maintainability.
+// In a new file: src/test/constants.ts
+export const TEST_FILES = {
+ BASIC_XLSX: '../ref/test-basic.xlsx',
+ EMPTY_ROWS_XLSX: '../ref/test-empty-rows.xlsx'
+} as const
-const buffer: Buffer = fs.readFileSync(
- path.join(__dirname, '../ref/test-basic.xlsx')
-)
+const buffer: Buffer = fs.readFileSync(
+ path.join(__dirname, TEST_FILES.BASIC_XLSX)
+)Also applies to: 67-69
utils/testing/src/test.helpers.ts (1)
39-46: Add JSDoc documentation for the setupDriver function.
The function looks good, but it's missing documentation. Consider adding JSDoc to maintain consistency with other exported functions in the file.
+/**
+ * Sets up a PubSub driver with the configured environment ID.
+ *
+ * @throws {Error} If FLATFILE_ENVIRONMENT_ID is not defined
+ * @returns {PubSubDriver} A configured PubSub driver instance
+ */
export function setupDriver() {
const environmentId = getEnvironmentId()
if (!environmentId) {
throw new Error('environmentId is not defined')
}
return new PubSubDriver(environmentId)
}plugins/graphql-schema/src/generate.field.spec.ts (2)
Line range hint 94-113: Remove duplicate test case
The test case "should handle fields with undefined descriptions" is duplicated. Remove one of the identical test blocks to maintain a clean test suite.
- it('should handle fields with undefined descriptions', () => {
- const field = { name: 'age', type: { kind: 'SCALAR', name: 'Int' } }
- const expected = {
- key: 'age',
- label: 'age',
- type: 'number',
- description: '',
- constraints: [],
- }
- expect(generateField(field, 'Person')).toEqual(expected)
- })Line range hint 4-156: Consider grouping related test cases
While the test coverage is comprehensive, consider organizing related test cases using describe blocks for better readability and maintenance. For example:
describe('generateField', () => {
+ describe('scalar types', () => {
it('should generate a base property for scalar types', () => {
// ... existing test
})
it('should generate a string property for scalar string types', () => {
// ... existing test
})
it('should generate a property for ID scalar types', () => {
// ... existing test
})
+ })
+
+ describe('non-null types', () => {
it('should include required constraint for NON_NULL scalar types', () => {
// ... existing test
})
it('should handle recursive NON_NULL object types', () => {
// ... existing test
})
+ })
+
+ describe('object types', () => {
it('should generate a reference property for object types', () => {
// ... existing test
})
+ })
+
+ describe('error cases', () => {
it('should handle unsupported field types by returning null', () => {
// ... existing test
})
it('should return null and log for unrecognized field types', () => {
// ... existing test
})
+ })
})import/faker/src/faker.plugin.spec.ts (1)
Line range hint 7-46: Consider adding explicit return type for mock implementation
The FlatfileClient mock implementation is well-structured and correctly uses Vitest mocking syntax. However, consider adding explicit return types to ensure type safety.
-vi.mock('@flatfile/api', () => ({
+vi.mock('@flatfile/api', (): { FlatfileClient: any } => ({plugins/json-extractor/src/parser.spec.ts (3)
3-3: Standardize test method usage across the file.
The file imports both it and test from Vitest but uses them inconsistently throughout the test suite. While both are valid, it's better to stick to one style for consistency.
Consider standardizing to use test throughout:
- it('has headers', () => {
+ test('has headers', () => {Line range hint 11-150: Consider adding error case tests.
The current test suite thoroughly validates successful JSON parsing but doesn't cover error scenarios. Consider adding tests for:
- Malformed JSON
- Empty JSON
- Missing required fields
Would you like me to provide example test cases for these scenarios?
Line range hint 13-14: Consider adding type assertions.
The test validates the structure but doesn't ensure type safety. Consider adding type assertions to verify the returned type matches the expected WorkbookCapture interface.
Example:
const capture = parseBuffer(buffer) as WorkbookCapture;
expect(capture.Sheet1).toHaveProperty('headers');
expect(capture.Sheet1).toHaveProperty('data');plugins/record-hook/src/RecordHook.e2e.spec.ts (3)
48-49: Consider adding type safety to console spies
While the spy implementation is correct, consider adding explicit return types for better type safety:
-const logSpy = vi.spyOn(global.console, 'log')
-const logErrorSpy = vi.spyOn(global.console, 'error')
+const logSpy = vi.spyOn(global.console, 'log') as SpyInstance<[message?: any, ...optionalParams: any[]], void>
+const logErrorSpy = vi.spyOn(global.console, 'error') as SpyInstance<[message?: any, ...optionalParams: any[]], void>Line range hint 58-78: Consider using a type-safe property factory
The current type assertions could be made more maintainable using a factory function:
// Create a type-safe property factory
const createProperty = <T extends keyof Flatfile.Property>(
key: string,
type: T,
config?: Partial<Flatfile.Property[T]['config']>
): Flatfile.Property[T] => ({
key,
type,
config,
}) as Flatfile.Property[T]
// Usage in tests
const properties = [
createProperty('firstName', 'string'),
createProperty('lastName', 'string'),
createProperty('email', 'string'),
createProperty('age', 'number'),
createProperty('alive', 'boolean'),
createProperty('category', 'enum', {
options: [
{ value: 'one', label: 'One' },
{ value: 'two', label: 'Two' },
],
}),
]Line range hint 265-450: Consider enhancing test robustness
While the test coverage is comprehensive, consider these improvements:
- Add explicit timeouts for all long-running tests, not just some
- Include more descriptive assertion messages
Example improvement:
-expect(records[0].valid).toBeTruthy()
+expect(records[0].valid).toBeTruthy('Record should be valid when enum value is valid')Also consider adding the timeout to all tests that interact with the API:
-it('sets primitive values', async () => {
+it('sets primitive values', async () => {
// ... test implementation
-})
+}, 15_000)📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (60)
.github/workflows/test.ymlis excluded by!**/*.ymlconfig/vitest/package.jsonis excluded by!**/*.jsonconvert/currency/package.jsonis excluded by!**/*.jsonconvert/translate/package.jsonis excluded by!**/*.jsonconvert/what3words/package.jsonis excluded by!**/*.jsonenrich/geocode/package.jsonis excluded by!**/*.jsonenrich/gpx/package.jsonis excluded by!**/*.jsonenrich/sentiment/package.jsonis excluded by!**/*.jsonenrich/summarize/package.jsonis excluded by!**/*.jsonexport/pivot-table/package.jsonis excluded by!**/*.jsonextract/html-table/package.jsonis excluded by!**/*.jsonextract/markdown/package.jsonis excluded by!**/*.jsonimport/faker/package.jsonis excluded by!**/*.jsonimport/llm-records/package.jsonis excluded by!**/*.jsonimport/rss/package.jsonis excluded by!**/*.jsonpackage-lock.jsonis excluded by!**/package-lock.json,!**/*.jsonpackage.jsonis excluded by!**/*.jsonplugins/autocast/package.jsonis excluded by!**/*.jsonplugins/automap/package.jsonis excluded by!**/*.jsonplugins/constraints/package.jsonis excluded by!**/*.jsonplugins/dedupe/package.jsonis excluded by!**/*.jsonplugins/delimiter-extractor/package.jsonis excluded by!**/*.jsonplugins/dxp-configure/package.jsonis excluded by!**/*.jsonplugins/export-workbook/package.jsonis excluded by!**/*.jsonplugins/foreign-db-extractor/package.jsonis excluded by!**/*.jsonplugins/graphql-schema/package.jsonis excluded by!**/*.jsonplugins/job-handler/package.jsonis excluded by!**/*.jsonplugins/json-extractor/package.jsonis excluded by!**/*.jsonplugins/json-schema/package.jsonis excluded by!**/*.jsonplugins/merge-connection/package.jsonis excluded by!**/*.jsonplugins/openapi-schema/package.jsonis excluded by!**/*.jsonplugins/pdf-extractor/package.jsonis excluded by!**/*.jsonplugins/psv-extractor/package.jsonis excluded by!**/*.jsonplugins/record-hook/package.jsonis excluded by!**/*.jsonplugins/rollout/package.jsonis excluded by!**/*.jsonplugins/space-configure/package.jsonis excluded by!**/*.jsonplugins/sql-ddl-converter/package.jsonis excluded by!**/*.jsonplugins/tsv-extractor/package.jsonis excluded by!**/*.jsonplugins/view-mapped/package.jsonis excluded by!**/*.jsonplugins/webhook-egress/package.jsonis excluded by!**/*.jsonplugins/webhook-event-forwarder/package.jsonis excluded by!**/*.jsonplugins/xlsx-extractor/package.jsonis excluded by!**/*.jsonplugins/xml-extractor/package.jsonis excluded by!**/*.jsonplugins/yaml-schema/package.jsonis excluded by!**/*.jsonplugins/zip-extractor/package.jsonis excluded by!**/*.jsonsupport/common-utils/package.jsonis excluded by!**/*.jsonturbo.jsonis excluded by!**/*.jsonutils/common/package.jsonis excluded by!**/*.jsonutils/extractor/package.jsonis excluded by!**/*.jsonutils/fetch-schema/package.jsonis excluded by!**/*.jsonutils/file-buffer/package.jsonis excluded by!**/*.jsonutils/response-rejection/package.jsonis excluded by!**/*.jsonutils/testing/package.jsonis excluded by!**/*.jsonvalidate/boolean/package.jsonis excluded by!**/*.jsonvalidate/date/package.jsonis excluded by!**/*.jsonvalidate/email/package.jsonis excluded by!**/*.jsonvalidate/isbn/package.jsonis excluded by!**/*.jsonvalidate/number/package.jsonis excluded by!**/*.jsonvalidate/phone/package.jsonis excluded by!**/*.jsonvalidate/string/package.jsonis excluded by!**/*.json
📒 Files selected for processing (157)
- .changeset/five-hounds-wash.md (1 hunks)
- config/vitest/src/index.js (1 hunks)
- config/vitest/src/utils.js (1 hunks)
- convert/currency/jest.config.cjs (0 hunks)
- convert/currency/src/convert.currency.plugin.spec.ts (1 hunks)
- convert/currency/src/convert.currency.plugin.ts (7 hunks)
- convert/currency/vitest.config.ts (1 hunks)
- convert/translate/jest.config.cjs (0 hunks)
- convert/translate/src/convert.translate.plugin.spec.ts (2 hunks)
- convert/translate/vitest.config.ts (1 hunks)
- convert/what3words/jest.config.cjs (0 hunks)
- convert/what3words/vitest.config.ts (1 hunks)
- enrich/geocode/jest.config.cjs (0 hunks)
- enrich/geocode/src/enrich.geocode.plugin.spec.ts (6 hunks)
- enrich/geocode/src/index.ts (1 hunks)
- enrich/geocode/vitest.config.ts (1 hunks)
- enrich/gpx/jest.config.cjs (0 hunks)
- enrich/gpx/src/enrich.gpx.plugin.spec.ts (3 hunks)
- enrich/gpx/src/enrich.gpx.plugin.ts (4 hunks)
- enrich/gpx/vitest.config.ts (1 hunks)
- enrich/sentiment/jest.config.cjs (0 hunks)
- enrich/sentiment/src/enrich.sentiment.plugin.spec.ts (4 hunks)
- enrich/sentiment/src/enrich.sentiment.plugin.ts (2 hunks)
- enrich/sentiment/vitest.config.ts (1 hunks)
- enrich/summarize/jest.config.cjs (0 hunks)
- enrich/summarize/src/summarize.plugin.spec.ts (1 hunks)
- enrich/summarize/src/summarize.plugin.ts (2 hunks)
- enrich/summarize/vitest.config.ts (1 hunks)
- export/pivot-table/jest.config.cjs (0 hunks)
- export/pivot-table/vitest.config.ts (1 hunks)
- extract/html-table/jest.config.cjs (0 hunks)
- extract/html-table/vitest.config.ts (1 hunks)
- extract/markdown/jest.config.cjs (0 hunks)
- extract/markdown/src/parser.spec.ts (1 hunks)
- extract/markdown/vitest.config.ts (1 hunks)
- import/faker/jest.config.cjs (0 hunks)
- import/faker/src/faker.plugin.spec.ts (4 hunks)
- import/faker/src/faker.utils.ts (2 hunks)
- import/faker/vitest.config.ts (1 hunks)
- import/llm-records/jest.config.cjs (0 hunks)
- import/llm-records/vitest.config.ts (1 hunks)
- import/rss/jest.config.cjs (0 hunks)
- import/rss/vitest.config.ts (1 hunks)
- jest.config.js (0 hunks)
- jest.setup.js (0 hunks)
- plugins/autocast/jest.config.cjs (0 hunks)
- plugins/autocast/src/autocast.plugin.spec.ts (2 hunks)
- plugins/autocast/vitest.config.ts (1 hunks)
- plugins/automap/jest.config.cjs (0 hunks)
- plugins/automap/src/automap.plugin.e2e.spec.ts (2 hunks)
- plugins/automap/vitest.config.ts (1 hunks)
- plugins/constraints/jest.config.cjs (0 hunks)
- plugins/constraints/src/external.constraint.e2e.spec.ts (2 hunks)
- plugins/constraints/src/external.sheet.constraint.e2e.spec.ts (1 hunks)
- plugins/constraints/vitest.config.ts (1 hunks)
- plugins/dedupe/jest.config.cjs (0 hunks)
- plugins/dedupe/src/keep.first.logic.spec.ts (1 hunks)
- plugins/dedupe/src/keep.last.logic.spec.ts (1 hunks)
- plugins/dedupe/vitest.config.ts (1 hunks)
- plugins/delimiter-extractor/jest.config.cjs (0 hunks)
- plugins/delimiter-extractor/src/parser.spec.ts (2 hunks)
- plugins/delimiter-extractor/vitest.config.ts (1 hunks)
- plugins/dxp-configure/jest.config.cjs (0 hunks)
- plugins/dxp-configure/src/shim.target.spec.ts (1 hunks)
- plugins/dxp-configure/vitest.config.ts (1 hunks)
- plugins/export-workbook/jest.config.cjs (0 hunks)
- plugins/export-workbook/vitest.config.ts (1 hunks)
- plugins/foreign-db-extractor/jest.config.cjs (0 hunks)
- plugins/foreign-db-extractor/vitest.config.ts (1 hunks)
- plugins/graphql-schema/jest.config.cjs (0 hunks)
- plugins/graphql-schema/src/generate.field.spec.ts (3 hunks)
- plugins/graphql-schema/vitest.config.ts (1 hunks)
- plugins/job-handler/jest.config.cjs (0 hunks)
- plugins/job-handler/src/job.handler.e2e.spec.ts (2 hunks)
- plugins/job-handler/vitest.config.ts (1 hunks)
- plugins/json-extractor/jest.config.cjs (0 hunks)
- plugins/json-extractor/src/parser.spec.ts (1 hunks)
- plugins/json-extractor/vitest.config.ts (1 hunks)
- plugins/json-schema/jest.config.cjs (0 hunks)
- plugins/json-schema/src/setup.factory.spec.ts (1 hunks)
- plugins/json-schema/vitest.config.ts (1 hunks)
- plugins/merge-connection/jest.config.cjs (0 hunks)
- plugins/merge-connection/vitest.config.ts (1 hunks)
- plugins/openapi-schema/jest.config.cjs (0 hunks)
- plugins/openapi-schema/src/setup.factory.openapi.spec.ts (1 hunks)
- plugins/openapi-schema/vitest.config.ts (1 hunks)
- plugins/pdf-extractor/jest.config.cjs (0 hunks)
- plugins/pdf-extractor/vitest.config.ts (1 hunks)
- plugins/psv-extractor/jest.config.cjs (0 hunks)
- plugins/psv-extractor/vitest.config.ts (1 hunks)
- plugins/record-hook/jest.config.cjs (0 hunks)
- plugins/record-hook/src/RecordHook.e2e.spec.ts (5 hunks)
- plugins/record-hook/vitest.config.ts (1 hunks)
- plugins/rollout/jest.config.cjs (0 hunks)
- plugins/rollout/vitest.config.ts (1 hunks)
- plugins/space-configure/jest.config.cjs (0 hunks)
- plugins/space-configure/src/space.configure.e2e.spec.ts (3 hunks)
- plugins/space-configure/vitest.config.ts (1 hunks)
- plugins/sql-ddl-converter/jest.config.cjs (0 hunks)
- plugins/sql-ddl-converter/vitest.config.ts (1 hunks)
- plugins/tsv-extractor/jest.config.cjs (0 hunks)
- plugins/tsv-extractor/vitest.config.ts (1 hunks)
- plugins/view-mapped/jest.config.cjs (0 hunks)
- plugins/view-mapped/vitest.config.ts (1 hunks)
- plugins/webhook-egress/jest.config.cjs (0 hunks)
- plugins/webhook-egress/src/webhook.egress.e2e.spec.ts (5 hunks)
- plugins/webhook-egress/vitest.config.ts (1 hunks)
- plugins/webhook-event-forwarder/jest.config.cjs (0 hunks)
- plugins/webhook-event-forwarder/src/forward-webhook.e2e.spec.ts (3 hunks)
- plugins/webhook-event-forwarder/vitest.config.ts (1 hunks)
- plugins/xlsx-extractor/jest.config.cjs (0 hunks)
- plugins/xlsx-extractor/src/parser.spec.ts (1 hunks)
- plugins/xlsx-extractor/src/utils.spec.ts (1 hunks)
- plugins/xlsx-extractor/vitest.config.ts (1 hunks)
- plugins/xml-extractor/jest.config.cjs (0 hunks)
- plugins/xml-extractor/src/parser.spec.ts (1 hunks)
- plugins/xml-extractor/vitest.config.ts (1 hunks)
- plugins/yaml-schema/jest.config.cjs (0 hunks)
- plugins/yaml-schema/src/setup.factory.spec.ts (2 hunks)
- plugins/yaml-schema/vitest.config.ts (1 hunks)
- plugins/zip-extractor/jest.config.cjs (0 hunks)
- plugins/zip-extractor/src/zip.extractor.e2e.spec.ts (2 hunks)
- plugins/zip-extractor/vitest.config.ts (1 hunks)
- support/common-utils/src/async.map.spec.ts (2 hunks)
- support/common-utils/vitest.config.ts (1 hunks)
- test/betterConsoleLog.js (0 hunks)
- test/dotenv-config.js (0 hunks)
- test/setup-global.js (0 hunks)
- test/toBePendingMatcher.ts (0 hunks)
- test/unit.cleanup.js (0 hunks)
- utils/common/jest.config.cjs (0 hunks)
- utils/common/src/all.records.e2e.spec.ts (7 hunks)
- utils/common/src/async.batch.spec.ts (6 hunks)
- utils/common/src/logging.helper.spec.ts (4 hunks)
- utils/common/src/slugify.spec.ts (1 hunks)
- utils/common/vitest.config.ts (1 hunks)
- utils/extractor/jest.config.cjs (0 hunks)
- utils/extractor/vitest.config.ts (1 hunks)
- utils/fetch-schema/jest.config.cjs (0 hunks)
- utils/fetch-schema/vitest.config.ts (1 hunks)
- utils/file-buffer/jest.config.cjs (0 hunks)
- utils/file-buffer/vitest.config.ts (1 hunks)
- utils/response-rejection/jest.config.cjs (0 hunks)
- utils/response-rejection/vitest.config.ts (1 hunks)
- utils/testing/jest.config.cjs (0 hunks)
- utils/testing/src/test.helpers.ts (3 hunks)
- utils/testing/src/test.listener.spec.ts (2 hunks)
- utils/testing/src/toBePendingMatcher.ts (1 hunks)
- utils/testing/vitest.config.ts (1 hunks)
- validate/boolean/jest.config.cjs (0 hunks)
- validate/boolean/src/validate.boolean.plugin.spec.ts (6 hunks)
- validate/boolean/vitest.config.ts (1 hunks)
- validate/date/jest.config.cjs (0 hunks)
- validate/date/src/validate.date.plugin.e2e.spec.ts (1 hunks)
- validate/date/vitest.config.ts (1 hunks)
- validate/email/jest.config.cjs (0 hunks)
- validate/email/src/validate.email.plugin.ts (1 hunks)
⛔ Files not processed due to max files limit (15)
- validate/email/src/validate.email.utils.spec.ts
- validate/email/src/validate.email.utils.ts
- validate/email/vitest.config.ts
- validate/isbn/jest.config.cjs
- validate/isbn/src/validate.isbn.plugin.spec.ts
- validate/isbn/vitest.config.ts
- validate/number/jest.config.cjs
- validate/number/src/validate.number.plugin.spec.ts
- validate/number/vitest.config.ts
- validate/phone/jest.config.cjs
- validate/phone/src/validate.phone.utils.spec.ts
- validate/phone/vitest.config.ts
- validate/string/jest.config.cjs
- validate/string/src/validate.string.plugin.spec.ts
- validate/string/vitest.config.ts
💤 Files with no reviewable changes (57)
- convert/currency/jest.config.cjs
- convert/translate/jest.config.cjs
- convert/what3words/jest.config.cjs
- enrich/geocode/jest.config.cjs
- enrich/gpx/jest.config.cjs
- enrich/sentiment/jest.config.cjs
- enrich/summarize/jest.config.cjs
- export/pivot-table/jest.config.cjs
- extract/html-table/jest.config.cjs
- extract/markdown/jest.config.cjs
- import/faker/jest.config.cjs
- import/llm-records/jest.config.cjs
- import/rss/jest.config.cjs
- jest.config.js
- jest.setup.js
- plugins/autocast/jest.config.cjs
- plugins/automap/jest.config.cjs
- plugins/constraints/jest.config.cjs
- plugins/dedupe/jest.config.cjs
- plugins/delimiter-extractor/jest.config.cjs
- plugins/dxp-configure/jest.config.cjs
- plugins/export-workbook/jest.config.cjs
- plugins/foreign-db-extractor/jest.config.cjs
- plugins/graphql-schema/jest.config.cjs
- plugins/job-handler/jest.config.cjs
- plugins/json-extractor/jest.config.cjs
- plugins/json-schema/jest.config.cjs
- plugins/merge-connection/jest.config.cjs
- plugins/openapi-schema/jest.config.cjs
- plugins/pdf-extractor/jest.config.cjs
- plugins/psv-extractor/jest.config.cjs
- plugins/record-hook/jest.config.cjs
- plugins/rollout/jest.config.cjs
- plugins/space-configure/jest.config.cjs
- plugins/sql-ddl-converter/jest.config.cjs
- plugins/tsv-extractor/jest.config.cjs
- plugins/view-mapped/jest.config.cjs
- plugins/webhook-egress/jest.config.cjs
- plugins/webhook-event-forwarder/jest.config.cjs
- plugins/xlsx-extractor/jest.config.cjs
- plugins/xml-extractor/jest.config.cjs
- plugins/yaml-schema/jest.config.cjs
- plugins/zip-extractor/jest.config.cjs
- test/betterConsoleLog.js
- test/dotenv-config.js
- test/setup-global.js
- test/toBePendingMatcher.ts
- test/unit.cleanup.js
- utils/common/jest.config.cjs
- utils/extractor/jest.config.cjs
- utils/fetch-schema/jest.config.cjs
- utils/file-buffer/jest.config.cjs
- utils/response-rejection/jest.config.cjs
- utils/testing/jest.config.cjs
- validate/boolean/jest.config.cjs
- validate/date/jest.config.cjs
- validate/email/jest.config.cjs
✅ Files skipped from review due to trivial changes (57)
- .changeset/five-hounds-wash.md
- convert/currency/src/convert.currency.plugin.ts
- convert/currency/vitest.config.ts
- convert/translate/vitest.config.ts
- convert/what3words/vitest.config.ts
- enrich/geocode/src/index.ts
- enrich/geocode/vitest.config.ts
- enrich/gpx/vitest.config.ts
- enrich/sentiment/src/enrich.sentiment.plugin.spec.ts
- enrich/sentiment/vitest.config.ts
- enrich/summarize/src/summarize.plugin.ts
- enrich/summarize/vitest.config.ts
- export/pivot-table/vitest.config.ts
- extract/html-table/vitest.config.ts
- extract/markdown/vitest.config.ts
- import/faker/src/faker.utils.ts
- import/faker/vitest.config.ts
- import/llm-records/vitest.config.ts
- import/rss/vitest.config.ts
- plugins/autocast/vitest.config.ts
- plugins/automap/vitest.config.ts
- plugins/constraints/vitest.config.ts
- plugins/dedupe/vitest.config.ts
- plugins/delimiter-extractor/vitest.config.ts
- plugins/dxp-configure/vitest.config.ts
- plugins/export-workbook/vitest.config.ts
- plugins/foreign-db-extractor/vitest.config.ts
- plugins/graphql-schema/vitest.config.ts
- plugins/job-handler/vitest.config.ts
- plugins/json-extractor/vitest.config.ts
- plugins/json-schema/vitest.config.ts
- plugins/merge-connection/vitest.config.ts
- plugins/openapi-schema/vitest.config.ts
- plugins/pdf-extractor/vitest.config.ts
- plugins/psv-extractor/vitest.config.ts
- plugins/record-hook/vitest.config.ts
- plugins/rollout/vitest.config.ts
- plugins/space-configure/vitest.config.ts
- plugins/sql-ddl-converter/vitest.config.ts
- plugins/tsv-extractor/vitest.config.ts
- plugins/view-mapped/vitest.config.ts
- plugins/webhook-egress/vitest.config.ts
- plugins/webhook-event-forwarder/vitest.config.ts
- plugins/xlsx-extractor/vitest.config.ts
- plugins/xml-extractor/vitest.config.ts
- plugins/yaml-schema/vitest.config.ts
- plugins/zip-extractor/vitest.config.ts
- support/common-utils/vitest.config.ts
- utils/common/vitest.config.ts
- utils/extractor/vitest.config.ts
- utils/fetch-schema/vitest.config.ts
- utils/file-buffer/vitest.config.ts
- utils/response-rejection/vitest.config.ts
- utils/testing/vitest.config.ts
- validate/boolean/src/validate.boolean.plugin.spec.ts
- validate/boolean/vitest.config.ts
- validate/date/vitest.config.ts
🧰 Additional context used
🪛 Biome
enrich/gpx/src/enrich.gpx.plugin.ts
[error] 183-183: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.
(lint/suspicious/noGlobalIsNan)
[error] 183-183: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.
(lint/suspicious/noGlobalIsNan)
plugins/webhook-event-forwarder/src/forward-webhook.e2e.spec.ts
[error] 59-61: Disallow duplicate setup and teardown hooks.
Disallow afterAll duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
[error] 64-71: Disallow duplicate setup and teardown hooks.
Disallow beforeEach duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
[error] 73-75: Disallow duplicate setup and teardown hooks.
Disallow afterEach duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
🔇 Additional comments (99)
plugins/dedupe/src/keep.first.logic.spec.ts (2)
1-3: LGTM! Imports are properly structured for Vitest.
The imports are well-organized, following a logical order from external framework to local implementation.
2-2: Verify test data structure.
Let's ensure the test data structure matches the expected format for deduplication testing.
✅ Verification successful
Test data structure is properly formatted for deduplication testing
The test data structure in fake.data.ts follows the expected Flatfile.RecordsWithLinks format with:
- Unique record IDs
- Structured values containing first_name, last_name, and email fields
- Proper metadata and validation fields
- Test data is correctly used in
keep.first.logic.spec.tsfor email deduplication testing
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the structure of fake.data.ts to ensure it contains proper test records
# Test: Check the content and structure of fake.data.ts
rg -A 10 "export const records" plugins/dedupe/src/fake.data.ts
Length of output: 318
Script:
#!/bin/bash
# Let's get more context about the test data structure and its usage
# Get full content of fake.data.ts
rg -A 30 "export const records" plugins/dedupe/src/fake.data.ts
# Check how this data is used in the test file
rg -A 10 "records" plugins/dedupe/src/keep.first.logic.spec.ts
# Check for any type definitions that might be relevant
rg "type.*RecordsWithLinks" -A 5
Length of output: 1050
config/vitest/src/index.js (3)
1-1: LGTM! Clean import statement with explicit file extension.
The import statement follows ESM best practices by including the .js extension.
18-18: LGTM! Appropriate use of default export.
The default export is suitable for a configuration module that provides a single main export.
9-12: Verify environment variable handling.
Let's ensure the environment variables are being loaded correctly and that the UTC timezone setting doesn't conflict with any tests.
✅ Verification successful
Let me analyze the results and gather more information about the loadEnv function to ensure it's properly handling environment variables.
Let me check one more thing to verify how the environment variables are being used in tests and if there are any potential timezone-related issues.
Environment variable handling and timezone configuration are properly implemented
The implementation is correct and safe because:
- The
loadEnvfunction inconfig/vitest/src/utils.jsproperly uses thedotenvpackage to load environment variables - Setting
TZ: 'UTC'is a good practice as it ensures consistent timezone behavior across all tests - The existing timezone-dependent tests are explicitly using UTC/GMT formats or ISO strings, which aligns with the UTC timezone configuration
- No conflicts were found between the environment variable handling and the test suite
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for potential timezone-dependent tests and environment variable usage
# Test 1: Look for date/time manipulation in tests
echo "Checking for date/time manipulation in tests..."
rg -l "new Date|Date\." -t js -t ts
# Test 2: Check for direct process.env access that might conflict
echo "Checking for direct process.env access..."
rg "process\.env\." -t js -t ts
# Test 3: Look for existing .env files
echo "Checking for .env files..."
fd -H "^\.env"
Length of output: 2685
Script:
#!/bin/bash
# Check the implementation and usage of loadEnv function
echo "Checking loadEnv implementation..."
ast-grep --pattern 'function loadEnv'
rg -A 5 "loadEnv"
# Check for any environment-related configurations
echo "Checking for environment configuration files..."
fd -e json -e js -e ts -e yaml -e yml -H "env|environment" --exclude node_modules
Length of output: 1225
Script:
#!/bin/bash
# Check for timezone-specific test assertions and date manipulations
echo "Checking for timezone-specific tests..."
rg -B 2 -A 2 "expect.*Date|assert.*Date|test.*timezone|describe.*timezone" -t js -t ts
# Check for dotenv usage
echo "Checking for dotenv usage..."
rg "require\(['\"](dotenv|\.env)" -B 2 -A 2
rg "import.*dotenv" -B 2 -A 2
Length of output: 3059
plugins/dedupe/src/keep.last.logic.spec.ts (2)
1-4: LGTM! Proper migration to Vitest testing utilities.
The imports and test structure follow Vitest conventions correctly.
Line range hint 1-10: Verify the test data structure and keepLast implementation.
Let's verify the structure of the test data and the implementation details of the keepLast function to ensure our testing approach aligns with the actual implementation.
✅ Verification successful
Test data and implementation are correctly aligned with the test case
The test case correctly verifies the keepLast function's behavior:
- The test data contains two records with duplicate email 'foo@bar.com' (recordId:1 and recordId:5)
- The
keepLastimplementation keeps the last occurrence of duplicates (recordId:5) - The test correctly expects ['recordId:1'] to be returned for removal
- The
uniquesSet is properly used to track unique values
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the structure of fake.data and keepLast implementation
# Check the structure of fake.data
echo "Checking fake.data structure:"
cat plugins/dedupe/src/fake.data.ts
# Check the keepLast implementation
echo -e "\nChecking keepLast implementation:"
cat plugins/dedupe/src/keep.last.logic.ts
Length of output: 3472
utils/testing/src/toBePendingMatcher.ts (3)
1-11: LGTM! Well-structured imports and type declarations.
The code follows best practices by using the 'node:' prefix for Node.js built-ins and properly extends Vitest's type system.
32-32: LGTM! Matcher registration is correct.
The matcher is properly registered with Vitest's expect system.
Let's verify this is the only matcher registration in this file:
✅ Verification successful
Matcher registration is correctly implemented and unique
The verification confirms this is the only expect.extend call in the file, which is the correct pattern for registering a single matcher with Vitest.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for multiple expect.extend calls
rg "expect\.extend" utils/testing/src/toBePendingMatcher.ts
Length of output: 92
13-30: 🛠️ Refactor suggestion
Consider making the matcher more robust and informative.
The current implementation has several potential improvements:
- Using
util.inspect()string inspection is fragile as it relies on Node.js's internal string representation. - The error messages could be more descriptive.
Consider this more robust implementation:
const toBePending = (received: unknown) => {
- const isPending = (promise: Promise<any>) =>
- util.inspect(promise).includes('pending')
+ const isPending = (promise: Promise<any>) => {
+ let resolved = false
+ let rejected = false
+ promise.then(() => { resolved = true }, () => { rejected = true })
+ return !resolved && !rejected
+ }
if (!(received instanceof Promise)) {
- throw new TypeError('Actual value is not a promise!')
+ throw new TypeError(
+ `Expected a Promise object but received: ${util.inspect(received)}`
+ )
}
const pass = isPending(received)
return {
pass,
message: () =>
pass
- ? `expected received promise not to be pending`
- : `expected received promise to be pending`,
+ ? `Expected promise to be settled but it was pending\n\nReceived: ${util.inspect(received)}`
+ : `Expected promise to be pending but it was settled\n\nReceived: ${util.inspect(received)}`,
}
}This implementation:
- Uses a more reliable method to check if a promise is pending
- Provides more detailed error messages
- Includes the actual value in messages for better debugging
Let's verify if there are any existing tests that might break with these changes:
config/vitest/src/utils.js (2)
37-37: LGTM!
The export statement is correct and follows the module export pattern.
6-17:
Add error handling for file operations.
The fs.readFileSync operations could throw errors (e.g., permission issues, file corruption) which should be handled gracefully.
if (fs.existsSync(envPath)) {
- env = { ...env, ...dotenv.parse(fs.readFileSync(envPath)) }
+ try {
+ const envContent = fs.readFileSync(envPath, 'utf8');
+ env = { ...env, ...dotenv.parse(envContent) };
+ } catch (error) {
+ console.warn(`Failed to read ${envPath}: ${error.message}`);
+ }
}
if (fs.existsSync(defaultEnvPath)) {
- env = { ...env, ...dotenv.parse(fs.readFileSync(defaultEnvPath)) }
+ try {
+ const defaultEnvContent = fs.readFileSync(defaultEnvPath, 'utf8');
+ env = { ...env, ...dotenv.parse(defaultEnvContent) };
+ } catch (error) {
+ console.warn(`Failed to read ${defaultEnvPath}: ${error.message}`);
+ }
}Additionally, the code has potential path traversal vulnerabilities as flagged by the security scanner.
plugins/dxp-configure/src/shim.target.spec.ts (2)
1-2: LGTM! Clean and focused imports.
The imports are correctly structured for Vitest and include only the necessary testing utilities.
Line range hint 4-49: Verify test structure consistency across the codebase.
The test structure follows good practices. Let's verify that this structure is consistent with other migrated test files.
✅ Verification successful
Test structure is consistent across the codebase
The test structure in shim.target.spec.ts follows the same pattern observed across all Vitest test files in the codebase:
- All tests use the
describe()block to group related tests - Test descriptions are clear and descriptive
- Each test file focuses on testing a specific unit of functionality
- Test names follow a consistent format using function names or feature descriptions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check the structure of other Vitest test files for consistency
# Look for other test files using Vitest
echo "Checking test structure in other Vitest files..."
rg -l "from 'vitest'" --glob "**/*.spec.ts" | while read -r file; do
echo "\nFile: $file"
# Show the basic structure of each test file
rg "^(describe|it)\(" "$file"
done
Length of output: 10585
plugins/openapi-schema/src/setup.factory.openapi.spec.ts (1)
1-3: LGTM: Clean Vitest import and setup.
The transition from Jest to Vitest is correctly implemented with the appropriate imports.
utils/common/src/slugify.spec.ts (1)
1-3: LGTM! Clean test setup with proper imports.
The test file follows best practices with clear imports and BDD-style test structure.
validate/email/src/validate.email.plugin.ts (1)
29-29: LGTM! Improved code readability.
The added whitespace effectively separates the validation logic from error handling, making the code more readable.
convert/translate/src/convert.translate.plugin.spec.ts (3)
3-7: LGTM: Clean migration of import statements
The Vitest imports are correctly structured, importing only the necessary testing functions. The migration maintains clean code organization by grouping related imports together.
44-44: LGTM: Proper spy implementation
The spy implementation has been correctly migrated from jest.spyOn to vi.spyOn.
10-14: Verify mock implementation completeness
The mock implementation has been correctly migrated from Jest to Vitest syntax. However, we should verify if the mock covers all necessary scenarios.
Let's verify the original mock implementation and ensure we haven't missed any scenarios:
Consider adding error scenarios to the mock implementation:
vi.mock('@google-cloud/translate/build/src/v2', () => {
return {
Translate: vi.fn().mockImplementation(() => ({
translate: vi.fn().mockResolvedValue([['Hola', 'Mundo']]),
+ // Add error scenario mock
+ translate: vi.fn().mockImplementation((text, options) => {
+ if (text.includes('error')) {
+ return Promise.reject(new Error('Translation failed'));
+ }
+ return Promise.resolve([['Hola', 'Mundo']]);
+ }),
})),
}
})✅ Verification successful
Mock implementation is sufficient but could be enhanced
The current mock implementation is adequate for basic testing scenarios as it covers the core translation functionality. The actual implementation shows that the mock correctly simulates the translate method's return format of [['Hola', 'Mundo']]. However, here are some optional enhancements you could consider:
- The mock could include error handling to match the actual implementation's error handling in
batchTranslateText() - You could add mocks for the configuration options like
fromandtolanguage parameters
The current mock is sufficient for the basic test cases shown in the spec file, but these enhancements would provide more thorough testing coverage.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for the original Jest mock implementation to compare
rg -B 2 -A 5 "jest\.mock.*@google-cloud/translate" --type ts
# Search for any other translate method usages that might need mocking
ast-grep --pattern 'translate($$$)'
Length of output: 100
Script:
#!/bin/bash
# Let's search for test files that might contain translate-related tests
fd "\.spec\.ts$|\.test\.ts$" | xargs cat
# Also search for actual usage of translate in the codebase
rg -B 2 -A 5 "translate\(" --type ts
Length of output: 143239
Script:
#!/bin/bash
# Let's look at the actual implementation and test file to understand the usage
cat convert/translate/src/convert.translate.plugin.ts
Length of output: 2769
plugins/zip-extractor/src/zip.extractor.e2e.spec.ts (3)
6-8: LGTM: Clean migration to Vitest with improved test utilities
The transition from Jest to Vitest is well-structured, and the introduction of TestListener and setupDriver utilities provides a more organized approach to test setup.
Also applies to: 12-20
29-44: LGTM: Well-structured test lifecycle management
The implementation of lifecycle hooks is comprehensive with proper setup and cleanup:
- Driver initialization and shutdown
- Listener mounting and reset
- Test state management
49-49: LGTM: Improved environment configuration handling
The explicit handling of the environment ID improves clarity and maintainability.
Also applies to: 60-60
support/common-utils/src/async.map.spec.ts (1)
1-1: LGTM: Clean migration to Vitest
The test setup has been correctly migrated from Jest to Vitest, maintaining proper test isolation with beforeEach and following Vitest's mocking patterns.
Also applies to: 6-8
plugins/xlsx-extractor/src/utils.spec.ts (2)
Line range hint 1-5: LGTM! Clean migration to Vitest
The test setup follows best practices with proper organization using nested describe blocks for better grouping and readability.
Line range hint 6-38: LGTM! Comprehensive test coverage for core functionality
The test cases are well-structured and cover essential scenarios including unique values, duplicates, empty strings, and case sensitivity. The test data is clear and illustrative.
plugins/autocast/src/autocast.plugin.spec.ts (3)
Line range hint 1-8: LGTM: Clean transition to Vitest
The import statements are properly organized, importing both Vitest utilities and the required casting functions.
33-38: LGTM: Well-structured boolean casting tests
Good use of constants and test.each for comprehensive boolean casting validation.
51-56: LGTM: Comprehensive error handling tests
Well-structured error handling tests using test.each to validate error cases across all casting functions.
enrich/summarize/src/summarize.plugin.spec.ts (2)
1-3: LGTM: Clean test structure and proper Vitest imports.
The migration to Vitest has been properly implemented with the correct imports and BDD-style test structure.
Line range hint 4-39: LGTM: Comprehensive test coverage for generateSummary.
The test suite effectively covers:
- Default behavior
- Custom length summaries
- Percentage-based summaries
- Edge cases with short content
extract/markdown/src/parser.spec.ts (2)
1-6: Clean migration to Vitest testing framework
The import statements are properly updated to use Vitest while maintaining the necessary functionality. The migration is clean as Vitest maintains API compatibility with Jest.
Line range hint 8-28: Well-structured test suite with comprehensive coverage
The test suite is well-organized using a test cases array and includes thorough assertions for table structure and content.
utils/common/src/logging.helper.spec.ts (2)
1-1: LGTM: Correct transition to Vitest imports
The transition from Jest to Vitest is properly implemented with the correct import of testing utilities.
6-8: LGTM: Correct implementation of Vitest spies
The transition from jest.spyOn to vi.spyOn is properly implemented across all test cases.
Also applies to: 34-34, 46-46, 58-58
plugins/yaml-schema/src/setup.factory.spec.ts (2)
3-3: LGTM: Clean migration to Vitest mocking patterns
The transition from Jest to Vitest follows the recommended patterns, with proper setup of the fetch mock using vi.fn() and vi.stubGlobal().
Also applies to: 6-8
59-59: 🛠️ Refactor suggestion
Consider using mockReset() for more thorough cleanup
mockClear() only clears the call history but retains mock implementations. Consider using mockReset() to clear both the history and implementations between tests, ensuring a clean slate for each test case.
- fetchMock.mockClear()
+ fetchMock.mockReset()plugins/constraints/src/external.sheet.constraint.e2e.spec.ts (2)
6-19: LGTM: Clean migration to Vitest with improved test utilities
The import changes appropriately reflect the migration from Jest to Vitest while introducing more structured testing utilities (TestListener, setupDriver).
26-41: LGTM: Well-structured test lifecycle management
The implementation of lifecycle hooks follows testing best practices:
- Driver lifecycle is properly managed with start/shutdown
- Listener state is reset between tests
- Clear separation of setup and teardown responsibilities
plugins/job-handler/src/job.handler.e2e.spec.ts (2)
1-16: Clean migration to Vitest with well-organized imports.
The transition from Jest to Vitest has been implemented correctly, with proper separation of testing utilities and clear import organization.
Line range hint 1-110: Verify complete migration from Jest to Vitest.
Let's ensure no Jest-specific code remains in the codebase.
✅ Verification successful
Let me try a different search strategy to verify the Jest to Vitest migration.
Based on the search results, I can now generate the final response:
Migration from Jest to Vitest is complete and verified.
The codebase shows no signs of remaining Jest-specific test code:
- All test files (*.spec.ts) are present and properly structured
- No Jest configuration files found
- Jest references in package-lock.json are only transitive dependencies from other packages
- No direct Jest dependencies in package.json
- The file under review correctly uses Vitest's testing constructs (vi.fn(), vi.spyOn())
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining Jest references in test files
echo "Checking for remaining Jest references..."
rg -l "jest\." "**/*.spec.ts" "**/*.test.ts"
# Check for jest configuration files that should have been removed
echo "Checking for remaining Jest config files..."
fd "jest.config" -t f
Length of output: 364
Script:
#!/bin/bash
# Search for test files with proper paths
echo "Finding test files..."
fd "\.(spec|test)\.(ts|js)$"
# Search for Jest-related terms more broadly
echo "Checking for Jest-related terms..."
rg -i "jest" -g "*.{ts,js,json}"
# Look for package.json to check test dependencies
echo "Checking package.json for test dependencies..."
cat package.json 2>/dev/null || echo "No package.json in root"
Length of output: 7174
plugins/space-configure/src/space.configure.e2e.spec.ts (3)
2-17: LGTM: Clean migration to Vitest and improved test utilities.
The import statements are well-organized and the transition from Jest to Vitest is implemented correctly.
51-70: LGTM: Robust test infrastructure setup.
The new test infrastructure using TestListener and setupDriver provides better test isolation and management.
88-98: 🛠️ Refactor suggestion
Consider more defensive null checking in assertions.
The extensive use of non-null assertions (!) could mask potential runtime issues. Consider implementing type guards or explicit null checks for better reliability.
- const workspace = await api.workbooks.get(space.data.primaryWorkbookId!)
+ if (!space.data.primaryWorkbookId) {
+ throw new Error('Primary workbook ID is required')
+ }
+ const workspace = await api.workbooks.get(space.data.primaryWorkbookId)
+
+ if (!workspace.data) {
+ throw new Error('Workspace data is required')
+ }
expect(workspace.data.name).toBe(setup.workbooks[0].name)
- expect(workspace.data.labels).toMatchObject(setup.workbooks[0].labels!)
- expect(workspace.data.sheets![0].name).toBe(
- setup.workbooks[0].sheets![0].name
+ expect(workspace.data.labels).toMatchObject(setup.workbooks[0].labels ?? [])
+
+ const workspaceSheets = workspace.data.sheets ?? []
+ const setupSheets = setup.workbooks[0].sheets ?? []
+
+ expect(workspaceSheets[0]?.name).toBe(
+ setupSheets[0]?.name
)
- expect(workspace.data.sheets![0].config).toMatchObject(
- setup.workbooks[0].sheets![0]
+ expect(workspaceSheets[0]?.config).toMatchObject(
+ setupSheets[0] ?? {}
)
- expect(workspace.data.actions).toMatchObject(setup.workbooks[0].actions!)
+ expect(workspace.data.actions).toMatchObject(setup.workbooks[0].actions ?? [])enrich/sentiment/src/enrich.sentiment.plugin.ts (2)
1-1: LGTM! Good use of type-only import.
The change to use import type is a TypeScript best practice that enables better tree-shaking and explicitly declares the import as type-only.
Line range hint 18-36: Consider adding input validation and error handling in analyzeSentiment.
The sentiment analysis could benefit from additional validation and error handling:
export function analyzeSentiment(text: string) {
+ if (typeof text !== 'string') {
+ throw new TypeError('Input must be a string');
+ }
+
const result = sentiment.analyze(text)
+
+ if (!result || typeof result.score !== 'number') {
+ throw new Error('Invalid sentiment analysis result');
+ }
+
let category: 'positive' | 'negative' | 'neutral'
if (result.score > 0) {
category = 'positive'
} else if (result.score < 0) {
category = 'negative'
} else {
category = 'neutral'
}
return {
score: result.score,
category: category,
+ comparative: result.comparative, // Additional context
+ tokens: result.tokens.length // Useful for validation
}
}plugins/constraints/src/external.constraint.e2e.spec.ts (3)
11-19: LGTM: Clean transition to Vitest imports
The migration from Jest to Vitest imports is clean and follows the standard Vitest testing patterns.
58-58: Simplified constraint callback signature
The callback signature has been simplified by removing the config parameter. This is a breaking change that needs to be verified across the codebase.
Let's check for any existing usages that might be affected:
#!/bin/bash
# Search for existing usages of externalConstraint that might expect the config parameter
ast-grep --pattern 'externalConstraint($_, ($_, $_, $_) => $_)'23-41:
Improved test infrastructure setup with proper lifecycle hooks
The new setup with TestListener and Driver is more robust than the previous implementation:
- Clear separation of concerns between listener and driver
- Proper cleanup in lifecycle hooks
- Consistent state management with reset operations
However, there's a potential issue with duplicate beforeAll hooks (lines 26-29 and 45-53).
Consider consolidating the two beforeAll blocks:
beforeAll(async () => {
await driver.start()
listener.mount(driver)
- })
-
- afterAll(() => {
- driver.shutdown()
- })
-
- beforeEach(() => {
- listener.resetCount()
- })
-
- afterEach(() => {
- listener.reset()
- })
-
- let spaceId: string
- let sheetId: string
-
- beforeAll(async () => {
const space = await setupSpace()
spaceId = space.id
const workbook = await setupSimpleWorkbook(
space.id,
defaultSimpleValueSchema
)
sheetId = workbook.sheets![0].id
})Likely invalid or redundant comment.
plugins/json-schema/src/setup.factory.spec.ts (1)
1-3: LGTM! Clean migration to Vitest.
The import structure is clean and follows Vitest best practices.
plugins/automap/src/automap.plugin.e2e.spec.ts (2)
4-20: LGTM: Clean transition to Vitest imports
The import changes properly reflect the migration from Jest to Vitest and the adoption of the driver pattern for better test isolation.
26-27: LGTM: Improved test setup with driver pattern
The introduction of a separate test listener instance and driver setup improves test isolation and control.
plugins/webhook-event-forwarder/src/forward-webhook.e2e.spec.ts (1)
Line range hint 1-19: Clean migration to Vitest imports and testing utilities.
The transition from Jest to Vitest has been implemented correctly with appropriate imports. The new TestListener and setupDriver utilities from @flatfile/utils-testing align well with the migration strategy.
enrich/gpx/src/enrich.gpx.plugin.spec.ts (3)
1-1: LGTM: Clean transition to Vitest
The migration from Jest to Vitest is clean and maintains the same testing API, ensuring minimal disruption to existing tests.
23-40: LGTM: Improved test data formatting
The test data structures have been reformatted for better readability while maintaining the same test coverage and assertions. The new format makes it easier to understand the test scenarios.
Also applies to: 77-95
Line range hint 1-95: Verify complete removal of Jest dependencies
While the test file has been successfully migrated to Vitest, let's ensure no Jest configurations or dependencies remain.
✅ Verification successful
Let me gather more information to ensure a thorough verification of the Jest to Vitest migration.
Jest dependencies and configurations successfully removed
The verification confirms that:
- No Jest configuration files exist in the codebase
- No Jest dependencies are present in any package.json files
- No Jest-specific API usage or imports were found
- No Jest types are being referenced
The migration to Vitest appears to be complete and clean.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any remaining Jest configuration files or dependencies
echo "Checking for Jest configuration files..."
fd -e json -e js -e cjs "jest.config"
echo "Checking for Jest dependencies in package.json files..."
fd "package.json" -x jq -r '.dependencies["jest"] // .devDependencies["jest"] // empty'
Length of output: 337
Script:
#!/bin/bash
# Check for any Jest-specific API usage in test files
echo "Checking for Jest-specific API usage..."
rg -l "jest\."
# Check for Jest types or imports
echo -e "\nChecking for Jest types or imports..."
rg -l "@types/jest|from 'jest'"
# Check for any Jest-related dependencies including testing libraries
echo -e "\nChecking all package.json files for Jest-related dependencies..."
fd "package.json" -x jq -r 'keys_unsorted as $k | .dependencies? + .devDependencies? | with_entries(select(.key | test("jest|testing-library"))) | keys[]'
Length of output: 536
enrich/geocode/src/enrich.geocode.plugin.spec.ts (1)
30-31: Verify test coverage and improve type safety in mocks.
The test cases provide good coverage of the geocoding functionality. However, the mock implementations could benefit from improved type safety.
Let's verify the test coverage:
Consider replacing the as any type assertions with proper response types:
- vi.mocked(fetch).mockResolvedValue({
- json: vi.fn().mockResolvedValue(mockResponse),
- } as any)
+ vi.mocked(fetch).mockResolvedValue({
+ json: vi.fn().mockResolvedValue(mockResponse),
+ } satisfies Partial<Response>)Also applies to: 64-65, 90-91, 106-106, 117-117
✅ Verification successful
Let me gather more information about the test coverage by checking the test file.
Test coverage is comprehensive and type assertions are justified
The test suite thoroughly covers all code paths identified in the implementation:
- Forward geocoding (address input)
- Reverse geocoding (latitude/longitude input)
- Zero results handling
- API error scenarios
- Unexpected errors
- Input validation
Regarding the as any type assertions in mocks: while type safety improvements would be ideal, the current usage is acceptable because:
- The mocks only implement the minimal interface required for testing
- The Response type from fetch API is complex and creating a full mock would add unnecessary complexity
- The test behavior and assertions properly verify the functionality
The current implementation strikes a good balance between type safety and test maintainability.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any uncovered code paths in the geocoding implementation
# Test: Look for conditional branches in the implementation file
rg -A 2 "if|switch|try|catch" ./enrich/geocode/src/enrich.geocode.plugin.ts
# Test: Verify all exported functions are tested
ast-grep --pattern 'export function $func($$$) { $$$ }' ./enrich/geocode/src/enrich.geocode.plugin.ts
Length of output: 1637
Script:
#!/bin/bash
# Check test cases against implementation branches
rg -A 2 "it\(.*" ./enrich/geocode/src/enrich.geocode.plugin.spec.ts
# Check mock response types
rg "mockResolvedValue|mockReturnValue" ./enrich/geocode/src/enrich.geocode.plugin.spec.ts
Length of output: 1177
plugins/xml-extractor/src/parser.spec.ts (1)
Line range hint 1-89: Well-structured test fixtures!
The test data is comprehensive and covers various scenarios including nested structures, attributes, and edge cases. This provides good coverage for testing XML parsing functionality.
plugins/xlsx-extractor/src/parser.spec.ts (3)
4-4: Clean transition to Vitest
The import statement correctly brings in the necessary Vitest testing functions, maintaining compatibility with the existing test structure.
Line range hint 7-42: Well-structured test organization
The test suite follows best practices with:
- Clear hierarchical organization using nested describe blocks
- Efficient test setup using beforeAll
- Meaningful test descriptions
Line range hint 89-134: Comprehensive empty lines handling tests
The tests thoroughly validate the skipEmptyLines functionality with:
- Clear test cases for both true and false scenarios
- Precise assertions verifying the expected data structure
- Good coverage of edge cases
convert/currency/src/convert.currency.plugin.spec.ts (5)
1-11: LGTM: Proper migration to Vitest imports and mocking setup
The transition from Jest to Vitest has been implemented correctly with appropriate imports and mocking configuration.
60-60: Security: Hardcoded API key issue persists
A previous security review has already identified this issue. Please refer to the existing comment about storing the API key in an environment variable.
85-90: LGTM: Test assertions properly migrated
The test assertions have been successfully migrated to Vitest while maintaining comprehensive test coverage.
Also applies to: 112-119
130-132: LGTM: Error handling tests properly implemented
The async error handling tests have been correctly migrated to Vitest, following best practices for testing rejected promises.
1-135: Verify complete removal of Jest dependencies
While this file has been properly migrated to Vitest, let's ensure no Jest-specific code remains in related files.
✅ Verification successful
Jest dependencies found as peer dependencies of React Native
The Jest-related packages found in package-lock.json are all marked as peer dependencies, primarily coming from React Native and its CLI tools. These are not direct dependencies of the project and don't affect the Vitest migration. The test file is correctly using Vitest's testing framework, and there are no direct Jest dependencies in package.json.
- The Jest packages are only present as peer dependencies through:
react-native(version 0.75.2)@react-native-community/cli-server-api@react-native/community-cli-plugin
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining Jest-specific code or configurations
echo "Checking for remaining Jest dependencies..."
rg -l "jest\." || echo "No Jest method calls found"
rg -l "@jest/" || echo "No Jest package imports found"
fd "jest.config" || echo "No Jest config files found"
Length of output: 226
Script:
#!/bin/bash
# Let's check the package.json for any Jest-related dependencies
echo "Checking package.json for Jest dependencies..."
cat package.json | jq '.dependencies,.devDependencies' | grep -i jest || echo "No Jest dependencies found in package.json"
# Let's also check the content of package-lock.json for @jest references
echo -e "\nChecking @jest references in package-lock.json..."
rg "@jest/" -A 5 -B 5 package-lock.json
Length of output: 6536
utils/testing/src/test.helpers.ts (3)
4-4: LGTM: Vitest lifecycle hooks imported correctly.
The import of Vitest lifecycle hooks aligns with the migration from Jest to Vitest.
54-54: LGTM: Improved driver setup handling.
The use of the new setupDriver function improves error handling consistency and code organization.
4-4: Verify complete removal of Jest dependencies.
Let's ensure the Jest to Vitest migration is complete for this module.
✅ Verification successful
Since the executed shell scripts show no occurrences of "jest" in the utils/testing/ directory and no Jest-related dependencies in package.json, I can confidently generate the final response.
Jest dependencies have been completely removed ✅
The migration from Jest to Vitest appears to be complete for this module. No traces of Jest were found in:
- The testing utilities directory
- Project dependencies
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining Jest-specific code or dependencies
# Test 1: Check for Jest imports or references
echo "Checking for Jest references..."
rg -i "jest" utils/testing/
# Test 2: Check package.json for Jest dependencies
echo "Checking package.json for Jest dependencies..."
if [ -f "package.json" ]; then
jq '.dependencies,.devDependencies | select(. != null) | with_entries(select(.key | contains("jest")))' package.json
fi
Length of output: 351
plugins/graphql-schema/src/generate.field.spec.ts (2)
1-1: LGTM: Clean migration to Vitest
The transition from Jest to Vitest has been implemented correctly with proper imports.
63-63: LGTM: Proper console spying implementation
The console spy implementation using vi.spyOn follows Vitest best practices and maintains the same functionality as the previous Jest implementation.
Also applies to: 151-151
validate/date/src/validate.date.plugin.e2e.spec.ts (3)
6-19: LGTM: Clean migration of test dependencies
The import changes correctly implement the transition from Jest to Vitest while maintaining a clean and organized structure.
25-26: LGTM: Well-structured test setup initialization
The test setup properly initializes both the TestListener and driver instances at the appropriate scope level.
28-43: LGTM: Well-implemented test lifecycle management
The lifecycle hooks properly manage the test environment setup, teardown, and state reset between tests.
import/faker/src/faker.plugin.spec.ts (4)
1-4: LGTM: Clean import structure
The imports are properly organized and correctly updated for Vitest.
50-81: LGTM: Comprehensive faker mock implementation
The faker mock implementation is thorough and correctly uses Vitest mocking patterns. Static return values are appropriate for predictable testing.
84-86: LGTM: Appropriate mock implementation
The createAllRecords mock is appropriately minimal for its purpose.
Line range hint 1-151: Verify test coverage after framework migration
The transition from Jest to Vitest has been implemented correctly. However, it's important to verify that test coverage hasn't been affected by the migration.
utils/common/src/async.batch.spec.ts (3)
2-2: LGTM: Correct Vitest imports
The import statement correctly includes all necessary Vitest testing utilities, properly replacing the Jest equivalents.
8-8: LGTM: Consistent migration of mock functions
All mock functions have been correctly migrated from jest.fn to vi.fn while maintaining the original callback implementations and signatures. The changes are consistent across all test cases.
Also applies to: 28-28, 44-44, 61-61, 78-78, 97-97
Line range hint 1-144: Verify complete migration to Vitest
The test suite maintains comprehensive coverage with proper assertions. Let's verify that no Jest-specific code or configurations remain in related files.
✅ Verification successful
The verification results show:
- No Jest config files found
- No Jest-specific imports or function usage found
- Vitest configuration files are present across all packages
Migration to Vitest is complete and properly configured
The test suite has been fully migrated to Vitest with no remaining Jest dependencies. All assertions are using Vitest's API, and proper configuration files are in place across all packages.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining Jest dependencies or configurations
# Test 1: Look for any remaining jest.config files
echo "Checking for remaining Jest config files..."
fd "jest.config"
# Test 2: Check for any remaining Jest-specific code
echo "Checking for remaining Jest-specific imports or usage..."
rg "from 'jest'" || rg "from \"jest\"" || rg "jest\.fn"
# Test 3: Verify Vitest config exists
echo "Verifying Vitest configuration..."
fd "vitest.config"
Length of output: 2391
utils/testing/src/test.listener.spec.ts (2)
3-4: LGTM: Clean migration to Vitest imports
The import statements have been correctly updated to use Vitest's testing functions, and the matcher import has been properly relocated.
44-45: LGTM: Correct migration of mock functions
The mock functions have been properly migrated from jest.fn() to vi.fn(). The test cases maintain their original behavior.
Let's verify if all Jest mocks have been migrated in the codebase:
✅ Verification successful
Mock functions correctly migrated across the codebase
The verification shows that all mock functions in the test files are using Vitest's vi.fn(). There are no remaining instances of jest.fn() in the codebase, and the migration appears to be complete and consistent across all test files.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining jest.fn() calls that might have been missed
rg "jest\.fn\(\)" -g "*.{ts,js,tsx,jsx}"
Length of output: 42
Script:
#!/bin/bash
# Let's try a more comprehensive search for Jest-related mocks and assertions
rg -i "jest\." -g "*.{ts,js,tsx,jsx}" -A 2
# Also search for vi.fn to understand the test framework usage
rg "vi\.fn\(\)" -g "*.{ts,js,tsx,jsx}"
# Search for test files to understand the testing setup
fd -e spec.ts -e spec.js -e test.ts -e test.js
Length of output: 4937
utils/common/src/all.records.e2e.spec.ts (5)
3-3: LGTM: Clean migration to Vitest imports
The import statement correctly includes all necessary test utilities from Vitest, aligning with the framework migration objective.
42-42: LGTM: Improved null safety with optional chaining
The addition of the optional chaining operator (?.) enhances the robustness of the findSheetId function by safely handling potential undefined values.
45-46: LGTM: Enhanced type safety with explicit annotations
The addition of type assertions and explicit type annotations improves code clarity and type safety. The Flatfile.RecordData[] type properly defines the shape of generated records.
Also applies to: 48-49
Line range hint 8-177: LGTM: Comprehensive test coverage maintained
The test suite maintains excellent coverage across various scenarios:
- Empty and populated sheets
- Pagination handling
- Record filtering
- Error cases
- Edge cases with different page sizes
The migration to Vitest preserves all test scenarios while improving the testing infrastructure.
68-68: Verify complete migration of mock functions
The mock functions have been correctly updated to use vi.fn(). Let's verify that no Jest mocks remain in the codebase.
Also applies to: 80-80, 90-90, 108-108, 126-126
✅ Verification successful
Mock functions migration is complete
The verification confirms that:
- No instances of
jest.fn()remain in the codebase - All mock functions are using Vitest's
vi.fn() - No Jest configuration files or imports from Jest are present
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining Jest mock functions that might have been missed
rg "jest\.fn\(\)" -g "*.{ts,tsx,js,jsx}"
Length of output: 42
Script:
#!/bin/bash
# Let's try a more comprehensive search for Jest-related mocks and functions
rg "jest\." -g "*.{ts,tsx,js,jsx}" -A 2
# Also search for potential mock function imports from Jest
rg "from ['|\"]jest['|\"]" -g "*.{ts,tsx,js,jsx}"
# Search for Jest configuration files
fd "jest.config" --extension js --extension ts --extension json
# Let's also verify the presence of Vitest mocks for comparison
rg "vi\.fn\(\)" -g "*.{ts,tsx,js,jsx}" -A 2
Length of output: 6412
plugins/json-extractor/src/parser.spec.ts (1)
Line range hint 6-9: Verify test data file existence.
The test suite depends on an external JSON file at '../ref/test-basic.json'. Let's ensure this file exists in the repository.
✅ Verification successful
Test data file exists at the expected location
The test data file test-basic.json is present at the expected path plugins/json-extractor/ref/test-basic.json, confirming that the test suite's external dependency is properly maintained in the repository.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the test data file exists
fd -t f "test-basic.json" plugins/json-extractor/ref/
Length of output: 96
plugins/webhook-egress/src/webhook.egress.e2e.spec.ts (3)
5-19: LGTM: Clean transition to Vitest imports.
The import changes appropriately set up Vitest testing infrastructure while maintaining the necessary Flatfile testing utilities.
25-43: LGTM: Robust test infrastructure setup.
The implementation follows testing best practices with proper initialization, mounting, and cleanup of test resources.
Line range hint 111-120: LGTM: Robust error handling implementation.
The error logging spy and verification are well implemented, providing good coverage of error scenarios while maintaining clean test code.
plugins/delimiter-extractor/src/parser.spec.ts (3)
3-3: LGTM: Correct Vitest import statement.
The import statement correctly includes all necessary Vitest testing utilities that replace their Jest counterparts.
133-133: LGTM: Correct migration of spy implementation to Vitest.
The spy implementation has been correctly updated to use Vitest's vi.spyOn instead of Jest's jest.spyOn, maintaining the same functionality.
Line range hint 1-241: Well-executed migration from Jest to Vitest.
The transition to Vitest has been implemented cleanly with minimal changes, preserving all test functionality. The test suite remains comprehensive, covering various parsing scenarios including:
- Basic and complex delimiter parsing
- Dynamic typing
- Empty line handling with different strategies
- Empty buffer cases
- Data transformations
The migration follows best practices and maintains the original test coverage.
enrich/gpx/src/enrich.gpx.plugin.ts (3)
1-2: LGTM: Improved type imports.
The changes follow TypeScript best practices by explicitly marking type imports, which helps reduce bundle size by ensuring types are removed during compilation.
202-212: LGTM: Improved function signature readability.
The multi-line formatting of the function signature and config destructuring improves code readability while maintaining functionality.
Line range hint 1-255: Verify test coverage during framework transition.
As this change is part of the transition from Jest to Vitest, please ensure that all existing test cases have been migrated and are passing with the new testing framework.
Let's verify the test coverage:
✅ Verification successful
Test coverage is complete and properly migrated to Vitest
The test file shows comprehensive coverage of all exported utility functions with proper Vitest imports and test cases:
calculateDistance: Tests distance calculation accuracyremoveDuplicatePoints: Tests duplicate point filteringfilterByDateRange: Tests date-based filteringcalculateStatistics: Tests distance and elevation calculationsconvertToTabularFormat: Tests data structure conversion
No Jest remnants were found, and all tests are properly using Vitest's testing framework.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for the existence of test files and their content
# Test 1: Look for test files
echo "Looking for test files..."
fd -e spec.ts -e test.ts . enrich/gpx/
# Test 2: Check for Vitest usage in test files
echo "Checking for Vitest usage..."
rg -l "import.*vitest" enrich/gpx/
Length of output: 315
Script:
#!/bin/bash
# Let's examine the test file content to verify coverage
echo "Checking test file content..."
cat enrich/gpx/src/enrich.gpx.plugin.spec.ts
# Also check if there are any Jest remnants
echo -e "\nChecking for Jest remnants..."
rg "jest" enrich/gpx/
Length of output: 3359
plugins/record-hook/src/RecordHook.e2e.spec.ts (1)
1-45: Well-structured test setup with proper lifecycle management
The migration to Vitest is well implemented with:
- Clean separation of test infrastructure using TestListener and Driver
- Proper resource cleanup in lifecycle hooks
- Consistent async/await usage
| import dotenv from 'dotenv' | ||
| import fs from 'fs' | ||
| import path from 'path' | ||
|
|
||
| const loadEnv = (mode) => { |
There was a problem hiding this comment.
Add input validation for the mode parameter.
The mode parameter should be validated to ensure it's a non-empty string and contains only allowed characters.
const loadEnv = (mode) => {
+ if (typeof mode !== 'string' || !mode.trim()) {
+ throw new TypeError('Mode parameter must be a non-empty string');
+ }
+ if (!/^[\w-]+$/.test(mode)) {
+ throw new Error('Mode parameter contains invalid characters');
+ }📝 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.
| import dotenv from 'dotenv' | |
| import fs from 'fs' | |
| import path from 'path' | |
| const loadEnv = (mode) => { | |
| import dotenv from 'dotenv' | |
| import fs from 'fs' | |
| import path from 'path' | |
| const loadEnv = (mode) => { | |
| if (typeof mode !== 'string' || !mode.trim()) { | |
| throw new TypeError('Mode parameter must be a non-empty string'); | |
| } | |
| if (!/^[\w-]+$/.test(mode)) { | |
| throw new Error('Mode parameter contains invalid characters'); | |
| } |
| // Resolve environment variables that reference other variables | ||
| Object.keys(env).forEach((key) => { | ||
| const value = env[key] | ||
| if ( | ||
| typeof value === 'string' && | ||
| value.startsWith('${') && | ||
| value.endsWith('}') | ||
| ) { | ||
| const referencedKey = value.slice(2, -1) | ||
| if (env[referencedKey]) { | ||
| env[key] = env[referencedKey] | ||
| } | ||
| } | ||
| }) |
There was a problem hiding this comment.
Enhance environment variable reference resolution.
The current implementation has several limitations:
- No protection against circular references (e.g., A=${B}, B=${A})
- Only resolves one level of references
- No maximum depth limit for nested references
Consider implementing a more robust solution:
+ const resolveEnvValue = (value, visited = new Set(), depth = 0) => {
+ const MAX_DEPTH = 10;
+
+ if (depth > MAX_DEPTH) {
+ throw new Error('Maximum reference depth exceeded');
+ }
+
+ if (typeof value !== 'string') return value;
+ if (!value.startsWith('${') || !value.endsWith('}')) return value;
+
+ const referencedKey = value.slice(2, -1);
+ if (visited.has(referencedKey)) {
+ throw new Error(`Circular reference detected: ${Array.from(visited).join(' -> ')} -> ${referencedKey}`);
+ }
+
+ if (!env[referencedKey]) return value;
+
+ visited.add(referencedKey);
+ return resolveEnvValue(env[referencedKey], visited, depth + 1);
+ };
+
- Object.keys(env).forEach((key) => {
- const value = env[key]
- if (
- typeof value === 'string' &&
- value.startsWith('${') &&
- value.endsWith('}')
- ) {
- const referencedKey = value.slice(2, -1)
- if (env[referencedKey]) {
- env[key] = env[referencedKey]
+ Object.keys(env).forEach((key) => {
+ try {
+ env[key] = resolveEnvValue(env[key]);
+ } catch (error) {
+ console.warn(`Failed to resolve reference for ${key}: ${error.message}`);
+ // Keep the original value
- }
})📝 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.
| // Resolve environment variables that reference other variables | |
| Object.keys(env).forEach((key) => { | |
| const value = env[key] | |
| if ( | |
| typeof value === 'string' && | |
| value.startsWith('${') && | |
| value.endsWith('}') | |
| ) { | |
| const referencedKey = value.slice(2, -1) | |
| if (env[referencedKey]) { | |
| env[key] = env[referencedKey] | |
| } | |
| } | |
| }) | |
| // Resolve environment variables that reference other variables | |
| const resolveEnvValue = (value, visited = new Set(), depth = 0) => { | |
| const MAX_DEPTH = 10; | |
| if (depth > MAX_DEPTH) { | |
| throw new Error('Maximum reference depth exceeded'); | |
| } | |
| if (typeof value !== 'string') return value; | |
| if (!value.startsWith('${') || !value.endsWith('}')) return value; | |
| const referencedKey = value.slice(2, -1); | |
| if (visited.has(referencedKey)) { | |
| throw new Error(`Circular reference detected: ${Array.from(visited).join(' -> ')} -> ${referencedKey}`); | |
| } | |
| if (!env[referencedKey]) return value; | |
| visited.add(referencedKey); | |
| return resolveEnvValue(env[referencedKey], visited, depth + 1); | |
| }; | |
| Object.keys(env).forEach((key) => { | |
| try { | |
| env[key] = resolveEnvValue(env[key]); | |
| } catch (error) { | |
| console.warn(`Failed to resolve reference for ${key}: ${error.message}`); | |
| // Keep the original value | |
| } | |
| }) |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (4)
plugins/job-handler/src/job.handler.e2e.spec.ts (1)
28-30: Add error handling for driver lifecycle operations.
The driver startup and shutdown operations should include error handling to ensure proper cleanup in case of failures.
beforeAll(async () => {
- await driver.start()
+ try {
+ await driver.start()
+ } catch (error) {
+ console.error('Failed to start driver:', error)
+ throw error
+ }
listener.mount(driver)
})
afterAll(() => {
- driver.shutdown()
+ try {
+ driver.shutdown()
+ } catch (error) {
+ console.error('Failed to shutdown driver:', error)
+ }
})Also applies to: 39-41
plugins/constraints/src/external.constraint.e2e.spec.ts (1)
Line range hint 42-61: Consolidate duplicate beforeEach hooks
There are two separate beforeEach hooks that could be combined to improve maintainability.
Apply this change:
beforeEach(() => {
+ listener.resetCount()
listener.use(
externalConstraint('test', (value, key, { record }) => {
if (value === 'John Doe') {
record.addError(key, 'No Johns please')
record.set(key, value.toUpperCase())
}
})
)
})
- beforeEach(() => {
- listener.resetCount()
- })🧰 Tools
🪛 Biome
[error] 59-61: Disallow duplicate setup and teardown hooks.
Disallow beforeEach duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
plugins/record-hook/src/RecordHook.e2e.spec.ts (2)
Line range hint 79-91: Consolidate duplicate afterEach blocks
There are two separate afterEach blocks that could be combined for better maintainability:
- afterEach(() => {
- listener.reset()
- })
-
- afterEach(async () => {
+ afterEach(async () => {
listener.reset()
logSpy.mockReset()
logErrorSpy.mockReset()
const records = await getRecords(sheetId)
if (records.length > 0) {
const ids = records.map((record) => record.id)
await api.records.delete(sheetId, { ids })
}
})Line range hint 426-428: Consider global timeout configuration
Some tests have explicit 15-second timeouts. Consider moving this configuration to the global Vitest config for consistency across all tests.
Instead of per-test timeouts:
- }, 15_000)
+ })Add to your vitest.config.ts:
export default defineConfig({
test: {
testTimeout: 15000
}
})Also applies to: 450-452
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (9)
- plugins/constraints/src/external.constraint.e2e.spec.ts (3 hunks)
- plugins/constraints/src/external.sheet.constraint.e2e.spec.ts (3 hunks)
- plugins/job-handler/src/job.handler.e2e.spec.ts (4 hunks)
- plugins/record-hook/src/RecordHook.e2e.spec.ts (4 hunks)
- plugins/space-configure/src/space.configure.e2e.spec.ts (3 hunks)
- plugins/webhook-egress/src/webhook.egress.e2e.spec.ts (6 hunks)
- plugins/webhook-event-forwarder/src/forward-webhook.e2e.spec.ts (3 hunks)
- plugins/zip-extractor/src/zip.extractor.e2e.spec.ts (2 hunks)
- validate/date/src/validate.date.plugin.e2e.spec.ts (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- plugins/webhook-egress/src/webhook.egress.e2e.spec.ts
- validate/date/src/validate.date.plugin.e2e.spec.ts
🧰 Additional context used
🪛 Biome
plugins/constraints/src/external.constraint.e2e.spec.ts
[error] 59-61: Disallow duplicate setup and teardown hooks.
Disallow beforeEach duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
🔇 Additional comments (21)
plugins/zip-extractor/src/zip.extractor.e2e.spec.ts (3)
6-8: LGTM: Clean migration of test framework imports
The import changes properly reflect the transition from Jest to Vitest while maintaining good organization.
Also applies to: 12-20
Line range hint 31-61: LGTM: Well-structured test lifecycle management
The lifecycle hooks are properly organized with clear responsibilities:
- beforeAll: Sets up test environment and initial data
- afterAll: Proper cleanup of resources
- beforeEach/afterEach: Maintains clean state between tests
26-27: LGTM: Clear test setup with new TestListener pattern
The test setup is clean and explicit. Let's verify this pattern is consistently applied across other test files.
✅ Verification successful
Let me analyze the test files further to verify the actual implementation pattern.
Pattern is consistently applied across e2e test files
The verification shows that all e2e test files follow the same pattern of initializing both TestListener and setupDriver() together. The implementation in zip.extractor.e2e.spec.ts matches the established pattern across the codebase, where:
TestListeneris instantiated firstsetupDriver()is called immediately after- Both are declared as constants at the test suite level
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify consistent usage of TestListener and setupDriver pattern
# across test files
# Search for test files using TestListener
echo "Files using TestListener:"
rg -l "TestListener" --type ts
# Search for test files using setupDriver
echo "\nFiles using setupDriver:"
rg -l "setupDriver" --type ts
# Look for potential inconsistencies where one is used without the other
echo "\nPotential inconsistencies (files using one but not both):"
rg -l "TestListener|setupDriver" --type ts | sort | uniq -u
Length of output: 2238
Script:
#!/bin/bash
# Check the actual usage pattern in e2e test files
echo "Checking TestListener and setupDriver usage patterns:"
rg -B2 -A2 "new TestListener\(\)" --type ts
echo "\nChecking setupDriver usage patterns:"
rg -B2 -A2 "setupDriver\(\)" --type ts
Length of output: 10145
plugins/constraints/src/external.sheet.constraint.e2e.spec.ts (3)
Line range hint 1-19: LGTM: Clean migration to Vitest imports
The import statements have been properly updated to use Vitest test runners and the new testing utilities.
22-24: LGTM: Clean implementation of the new test setup pattern
The implementation follows the recommended pattern using TestListener and setupDriver for better test isolation and control.
Line range hint 71-81: LGTM: Well-structured test implementation
The test case maintains good practices with proper async/await usage and clear expectations.
plugins/job-handler/src/job.handler.e2e.spec.ts (2)
1-16: Clean transition to Vitest and improved test utilities.
The migration from Jest to Vitest has been implemented correctly, with appropriate imports. The introduction of TestListener and setupDriver from @flatfile/utils-testing suggests improved test infrastructure.
Line range hint 21-48: Previous refactoring suggestion for duplicate test setup remains valid.
The test setup code is still duplicated between the two describe blocks, as noted in the previous review.
Also applies to: 59-92
plugins/space-configure/src/space.configure.e2e.spec.ts (4)
2-17: LGTM: Clean migration of test imports
The transition from Jest to Vitest is properly implemented with all necessary imports.
Line range hint 57-77: Previous comment about consolidating lifecycle hooks is still applicable
The suggestion to consolidate the beforeAll hooks for better maintainability remains valid.
84-94: Verify the safety of non-null assertions
The addition of non-null assertions (!) assumes these properties will always exist. We should verify this assumption by checking the contract with the API.
#!/bin/bash
# Description: Verify the presence of required properties in API responses
# Test 1: Check Space type definition
rg -A 10 "interface Space"
# Test 2: Check Workbook type definition
rg -A 15 "interface Workbook"
# Test 3: Check actual usage patterns
rg "primaryWorkbookId|labels|sheets|actions" --type ts51-53: Verify TestListener implementation compatibility
The transition to TestListener looks good, but let's verify its implementation matches the previous listener functionality.
✅ Verification successful
Let me gather more specific information about TestListener.
Let me check the complete implementation of TestListener to verify its functionality.
TestListener implementation is fully compatible and well-designed
The TestListener implementation extends FlatfileListener with robust event tracking capabilities, including:
- Event invocation counting and history tracking
- Async event waiting with filtering support
- Proper state management with reset capabilities
- Full compatibility with the base FlatfileListener class
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify TestListener implementation matches required functionality
# Test: Search for TestListener class definition and its usage patterns
ast-grep --pattern 'class TestListener {
$$$
mount($_) {
$$$
}
$$$
reset() {
$$$
}
$$$
resetCount() {
$$$
}
$$$
}'
Length of output: 12639
Script:
#!/bin/bash
# Search for TestListener class definition and its usage
rg -A 10 "class TestListener" --type ts
Length of output: 824
Script:
#!/bin/bash
# Get the complete TestListener implementation
cat utils/testing/src/test.listener.ts
Length of output: 3077
plugins/constraints/src/external.constraint.e2e.spec.ts (3)
6-19: LGTM: Imports correctly updated for Vitest
The imports have been properly updated to use Vitest testing functions and the necessary testing utilities.
22-24: Remove .skip from the test suite
The test suite is currently skipped but appears to be fully migrated to Vitest. The implementation looks correct and matches the functionality in external.constraint.ts.
Line range hint 67-120: LGTM: Test cases are well-structured
The test cases provide good coverage of the main functionality:
- Error assignment
- Error handling
- Value setting
The test data and schema are properly exported for reuse.
🧰 Tools
🪛 Biome
[error] 59-61: Disallow duplicate setup and teardown hooks.
Disallow beforeEach duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
plugins/webhook-event-forwarder/src/forward-webhook.e2e.spec.ts (4)
5-19: LGTM: Clean migration to Vitest imports.
The transition from Jest to Vitest is well-implemented with appropriate imports.
23-24: LGTM: Clean TestListener and driver setup.
The initialization and mounting of the TestListener with the driver is well-implemented.
Also applies to: 31-32
60-60: LGTM: Secure webhook URL.
Good security practice using HTTPS protocol for the webhook URL.
Line range hint 94-116: Verify error handling implementation.
The error handling test case looks good, but let's verify that all error scenarios are covered.
✅ Verification successful
Error handling test coverage appears sufficient
Based on the test file examination, the error handling test case is comprehensive, covering:
- HTTP 500 error response scenario
- Proper error message propagation
- Response header validation
- Error state in callback results
The test adequately verifies the error path by:
- Mocking a failed HTTP response (status 500)
- Validating the error message structure
- Checking the error flag in the callback data
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other error handling test cases in webhook-related files
rg -l "should error" | xargs rg "global.fetch.*mockResolvedValue.*status.*5"
Length of output: 78
Script:
#!/bin/bash
# Let's first find all webhook-related test files
fd ".*\.spec\.(ts|js)" | grep -i "webhook"
# Then search for error handling patterns in these files
rg "global\.fetch.*mock.*status.*"
rg "should.*error" -A 5
Length of output: 9495
Script:
#!/bin/bash
# Let's examine the webhook-related test files more closely for error scenarios
rg -A 10 "it\('should.*error" plugins/webhook-event-forwarder/src/forward-webhook.e2e.spec.ts
rg -A 10 "it\('should.*error" plugins/webhook-egress/src/webhook.egress.e2e.spec.ts
# Also check the implementation files for error handling
rg "catch|throw|Error" plugins/webhook-event-forwarder/src/forward-webhook.ts
rg "catch|throw|Error" plugins/webhook-egress/src/webhook.egress.ts
Length of output: 942
Script:
#!/bin/bash
# Let's check for the forward-webhook implementation file in different locations
fd "forward-webhook.ts" --exclude "*.spec.ts"
# And examine its content for error handling patterns
fd "forward-webhook.ts" --exclude "*.spec.ts" --exec cat {}
Length of output: 107
plugins/record-hook/src/RecordHook.e2e.spec.ts (2)
1-28: LGTM! Improved test setup with better type safety
The transition to Vitest includes several improvements:
- Type imports enhance type safety
- TestListener provides better test isolation
- Explicit driver setup improves test control
Line range hint 93-452: LGTM! Well-structured and comprehensive tests
The test implementation demonstrates good practices:
- Clear Arrange-Act-Assert pattern
- Proper async/await usage
- Comprehensive error handling coverage
- Thorough cleanup
| afterAll(async () => { | ||
| await deleteSpace(spaceId) | ||
|
|
||
| driver.shutdown() | ||
| }) |
There was a problem hiding this comment.
Consider adding await to driver.shutdown()
The driver shutdown in afterAll might create a race condition if the operation is asynchronous. Consider adding await to ensure proper cleanup:
afterAll(async () => {
await deleteSpace(spaceId)
- driver.shutdown()
+ await driver.shutdown()
})📝 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.
| afterAll(async () => { | |
| await deleteSpace(spaceId) | |
| driver.shutdown() | |
| }) | |
| afterAll(async () => { | |
| await deleteSpace(spaceId) | |
| await driver.shutdown() | |
| }) |
bangarang
force-pushed
the
feat/vitest
branch
from
8ff5566 to
8d50ed7
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (4)
plugins/space-configure/src/space.configure.e2e.spec.ts (1)
67-70: Ensure proper cleanup order in afterAllConsider awaiting the driver shutdown to prevent potential memory leaks:
afterAll(async () => { await deleteSpace(spaceId) - driver.shutdown() + await driver.shutdown() })plugins/constraints/src/external.constraint.e2e.spec.ts (1)
Line range hint
42-64: Consolidate duplicatebeforeEachhooksThere are two separate
beforeEachhooks that should be consolidated to follow testing best practices and fix the static analysis warning.Apply this diff to combine the hooks:
beforeEach(() => { listener.use( externalConstraint('test', (value, key, { record }) => { if (value === 'John Doe') { record.addError(key, 'No Johns please') record.set(key, value.toUpperCase()) } }) ) + listener.resetCount() }) - beforeEach(() => { - listener.resetCount() - })🧰 Tools
🪛 Biome
[error] 59-61: Disallow duplicate setup and teardown hooks.
Disallow beforeEach duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
utils/testing/src/test.helpers.ts (1)
39-46: Enhance error message and align validation approach.While the validation is good, consider:
- Making the error message more descriptive
- Aligning the validation approach with getEnvironmentId for consistency
export function setupDriver() { const environmentId = getEnvironmentId() if (!environmentId) { - throw new Error('environmentId is not defined') + throw new Error('FLATFILE_ENVIRONMENT_ID environment variable is not configured') } return new PubSubDriver(environmentId) }plugins/graphql-schema/src/generate.field.spec.ts (1)
Line range hint
94-103: Remove duplicate test caseThis test case "should handle fields with undefined descriptions" is an exact duplicate of the previous test case. Consider removing one of them to maintain test clarity and avoid redundancy.
- it('should handle fields with undefined descriptions', () => { - const field = { name: 'age', type: { kind: 'SCALAR', name: 'Int' } } - const expected = { - key: 'age', - label: 'age', - type: 'number', - description: '', - constraints: [], - } - expect(generateField(field, 'Person')).toEqual(expected) - })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (60)
.github/workflows/lint.ymlis excluded by!**/*.ymlconfig/vitest/package.jsonis excluded by!**/*.jsonconvert/currency/package.jsonis excluded by!**/*.jsonconvert/translate/package.jsonis excluded by!**/*.jsonconvert/what3words/package.jsonis excluded by!**/*.jsonenrich/geocode/package.jsonis excluded by!**/*.jsonenrich/gpx/package.jsonis excluded by!**/*.jsonenrich/sentiment/package.jsonis excluded by!**/*.jsonenrich/summarize/package.jsonis excluded by!**/*.jsonexport/pivot-table/package.jsonis excluded by!**/*.jsonextract/html-table/package.jsonis excluded by!**/*.jsonextract/markdown/package.jsonis excluded by!**/*.jsonimport/faker/package.jsonis excluded by!**/*.jsonimport/llm-records/package.jsonis excluded by!**/*.jsonimport/rss/package.jsonis excluded by!**/*.jsonpackage-lock.jsonis excluded by!**/package-lock.json,!**/*.jsonpackage.jsonis excluded by!**/*.jsonplugins/autocast/package.jsonis excluded by!**/*.jsonplugins/automap/package.jsonis excluded by!**/*.jsonplugins/constraints/package.jsonis excluded by!**/*.jsonplugins/dedupe/package.jsonis excluded by!**/*.jsonplugins/delimiter-extractor/package.jsonis excluded by!**/*.jsonplugins/dxp-configure/package.jsonis excluded by!**/*.jsonplugins/export-workbook/package.jsonis excluded by!**/*.jsonplugins/foreign-db-extractor/package.jsonis excluded by!**/*.jsonplugins/graphql-schema/package.jsonis excluded by!**/*.jsonplugins/job-handler/package.jsonis excluded by!**/*.jsonplugins/json-extractor/package.jsonis excluded by!**/*.jsonplugins/json-schema/package.jsonis excluded by!**/*.jsonplugins/merge-connection/package.jsonis excluded by!**/*.jsonplugins/openapi-schema/package.jsonis excluded by!**/*.jsonplugins/pdf-extractor/package.jsonis excluded by!**/*.jsonplugins/psv-extractor/package.jsonis excluded by!**/*.jsonplugins/record-hook/package.jsonis excluded by!**/*.jsonplugins/rollout/package.jsonis excluded by!**/*.jsonplugins/space-configure/package.jsonis excluded by!**/*.jsonplugins/sql-ddl-converter/package.jsonis excluded by!**/*.jsonplugins/tsv-extractor/package.jsonis excluded by!**/*.jsonplugins/view-mapped/package.jsonis excluded by!**/*.jsonplugins/webhook-egress/package.jsonis excluded by!**/*.jsonplugins/webhook-event-forwarder/package.jsonis excluded by!**/*.jsonplugins/xlsx-extractor/package.jsonis excluded by!**/*.jsonplugins/xml-extractor/package.jsonis excluded by!**/*.jsonplugins/yaml-schema/package.jsonis excluded by!**/*.jsonplugins/zip-extractor/package.jsonis excluded by!**/*.jsonsupport/common-utils/package.jsonis excluded by!**/*.jsonturbo.jsonis excluded by!**/*.jsonutils/common/package.jsonis excluded by!**/*.jsonutils/extractor/package.jsonis excluded by!**/*.jsonutils/fetch-schema/package.jsonis excluded by!**/*.jsonutils/file-buffer/package.jsonis excluded by!**/*.jsonutils/response-rejection/package.jsonis excluded by!**/*.jsonutils/testing/package.jsonis excluded by!**/*.jsonvalidate/boolean/package.jsonis excluded by!**/*.jsonvalidate/date/package.jsonis excluded by!**/*.jsonvalidate/email/package.jsonis excluded by!**/*.jsonvalidate/isbn/package.jsonis excluded by!**/*.jsonvalidate/number/package.jsonis excluded by!**/*.jsonvalidate/phone/package.jsonis excluded by!**/*.jsonvalidate/string/package.jsonis excluded by!**/*.json
📒 Files selected for processing (157)
.changeset/five-hounds-wash.md(1 hunks)config/vitest/src/index.js(1 hunks)config/vitest/src/utils.js(1 hunks)convert/currency/jest.config.cjs(0 hunks)convert/currency/src/convert.currency.plugin.spec.ts(1 hunks)convert/currency/src/convert.currency.plugin.ts(7 hunks)convert/currency/vitest.config.ts(1 hunks)convert/translate/jest.config.cjs(0 hunks)convert/translate/src/convert.translate.plugin.spec.ts(2 hunks)convert/translate/vitest.config.ts(1 hunks)convert/what3words/jest.config.cjs(0 hunks)convert/what3words/vitest.config.ts(1 hunks)enrich/geocode/jest.config.cjs(0 hunks)enrich/geocode/src/enrich.geocode.plugin.spec.ts(6 hunks)enrich/geocode/src/index.ts(1 hunks)enrich/geocode/vitest.config.ts(1 hunks)enrich/gpx/jest.config.cjs(0 hunks)enrich/gpx/src/enrich.gpx.plugin.spec.ts(3 hunks)enrich/gpx/src/enrich.gpx.plugin.ts(4 hunks)enrich/gpx/vitest.config.ts(1 hunks)enrich/sentiment/jest.config.cjs(0 hunks)enrich/sentiment/src/enrich.sentiment.plugin.spec.ts(4 hunks)enrich/sentiment/src/enrich.sentiment.plugin.ts(2 hunks)enrich/sentiment/vitest.config.ts(1 hunks)enrich/summarize/jest.config.cjs(0 hunks)enrich/summarize/src/summarize.plugin.spec.ts(1 hunks)enrich/summarize/src/summarize.plugin.ts(2 hunks)enrich/summarize/vitest.config.ts(1 hunks)export/pivot-table/jest.config.cjs(0 hunks)export/pivot-table/vitest.config.ts(1 hunks)extract/html-table/jest.config.cjs(0 hunks)extract/html-table/vitest.config.ts(1 hunks)extract/markdown/jest.config.cjs(0 hunks)extract/markdown/src/parser.spec.ts(1 hunks)extract/markdown/vitest.config.ts(1 hunks)import/faker/jest.config.cjs(0 hunks)import/faker/src/faker.plugin.spec.ts(4 hunks)import/faker/src/faker.utils.ts(2 hunks)import/faker/vitest.config.ts(1 hunks)import/llm-records/jest.config.cjs(0 hunks)import/llm-records/vitest.config.ts(1 hunks)import/rss/jest.config.cjs(0 hunks)import/rss/vitest.config.ts(1 hunks)jest.config.js(0 hunks)jest.setup.js(0 hunks)plugins/autocast/jest.config.cjs(0 hunks)plugins/autocast/src/autocast.plugin.spec.ts(2 hunks)plugins/autocast/vitest.config.ts(1 hunks)plugins/automap/jest.config.cjs(0 hunks)plugins/automap/src/automap.plugin.e2e.spec.ts(2 hunks)plugins/automap/vitest.config.ts(1 hunks)plugins/constraints/jest.config.cjs(0 hunks)plugins/constraints/src/external.constraint.e2e.spec.ts(3 hunks)plugins/constraints/src/external.sheet.constraint.e2e.spec.ts(3 hunks)plugins/constraints/vitest.config.ts(1 hunks)plugins/dedupe/jest.config.cjs(0 hunks)plugins/dedupe/src/keep.first.logic.spec.ts(1 hunks)plugins/dedupe/src/keep.last.logic.spec.ts(1 hunks)plugins/dedupe/vitest.config.ts(1 hunks)plugins/delimiter-extractor/jest.config.cjs(0 hunks)plugins/delimiter-extractor/src/parser.spec.ts(2 hunks)plugins/delimiter-extractor/vitest.config.ts(1 hunks)plugins/dxp-configure/jest.config.cjs(0 hunks)plugins/dxp-configure/src/shim.target.spec.ts(1 hunks)plugins/dxp-configure/vitest.config.ts(1 hunks)plugins/export-workbook/jest.config.cjs(0 hunks)plugins/export-workbook/vitest.config.ts(1 hunks)plugins/foreign-db-extractor/jest.config.cjs(0 hunks)plugins/foreign-db-extractor/vitest.config.ts(1 hunks)plugins/graphql-schema/jest.config.cjs(0 hunks)plugins/graphql-schema/src/generate.field.spec.ts(3 hunks)plugins/graphql-schema/vitest.config.ts(1 hunks)plugins/job-handler/jest.config.cjs(0 hunks)plugins/job-handler/src/job.handler.e2e.spec.ts(4 hunks)plugins/job-handler/vitest.config.ts(1 hunks)plugins/json-extractor/jest.config.cjs(0 hunks)plugins/json-extractor/src/parser.spec.ts(1 hunks)plugins/json-extractor/vitest.config.ts(1 hunks)plugins/json-schema/jest.config.cjs(0 hunks)plugins/json-schema/src/setup.factory.spec.ts(1 hunks)plugins/json-schema/vitest.config.ts(1 hunks)plugins/merge-connection/jest.config.cjs(0 hunks)plugins/merge-connection/vitest.config.ts(1 hunks)plugins/openapi-schema/jest.config.cjs(0 hunks)plugins/openapi-schema/src/setup.factory.openapi.spec.ts(1 hunks)plugins/openapi-schema/vitest.config.ts(1 hunks)plugins/pdf-extractor/jest.config.cjs(0 hunks)plugins/pdf-extractor/vitest.config.ts(1 hunks)plugins/psv-extractor/jest.config.cjs(0 hunks)plugins/psv-extractor/vitest.config.ts(1 hunks)plugins/record-hook/jest.config.cjs(0 hunks)plugins/record-hook/src/RecordHook.e2e.spec.ts(4 hunks)plugins/record-hook/vitest.config.ts(1 hunks)plugins/rollout/jest.config.cjs(0 hunks)plugins/rollout/vitest.config.ts(1 hunks)plugins/space-configure/jest.config.cjs(0 hunks)plugins/space-configure/src/space.configure.e2e.spec.ts(3 hunks)plugins/space-configure/vitest.config.ts(1 hunks)plugins/sql-ddl-converter/jest.config.cjs(0 hunks)plugins/sql-ddl-converter/vitest.config.ts(1 hunks)plugins/tsv-extractor/jest.config.cjs(0 hunks)plugins/tsv-extractor/vitest.config.ts(1 hunks)plugins/view-mapped/jest.config.cjs(0 hunks)plugins/view-mapped/vitest.config.ts(1 hunks)plugins/webhook-egress/jest.config.cjs(0 hunks)plugins/webhook-egress/src/webhook.egress.e2e.spec.ts(6 hunks)plugins/webhook-egress/vitest.config.ts(1 hunks)plugins/webhook-event-forwarder/jest.config.cjs(0 hunks)plugins/webhook-event-forwarder/src/forward-webhook.e2e.spec.ts(3 hunks)plugins/webhook-event-forwarder/vitest.config.ts(1 hunks)plugins/xlsx-extractor/jest.config.cjs(0 hunks)plugins/xlsx-extractor/src/parser.spec.ts(1 hunks)plugins/xlsx-extractor/src/utils.spec.ts(1 hunks)plugins/xlsx-extractor/vitest.config.ts(1 hunks)plugins/xml-extractor/jest.config.cjs(0 hunks)plugins/xml-extractor/src/parser.spec.ts(1 hunks)plugins/xml-extractor/vitest.config.ts(1 hunks)plugins/yaml-schema/jest.config.cjs(0 hunks)plugins/yaml-schema/src/setup.factory.spec.ts(2 hunks)plugins/yaml-schema/vitest.config.ts(1 hunks)plugins/zip-extractor/jest.config.cjs(0 hunks)plugins/zip-extractor/src/zip.extractor.e2e.spec.ts(2 hunks)plugins/zip-extractor/vitest.config.ts(1 hunks)support/common-utils/src/async.map.spec.ts(2 hunks)support/common-utils/vitest.config.ts(1 hunks)test/betterConsoleLog.js(0 hunks)test/dotenv-config.js(0 hunks)test/setup-global.js(0 hunks)test/toBePendingMatcher.ts(0 hunks)test/unit.cleanup.js(0 hunks)utils/common/jest.config.cjs(0 hunks)utils/common/src/all.records.e2e.spec.ts(7 hunks)utils/common/src/async.batch.spec.ts(6 hunks)utils/common/src/logging.helper.spec.ts(4 hunks)utils/common/src/slugify.spec.ts(1 hunks)utils/common/vitest.config.ts(1 hunks)utils/extractor/jest.config.cjs(0 hunks)utils/extractor/vitest.config.ts(1 hunks)utils/fetch-schema/jest.config.cjs(0 hunks)utils/fetch-schema/vitest.config.ts(1 hunks)utils/file-buffer/jest.config.cjs(0 hunks)utils/file-buffer/vitest.config.ts(1 hunks)utils/response-rejection/jest.config.cjs(0 hunks)utils/response-rejection/vitest.config.ts(1 hunks)utils/testing/jest.config.cjs(0 hunks)utils/testing/src/test.helpers.ts(3 hunks)utils/testing/src/test.listener.spec.ts(2 hunks)utils/testing/src/toBePendingMatcher.ts(1 hunks)utils/testing/vitest.config.ts(1 hunks)validate/boolean/jest.config.cjs(0 hunks)validate/boolean/src/validate.boolean.plugin.spec.ts(6 hunks)validate/boolean/vitest.config.ts(1 hunks)validate/date/jest.config.cjs(0 hunks)validate/date/src/validate.date.plugin.e2e.spec.ts(2 hunks)validate/date/vitest.config.ts(1 hunks)validate/email/jest.config.cjs(0 hunks)validate/email/src/validate.email.plugin.ts(1 hunks)
⛔ Files not processed due to max files limit (15)
- validate/email/src/validate.email.utils.spec.ts
- validate/email/src/validate.email.utils.ts
- validate/email/vitest.config.ts
- validate/isbn/jest.config.cjs
- validate/isbn/src/validate.isbn.plugin.spec.ts
- validate/isbn/vitest.config.ts
- validate/number/jest.config.cjs
- validate/number/src/validate.number.plugin.spec.ts
- validate/number/vitest.config.ts
- validate/phone/jest.config.cjs
- validate/phone/src/validate.phone.utils.spec.ts
- validate/phone/vitest.config.ts
- validate/string/jest.config.cjs
- validate/string/src/validate.string.plugin.spec.ts
- validate/string/vitest.config.ts
💤 Files with no reviewable changes (57)
- convert/currency/jest.config.cjs
- convert/translate/jest.config.cjs
- convert/what3words/jest.config.cjs
- enrich/geocode/jest.config.cjs
- enrich/gpx/jest.config.cjs
- enrich/sentiment/jest.config.cjs
- enrich/summarize/jest.config.cjs
- export/pivot-table/jest.config.cjs
- extract/html-table/jest.config.cjs
- extract/markdown/jest.config.cjs
- import/faker/jest.config.cjs
- import/llm-records/jest.config.cjs
- import/rss/jest.config.cjs
- jest.config.js
- jest.setup.js
- plugins/autocast/jest.config.cjs
- plugins/automap/jest.config.cjs
- plugins/constraints/jest.config.cjs
- plugins/dedupe/jest.config.cjs
- plugins/delimiter-extractor/jest.config.cjs
- plugins/dxp-configure/jest.config.cjs
- plugins/export-workbook/jest.config.cjs
- plugins/foreign-db-extractor/jest.config.cjs
- plugins/graphql-schema/jest.config.cjs
- plugins/job-handler/jest.config.cjs
- plugins/json-extractor/jest.config.cjs
- plugins/json-schema/jest.config.cjs
- plugins/merge-connection/jest.config.cjs
- plugins/openapi-schema/jest.config.cjs
- plugins/pdf-extractor/jest.config.cjs
- plugins/psv-extractor/jest.config.cjs
- plugins/record-hook/jest.config.cjs
- plugins/rollout/jest.config.cjs
- plugins/space-configure/jest.config.cjs
- plugins/sql-ddl-converter/jest.config.cjs
- plugins/tsv-extractor/jest.config.cjs
- plugins/view-mapped/jest.config.cjs
- plugins/webhook-egress/jest.config.cjs
- plugins/webhook-event-forwarder/jest.config.cjs
- plugins/xlsx-extractor/jest.config.cjs
- plugins/xml-extractor/jest.config.cjs
- plugins/yaml-schema/jest.config.cjs
- plugins/zip-extractor/jest.config.cjs
- test/betterConsoleLog.js
- test/dotenv-config.js
- test/setup-global.js
- test/toBePendingMatcher.ts
- test/unit.cleanup.js
- utils/common/jest.config.cjs
- utils/extractor/jest.config.cjs
- utils/fetch-schema/jest.config.cjs
- utils/file-buffer/jest.config.cjs
- utils/response-rejection/jest.config.cjs
- utils/testing/jest.config.cjs
- validate/boolean/jest.config.cjs
- validate/date/jest.config.cjs
- validate/email/jest.config.cjs
🚧 Files skipped from review as they are similar to previous changes (94)
- .changeset/five-hounds-wash.md
- config/vitest/src/index.js
- config/vitest/src/utils.js
- convert/currency/src/convert.currency.plugin.spec.ts
- convert/currency/src/convert.currency.plugin.ts
- convert/currency/vitest.config.ts
- convert/translate/src/convert.translate.plugin.spec.ts
- convert/translate/vitest.config.ts
- convert/what3words/vitest.config.ts
- enrich/geocode/src/enrich.geocode.plugin.spec.ts
- enrich/geocode/src/index.ts
- enrich/geocode/vitest.config.ts
- enrich/gpx/src/enrich.gpx.plugin.spec.ts
- enrich/gpx/vitest.config.ts
- enrich/sentiment/src/enrich.sentiment.plugin.spec.ts
- enrich/sentiment/src/enrich.sentiment.plugin.ts
- enrich/sentiment/vitest.config.ts
- enrich/summarize/src/summarize.plugin.spec.ts
- enrich/summarize/src/summarize.plugin.ts
- enrich/summarize/vitest.config.ts
- export/pivot-table/vitest.config.ts
- extract/html-table/vitest.config.ts
- extract/markdown/src/parser.spec.ts
- extract/markdown/vitest.config.ts
- import/faker/src/faker.plugin.spec.ts
- import/faker/src/faker.utils.ts
- import/faker/vitest.config.ts
- import/llm-records/vitest.config.ts
- import/rss/vitest.config.ts
- plugins/autocast/src/autocast.plugin.spec.ts
- plugins/autocast/vitest.config.ts
- plugins/automap/src/automap.plugin.e2e.spec.ts
- plugins/automap/vitest.config.ts
- plugins/constraints/src/external.sheet.constraint.e2e.spec.ts
- plugins/constraints/vitest.config.ts
- plugins/dedupe/src/keep.first.logic.spec.ts
- plugins/dedupe/src/keep.last.logic.spec.ts
- plugins/dedupe/vitest.config.ts
- plugins/delimiter-extractor/src/parser.spec.ts
- plugins/delimiter-extractor/vitest.config.ts
- plugins/dxp-configure/src/shim.target.spec.ts
- plugins/dxp-configure/vitest.config.ts
- plugins/export-workbook/vitest.config.ts
- plugins/foreign-db-extractor/vitest.config.ts
- plugins/graphql-schema/vitest.config.ts
- plugins/job-handler/src/job.handler.e2e.spec.ts
- plugins/job-handler/vitest.config.ts
- plugins/json-extractor/src/parser.spec.ts
- plugins/json-extractor/vitest.config.ts
- plugins/json-schema/src/setup.factory.spec.ts
- plugins/json-schema/vitest.config.ts
- plugins/merge-connection/vitest.config.ts
- plugins/openapi-schema/src/setup.factory.openapi.spec.ts
- plugins/openapi-schema/vitest.config.ts
- plugins/pdf-extractor/vitest.config.ts
- plugins/psv-extractor/vitest.config.ts
- plugins/record-hook/src/RecordHook.e2e.spec.ts
- plugins/record-hook/vitest.config.ts
- plugins/rollout/vitest.config.ts
- plugins/space-configure/vitest.config.ts
- plugins/sql-ddl-converter/vitest.config.ts
- plugins/tsv-extractor/vitest.config.ts
- plugins/view-mapped/vitest.config.ts
- plugins/webhook-egress/src/webhook.egress.e2e.spec.ts
- plugins/webhook-egress/vitest.config.ts
- plugins/webhook-event-forwarder/src/forward-webhook.e2e.spec.ts
- plugins/webhook-event-forwarder/vitest.config.ts
- plugins/xlsx-extractor/src/parser.spec.ts
- plugins/xlsx-extractor/src/utils.spec.ts
- plugins/xlsx-extractor/vitest.config.ts
- plugins/xml-extractor/src/parser.spec.ts
- plugins/xml-extractor/vitest.config.ts
- plugins/yaml-schema/src/setup.factory.spec.ts
- plugins/yaml-schema/vitest.config.ts
- plugins/zip-extractor/vitest.config.ts
- support/common-utils/src/async.map.spec.ts
- support/common-utils/vitest.config.ts
- utils/common/src/all.records.e2e.spec.ts
- utils/common/src/async.batch.spec.ts
- utils/common/src/logging.helper.spec.ts
- utils/common/src/slugify.spec.ts
- utils/common/vitest.config.ts
- utils/extractor/vitest.config.ts
- utils/fetch-schema/vitest.config.ts
- utils/file-buffer/vitest.config.ts
- utils/response-rejection/vitest.config.ts
- utils/testing/src/test.listener.spec.ts
- utils/testing/src/toBePendingMatcher.ts
- utils/testing/vitest.config.ts
- validate/boolean/src/validate.boolean.plugin.spec.ts
- validate/boolean/vitest.config.ts
- validate/date/src/validate.date.plugin.e2e.spec.ts
- validate/date/vitest.config.ts
- validate/email/src/validate.email.plugin.ts
🧰 Additional context used
🪛 Biome
enrich/gpx/src/enrich.gpx.plugin.ts
[error] 183-183: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.
(lint/suspicious/noGlobalIsNan)
[error] 183-183: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.
(lint/suspicious/noGlobalIsNan)
plugins/constraints/src/external.constraint.e2e.spec.ts
[error] 59-61: Disallow duplicate setup and teardown hooks.
Disallow beforeEach duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
🔇 Additional comments (18)
plugins/zip-extractor/src/zip.extractor.e2e.spec.ts (3)
6-20: LGTM: Clean migration of test framework imports
The import changes appropriately reflect the migration from Jest to Vitest while introducing new testing utilities that improve the test infrastructure.
26-27: LGTM: Improved test setup with TestListener class
The transition to a class-based TestListener and explicit driver setup improves state management and makes the test setup more maintainable.
Line range hint 32-61: Consider improving test lifecycle management further
While the hooks now properly manage the driver lifecycle and listener state, there's still room for improvement in the organization of the test setup.
In addition to consolidating the beforeAll hooks as suggested in the previous review, consider this enhanced structure:
beforeAll(async () => {
await driver.start()
listener.mount(driver)
- })
-
- afterAll(async () => {
- await deleteSpace(spaceId)
-
- driver.shutdown()
- })
-
- beforeEach(() => {
- listener.resetCount()
- })
-
- afterEach(() => {
- listener.reset()
- })
-
- let spaceId
-
- beforeAll(async () => {
const environmentId = getEnvironmentId()!
const space = await setupSpace()
spaceId = space.id
listener.use(ZipExtractor())
const stream = fs.createReadStream(
path.join(__dirname, '../ref/getting-started-flat.zip')
)
await api.files.upload(stream, {
spaceId,
environmentId,
})
})
afterAll(async () => {
await Promise.all([
deleteSpace(spaceId),
driver.shutdown()
])
})
beforeEach(() => {
listener.resetCount()
listener.reset()
})This refactoring:
- Consolidates the beforeAll hooks
- Parallelizes cleanup operations in afterAll
- Combines listener reset operations in beforeEach
- Removes redundant afterEach
plugins/space-configure/src/space.configure.e2e.spec.ts (4)
2-17: LGTM: Clean transition to Vitest imports
The migration from Jest to Vitest is properly implemented with appropriate imports. The new testing utilities from @flatfile/utils-testing are correctly imported.
51-53: LGTM: Well-structured test setup
The test configuration properly utilizes Vitest's mocking capabilities and the new testing utilities.
83-94: 🛠️ Refactor suggestion
Consider improving test robustness
The current implementation has several areas that could be enhanced:
- The non-null assertions (!) might mask potential runtime issues
- Multiple assertions without custom error messages make debugging harder
- Consider breaking down the complex validation into smaller, focused test cases
Here's a suggested improvement:
const space = await api.spaces.get(spaceId)
- const workspace = await api.workbooks.get(space.data.primaryWorkbookId!)
+ const primaryWorkbookId = space.data.primaryWorkbookId
+ expect(primaryWorkbookId, 'Primary workbook ID should exist').toBeDefined()
+ const workspace = await api.workbooks.get(primaryWorkbookId)
- expect(workspace.data.name).toBe(setup.workbooks[0].name)
- expect(workspace.data.labels).toMatchObject(setup.workbooks[0].labels!)
+ expect(workspace.data.name, 'Workspace name should match setup').toBe(setup.workbooks[0].name)
+
+ const expectedLabels = setup.workbooks[0].labels
+ expect(expectedLabels, 'Setup should include labels').toBeDefined()
+ expect(workspace.data.labels, 'Workspace should have matching labels').toMatchObject(expectedLabels)Consider splitting this into multiple test cases:
- "should configure workspace name and labels"
- "should configure sheets"
- "should configure actions"
- "should trigger callback"
52-53: Verify consistent testing patterns across the codebase
Let's ensure the new testing utilities are used consistently across all migrated test files.
✅ Verification successful
Testing utilities are consistently used across e2e test files
The verification shows that TestListener and setupDriver are used consistently across all e2e test files in the codebase, following the same pattern:
- Both utilities are initialized together at the test setup
- The pattern
const listener = new TestListener()followed byconst driver = setupDriver()is uniform across all files - This includes tests in plugins (automap, constraints, webhook, etc.) and validate modules
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for consistent usage of TestListener and setupDriver
echo "Checking TestListener usage..."
rg "new TestListener\(\)" -A 2 -g "*.spec.ts"
echo "Checking setupDriver usage..."
rg "setupDriver\(\)" -A 2 -g "*.spec.ts"
Length of output: 5374
plugins/constraints/src/external.constraint.e2e.spec.ts (2)
6-19: LGTM: Import changes align with Vitest migration
The updated imports correctly reflect the transition from Jest to Vitest and introduce the new test infrastructure components.
30-32: LGTM: Robust test infrastructure setup
The new test infrastructure implementation with TestListener and driver setup follows best practices:
- Proper initialization in
beforeAll - Clean shutdown in
afterAll - Clear mounting of listener to driver
Also applies to: 55-57
utils/testing/src/test.helpers.ts (3)
4-4: LGTM: Vitest imports added correctly.
The addition of Vitest lifecycle hooks aligns with the migration from Jest to Vitest.
15-15: Previous review comment about environment ID validation is still applicable.
The current implementation could mask configuration issues by converting undefined to the string "undefined".
54-54: LGTM: Improved driver setup and proper test lifecycle management.
The changes improve code organization by centralizing driver setup and ensure proper resource cleanup through lifecycle hooks.
plugins/graphql-schema/src/generate.field.spec.ts (2)
1-1: LGTM: Correct Vitest imports
The transition from Jest to Vitest imports is implemented correctly, including all necessary testing utilities.
Line range hint 1-156: Overall implementation looks good
The migration to Vitest has been implemented correctly, and the test suite provides comprehensive coverage of the generateField function's behavior. The test cases effectively validate various scenarios including scalar types, non-null constraints, object references, and error cases.
enrich/gpx/src/enrich.gpx.plugin.ts (4)
1-2: LGTM: Import statements follow TypeScript best practices.
The updated import statements properly separate type and value imports, improving type safety and bundle size optimization.
183-183: Skip: Existing comment addresses the unsafe isNaN usage.
🧰 Tools
🪛 Biome
[error] 183-183: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.
(lint/suspicious/noGlobalIsNan)
[error] 183-183: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.
(lint/suspicious/noGlobalIsNan)
202-205: LGTM: Improved function signature formatting.
The multi-line parameter formatting enhances readability while maintaining consistent indentation.
Also applies to: 212-212
229-231: Skip: Existing comment addresses the date validation concerns.
| describe.skip('externalConstraint()', () => { | ||
| const listener = new TestListener() | ||
| const driver = setupDriver() |
There was a problem hiding this comment.
Remove .skip as Vitest migration is complete
The test suite was temporarily skipped during the Jest to Vitest migration. Since the migration is now complete and the new test infrastructure is in place, we should remove the .skip to re-enable these tests.
Apply this diff:
-describe.skip('externalConstraint()', () => {
+describe('externalConstraint()', () => {📝 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.
| describe.skip('externalConstraint()', () => { | |
| const listener = new TestListener() | |
| const driver = setupDriver() | |
| describe('externalConstraint()', () => { | |
| const listener = new TestListener() | |
| const driver = setupDriver() |
| description: 'An unsupported field', | ||
| } | ||
| const consoleSpy = jest.spyOn(console, 'log') | ||
| const consoleSpy = vi.spyOn(console, 'log') |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider centralizing console spy setup
The console spy setup is duplicated across test cases. Consider using beforeEach/afterEach hooks to centralize the spy setup and cleanup.
describe('generateField', () => {
+ let consoleSpy;
+
+ beforeEach(() => {
+ consoleSpy = vi.spyOn(console, 'log');
+ });
+
+ afterEach(() => {
+ consoleSpy.mockRestore();
+ });
// ... existing test cases ...
it('should handle unsupported field types by returning null', () => {
const field = {
name: 'unknown',
type: { kind: 'UNSUPPORTED' },
description: 'An unsupported field',
}
- const consoleSpy = vi.spyOn(console, 'log')
expect(generateField(field, 'TestSheet')).toBeNull()
// ... rest of the test
})
// ... other test cases ...
it('should return null and log for unrecognized field types', () => {
// ... test setup ...
- const consoleSpy = vi.spyOn(console, 'log')
expect(generateField(field, 'WeirdSheet')).toBeNull()
// ... rest of the test
})Also applies to: 151-151
carlbrugger
force-pushed
the
feat/vitest
branch
from
e165e69 to
ba61c40
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (5)
plugins/zip-extractor/src/zip.extractor.e2e.spec.ts (2)
Line range hint
31-61: Consider adding error handling in lifecycle hooksWhile the lifecycle hooks are well-structured, consider adding try-catch blocks for robust error handling, especially in the cleanup hooks.
Here's a suggested improvement:
afterAll(async () => { + try { await deleteSpace(spaceId) driver.shutdown() + } catch (error) { + console.error('Cleanup failed:', error) + throw error + } }) beforeEach(() => { + try { listener.resetCount() + } catch (error) { + console.error('Listener reset failed:', error) + throw error + } }) afterEach(() => { + try { listener.reset() + } catch (error) { + console.error('Listener cleanup failed:', error) + throw error + } })
Line range hint
64-70: Consider enhancing test assertionsWhile the test verifies the number of files, consider adding assertions for:
- File names/types
- File contents or metadata
- Error cases (e.g., corrupted zip)
Would you like me to help generate additional test cases to improve coverage?
plugins/webhook-event-forwarder/src/forward-webhook.e2e.spec.ts (1)
68-77: Add type safety to mock responses.While the fetch mocking implementation is correct, consider adding type safety to prevent potential runtime issues:
interface MockResponse { hello: string; } const mockResponse: MockResponse = { hello: 'Flatfile', };This will help catch type-related issues during development and make the test more maintainable.
Also applies to: 94-99
plugins/automap/src/automap.plugin.e2e.spec.ts (1)
Line range hint
82-83: Consider reducing the test timeoutThe 90-second timeout seems excessive for these operations. Consider:
- Reducing the timeout to a more reasonable value (e.g., 30 seconds)
- Adding a comment explaining why such a long timeout is necessary if it can't be reduced
Also applies to: 127-128
utils/testing/src/test.helpers.ts (1)
39-46: Enhance error message clarity.While the validation is good, consider making the error message more descriptive to aid in troubleshooting.
- throw new Error('environmentId is not defined') + throw new Error('FLATFILE_ENVIRONMENT_ID environment variable is not configured')
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (61)
.github/workflows/lint.ymlis excluded by!**/*.ymlconfig/vitest/package.jsonis excluded by!**/*.jsonconvert/currency/package.jsonis excluded by!**/*.jsonconvert/translate/package.jsonis excluded by!**/*.jsonconvert/what3words/package.jsonis excluded by!**/*.jsonenrich/geocode/package.jsonis excluded by!**/*.jsonenrich/gpx/package.jsonis excluded by!**/*.jsonenrich/sentiment/package.jsonis excluded by!**/*.jsonenrich/summarize/package.jsonis excluded by!**/*.jsonexport/delimited-zip/package.jsonis excluded by!**/*.jsonexport/pivot-table/package.jsonis excluded by!**/*.jsonextract/html-table/package.jsonis excluded by!**/*.jsonextract/markdown/package.jsonis excluded by!**/*.jsonimport/faker/package.jsonis excluded by!**/*.jsonimport/llm-records/package.jsonis excluded by!**/*.jsonimport/rss/package.jsonis excluded by!**/*.jsonpackage-lock.jsonis excluded by!**/package-lock.json,!**/*.jsonpackage.jsonis excluded by!**/*.jsonplugins/autocast/package.jsonis excluded by!**/*.jsonplugins/automap/package.jsonis excluded by!**/*.jsonplugins/constraints/package.jsonis excluded by!**/*.jsonplugins/dedupe/package.jsonis excluded by!**/*.jsonplugins/delimiter-extractor/package.jsonis excluded by!**/*.jsonplugins/dxp-configure/package.jsonis excluded by!**/*.jsonplugins/export-workbook/package.jsonis excluded by!**/*.jsonplugins/foreign-db-extractor/package.jsonis excluded by!**/*.jsonplugins/graphql-schema/package.jsonis excluded by!**/*.jsonplugins/job-handler/package.jsonis excluded by!**/*.jsonplugins/json-extractor/package.jsonis excluded by!**/*.jsonplugins/json-schema/package.jsonis excluded by!**/*.jsonplugins/merge-connection/package.jsonis excluded by!**/*.jsonplugins/openapi-schema/package.jsonis excluded by!**/*.jsonplugins/pdf-extractor/package.jsonis excluded by!**/*.jsonplugins/psv-extractor/package.jsonis excluded by!**/*.jsonplugins/record-hook/package.jsonis excluded by!**/*.jsonplugins/rollout/package.jsonis excluded by!**/*.jsonplugins/space-configure/package.jsonis excluded by!**/*.jsonplugins/sql-ddl-converter/package.jsonis excluded by!**/*.jsonplugins/tsv-extractor/package.jsonis excluded by!**/*.jsonplugins/view-mapped/package.jsonis excluded by!**/*.jsonplugins/webhook-egress/package.jsonis excluded by!**/*.jsonplugins/webhook-event-forwarder/package.jsonis excluded by!**/*.jsonplugins/xlsx-extractor/package.jsonis excluded by!**/*.jsonplugins/xml-extractor/package.jsonis excluded by!**/*.jsonplugins/yaml-schema/package.jsonis excluded by!**/*.jsonplugins/zip-extractor/package.jsonis excluded by!**/*.jsonsupport/common-utils/package.jsonis excluded by!**/*.jsonturbo.jsonis excluded by!**/*.jsonutils/common/package.jsonis excluded by!**/*.jsonutils/extractor/package.jsonis excluded by!**/*.jsonutils/fetch-schema/package.jsonis excluded by!**/*.jsonutils/file-buffer/package.jsonis excluded by!**/*.jsonutils/response-rejection/package.jsonis excluded by!**/*.jsonutils/testing/package.jsonis excluded by!**/*.jsonvalidate/boolean/package.jsonis excluded by!**/*.jsonvalidate/date/package.jsonis excluded by!**/*.jsonvalidate/email/package.jsonis excluded by!**/*.jsonvalidate/isbn/package.jsonis excluded by!**/*.jsonvalidate/number/package.jsonis excluded by!**/*.jsonvalidate/phone/package.jsonis excluded by!**/*.jsonvalidate/string/package.jsonis excluded by!**/*.json
📒 Files selected for processing (157)
.changeset/five-hounds-wash.md(1 hunks)config/vitest/src/index.js(1 hunks)config/vitest/src/utils.js(1 hunks)convert/currency/src/convert.currency.plugin.spec.ts(1 hunks)convert/currency/src/convert.currency.plugin.ts(7 hunks)convert/currency/vitest.config.ts(1 hunks)convert/translate/jest.config.cjs(0 hunks)convert/translate/src/convert.translate.plugin.spec.ts(2 hunks)convert/translate/vitest.config.ts(1 hunks)convert/what3words/jest.config.cjs(0 hunks)convert/what3words/vitest.config.ts(1 hunks)enrich/geocode/jest.config.cjs(0 hunks)enrich/geocode/src/enrich.geocode.plugin.spec.ts(6 hunks)enrich/geocode/src/index.ts(1 hunks)enrich/geocode/vitest.config.ts(1 hunks)enrich/gpx/jest.config.cjs(0 hunks)enrich/gpx/src/enrich.gpx.plugin.spec.ts(3 hunks)enrich/gpx/src/enrich.gpx.plugin.ts(4 hunks)enrich/gpx/vitest.config.ts(1 hunks)enrich/sentiment/jest.config.cjs(0 hunks)enrich/sentiment/src/enrich.sentiment.plugin.spec.ts(4 hunks)enrich/sentiment/src/enrich.sentiment.plugin.ts(2 hunks)enrich/sentiment/vitest.config.ts(1 hunks)enrich/summarize/jest.config.cjs(0 hunks)enrich/summarize/src/summarize.plugin.spec.ts(1 hunks)enrich/summarize/src/summarize.plugin.ts(2 hunks)enrich/summarize/vitest.config.ts(1 hunks)export/delimited-zip/vitest.config.ts(1 hunks)export/pivot-table/jest.config.cjs(0 hunks)export/pivot-table/vitest.config.ts(1 hunks)extract/html-table/jest.config.cjs(0 hunks)extract/html-table/vitest.config.ts(1 hunks)extract/markdown/jest.config.cjs(0 hunks)extract/markdown/src/parser.spec.ts(1 hunks)extract/markdown/vitest.config.ts(1 hunks)import/faker/jest.config.cjs(0 hunks)import/faker/src/faker.plugin.spec.ts(4 hunks)import/faker/src/faker.utils.ts(2 hunks)import/faker/vitest.config.ts(1 hunks)import/llm-records/jest.config.cjs(0 hunks)import/llm-records/vitest.config.ts(1 hunks)import/rss/jest.config.cjs(0 hunks)import/rss/vitest.config.ts(1 hunks)jest.config.js(0 hunks)jest.setup.js(0 hunks)plugins/autocast/jest.config.cjs(0 hunks)plugins/autocast/src/autocast.plugin.spec.ts(2 hunks)plugins/autocast/vitest.config.ts(1 hunks)plugins/automap/jest.config.cjs(0 hunks)plugins/automap/src/automap.plugin.e2e.spec.ts(2 hunks)plugins/automap/vitest.config.ts(1 hunks)plugins/constraints/jest.config.cjs(0 hunks)plugins/constraints/src/external.constraint.e2e.spec.ts(3 hunks)plugins/constraints/src/external.sheet.constraint.e2e.spec.ts(3 hunks)plugins/constraints/vitest.config.ts(1 hunks)plugins/dedupe/jest.config.cjs(0 hunks)plugins/dedupe/src/keep.first.logic.spec.ts(1 hunks)plugins/dedupe/src/keep.last.logic.spec.ts(1 hunks)plugins/dedupe/vitest.config.ts(1 hunks)plugins/delimiter-extractor/jest.config.cjs(0 hunks)plugins/delimiter-extractor/src/parser.spec.ts(2 hunks)plugins/delimiter-extractor/vitest.config.ts(1 hunks)plugins/dxp-configure/jest.config.cjs(0 hunks)plugins/dxp-configure/src/shim.target.spec.ts(1 hunks)plugins/dxp-configure/vitest.config.ts(1 hunks)plugins/export-workbook/jest.config.cjs(0 hunks)plugins/export-workbook/vitest.config.ts(1 hunks)plugins/foreign-db-extractor/jest.config.cjs(0 hunks)plugins/foreign-db-extractor/vitest.config.ts(1 hunks)plugins/graphql-schema/jest.config.cjs(0 hunks)plugins/graphql-schema/src/generate.field.spec.ts(3 hunks)plugins/graphql-schema/vitest.config.ts(1 hunks)plugins/job-handler/jest.config.cjs(0 hunks)plugins/job-handler/src/job.handler.e2e.spec.ts(4 hunks)plugins/job-handler/vitest.config.ts(1 hunks)plugins/json-extractor/jest.config.cjs(0 hunks)plugins/json-extractor/src/parser.spec.ts(1 hunks)plugins/json-extractor/vitest.config.ts(1 hunks)plugins/json-schema/jest.config.cjs(0 hunks)plugins/json-schema/src/setup.factory.spec.ts(1 hunks)plugins/json-schema/vitest.config.ts(1 hunks)plugins/merge-connection/jest.config.cjs(0 hunks)plugins/merge-connection/vitest.config.ts(1 hunks)plugins/openapi-schema/jest.config.cjs(0 hunks)plugins/openapi-schema/src/setup.factory.openapi.spec.ts(1 hunks)plugins/openapi-schema/vitest.config.ts(1 hunks)plugins/pdf-extractor/jest.config.cjs(0 hunks)plugins/pdf-extractor/vitest.config.ts(1 hunks)plugins/psv-extractor/jest.config.cjs(0 hunks)plugins/psv-extractor/vitest.config.ts(1 hunks)plugins/record-hook/jest.config.cjs(0 hunks)plugins/record-hook/src/RecordHook.e2e.spec.ts(4 hunks)plugins/record-hook/vitest.config.ts(1 hunks)plugins/rollout/jest.config.cjs(0 hunks)plugins/rollout/vitest.config.ts(1 hunks)plugins/space-configure/jest.config.cjs(0 hunks)plugins/space-configure/src/space.configure.e2e.spec.ts(3 hunks)plugins/space-configure/vitest.config.ts(1 hunks)plugins/sql-ddl-converter/jest.config.cjs(0 hunks)plugins/sql-ddl-converter/vitest.config.ts(1 hunks)plugins/tsv-extractor/jest.config.cjs(0 hunks)plugins/tsv-extractor/vitest.config.ts(1 hunks)plugins/view-mapped/jest.config.cjs(0 hunks)plugins/view-mapped/vitest.config.ts(1 hunks)plugins/webhook-egress/jest.config.cjs(0 hunks)plugins/webhook-egress/src/webhook.egress.e2e.spec.ts(6 hunks)plugins/webhook-egress/vitest.config.ts(1 hunks)plugins/webhook-event-forwarder/jest.config.cjs(0 hunks)plugins/webhook-event-forwarder/src/forward-webhook.e2e.spec.ts(3 hunks)plugins/webhook-event-forwarder/vitest.config.ts(1 hunks)plugins/xlsx-extractor/jest.config.cjs(0 hunks)plugins/xlsx-extractor/src/parser.spec.ts(1 hunks)plugins/xlsx-extractor/src/utils.spec.ts(1 hunks)plugins/xlsx-extractor/vitest.config.ts(1 hunks)plugins/xml-extractor/jest.config.cjs(0 hunks)plugins/xml-extractor/src/parser.spec.ts(1 hunks)plugins/xml-extractor/vitest.config.ts(1 hunks)plugins/yaml-schema/jest.config.cjs(0 hunks)plugins/yaml-schema/src/setup.factory.spec.ts(2 hunks)plugins/yaml-schema/vitest.config.ts(1 hunks)plugins/zip-extractor/jest.config.cjs(0 hunks)plugins/zip-extractor/src/zip.extractor.e2e.spec.ts(2 hunks)plugins/zip-extractor/vitest.config.ts(1 hunks)support/common-utils/src/async.map.spec.ts(2 hunks)support/common-utils/vitest.config.ts(1 hunks)test/betterConsoleLog.js(0 hunks)test/dotenv-config.js(0 hunks)test/setup-global.js(0 hunks)test/toBePendingMatcher.ts(0 hunks)test/unit.cleanup.js(0 hunks)utils/common/jest.config.cjs(0 hunks)utils/common/src/all.records.e2e.spec.ts(7 hunks)utils/common/src/async.batch.spec.ts(6 hunks)utils/common/src/logging.helper.spec.ts(4 hunks)utils/common/src/slugify.spec.ts(1 hunks)utils/common/vitest.config.ts(1 hunks)utils/extractor/jest.config.cjs(0 hunks)utils/extractor/vitest.config.ts(1 hunks)utils/fetch-schema/jest.config.cjs(0 hunks)utils/fetch-schema/vitest.config.ts(1 hunks)utils/file-buffer/jest.config.cjs(0 hunks)utils/file-buffer/vitest.config.ts(1 hunks)utils/response-rejection/jest.config.cjs(0 hunks)utils/response-rejection/vitest.config.ts(1 hunks)utils/testing/jest.config.cjs(0 hunks)utils/testing/src/test.helpers.ts(3 hunks)utils/testing/src/test.listener.spec.ts(2 hunks)utils/testing/src/toBePendingMatcher.ts(1 hunks)utils/testing/vitest.config.ts(1 hunks)validate/boolean/jest.config.cjs(0 hunks)validate/boolean/src/validate.boolean.plugin.spec.ts(6 hunks)validate/boolean/vitest.config.ts(1 hunks)validate/date/jest.config.cjs(0 hunks)validate/date/src/validate.date.plugin.e2e.spec.ts(2 hunks)validate/date/vitest.config.ts(1 hunks)validate/email/jest.config.cjs(0 hunks)validate/email/src/validate.email.plugin.ts(1 hunks)
⛔ Files not processed due to max files limit (15)
- validate/email/src/validate.email.utils.spec.ts
- validate/email/src/validate.email.utils.ts
- validate/email/vitest.config.ts
- validate/isbn/jest.config.cjs
- validate/isbn/src/validate.isbn.plugin.spec.ts
- validate/isbn/vitest.config.ts
- validate/number/jest.config.cjs
- validate/number/src/validate.number.plugin.spec.ts
- validate/number/vitest.config.ts
- validate/phone/jest.config.cjs
- validate/phone/src/validate.phone.utils.spec.ts
- validate/phone/vitest.config.ts
- validate/string/jest.config.cjs
- validate/string/src/validate.string.plugin.spec.ts
- validate/string/vitest.config.ts
💤 Files with no reviewable changes (56)
- convert/translate/jest.config.cjs
- convert/what3words/jest.config.cjs
- enrich/geocode/jest.config.cjs
- enrich/gpx/jest.config.cjs
- enrich/sentiment/jest.config.cjs
- enrich/summarize/jest.config.cjs
- export/pivot-table/jest.config.cjs
- extract/html-table/jest.config.cjs
- extract/markdown/jest.config.cjs
- import/faker/jest.config.cjs
- import/llm-records/jest.config.cjs
- import/rss/jest.config.cjs
- jest.config.js
- jest.setup.js
- plugins/autocast/jest.config.cjs
- plugins/automap/jest.config.cjs
- plugins/constraints/jest.config.cjs
- plugins/dedupe/jest.config.cjs
- plugins/delimiter-extractor/jest.config.cjs
- plugins/dxp-configure/jest.config.cjs
- plugins/export-workbook/jest.config.cjs
- plugins/foreign-db-extractor/jest.config.cjs
- plugins/graphql-schema/jest.config.cjs
- plugins/job-handler/jest.config.cjs
- plugins/json-extractor/jest.config.cjs
- plugins/json-schema/jest.config.cjs
- plugins/merge-connection/jest.config.cjs
- plugins/openapi-schema/jest.config.cjs
- plugins/pdf-extractor/jest.config.cjs
- plugins/psv-extractor/jest.config.cjs
- plugins/record-hook/jest.config.cjs
- plugins/rollout/jest.config.cjs
- plugins/space-configure/jest.config.cjs
- plugins/sql-ddl-converter/jest.config.cjs
- plugins/tsv-extractor/jest.config.cjs
- plugins/view-mapped/jest.config.cjs
- plugins/webhook-egress/jest.config.cjs
- plugins/webhook-event-forwarder/jest.config.cjs
- plugins/xlsx-extractor/jest.config.cjs
- plugins/xml-extractor/jest.config.cjs
- plugins/yaml-schema/jest.config.cjs
- plugins/zip-extractor/jest.config.cjs
- test/betterConsoleLog.js
- test/dotenv-config.js
- test/setup-global.js
- test/toBePendingMatcher.ts
- test/unit.cleanup.js
- utils/common/jest.config.cjs
- utils/extractor/jest.config.cjs
- utils/fetch-schema/jest.config.cjs
- utils/file-buffer/jest.config.cjs
- utils/response-rejection/jest.config.cjs
- utils/testing/jest.config.cjs
- validate/boolean/jest.config.cjs
- validate/date/jest.config.cjs
- validate/email/jest.config.cjs
✅ Files skipped from review due to trivial changes (1)
- export/delimited-zip/vitest.config.ts
🚧 Files skipped from review as they are similar to previous changes (93)
- .changeset/five-hounds-wash.md
- config/vitest/src/index.js
- config/vitest/src/utils.js
- convert/currency/src/convert.currency.plugin.spec.ts
- convert/currency/src/convert.currency.plugin.ts
- convert/currency/vitest.config.ts
- convert/translate/src/convert.translate.plugin.spec.ts
- convert/translate/vitest.config.ts
- convert/what3words/vitest.config.ts
- enrich/geocode/src/enrich.geocode.plugin.spec.ts
- enrich/geocode/src/index.ts
- enrich/geocode/vitest.config.ts
- enrich/gpx/src/enrich.gpx.plugin.spec.ts
- enrich/gpx/vitest.config.ts
- enrich/sentiment/src/enrich.sentiment.plugin.spec.ts
- enrich/sentiment/src/enrich.sentiment.plugin.ts
- enrich/sentiment/vitest.config.ts
- enrich/summarize/src/summarize.plugin.spec.ts
- enrich/summarize/src/summarize.plugin.ts
- enrich/summarize/vitest.config.ts
- export/pivot-table/vitest.config.ts
- extract/html-table/vitest.config.ts
- extract/markdown/src/parser.spec.ts
- extract/markdown/vitest.config.ts
- import/faker/src/faker.plugin.spec.ts
- import/faker/src/faker.utils.ts
- import/faker/vitest.config.ts
- import/llm-records/vitest.config.ts
- import/rss/vitest.config.ts
- plugins/autocast/src/autocast.plugin.spec.ts
- plugins/autocast/vitest.config.ts
- plugins/automap/vitest.config.ts
- plugins/constraints/src/external.sheet.constraint.e2e.spec.ts
- plugins/constraints/vitest.config.ts
- plugins/dedupe/src/keep.first.logic.spec.ts
- plugins/dedupe/src/keep.last.logic.spec.ts
- plugins/dedupe/vitest.config.ts
- plugins/delimiter-extractor/src/parser.spec.ts
- plugins/delimiter-extractor/vitest.config.ts
- plugins/dxp-configure/src/shim.target.spec.ts
- plugins/dxp-configure/vitest.config.ts
- plugins/export-workbook/vitest.config.ts
- plugins/foreign-db-extractor/vitest.config.ts
- plugins/graphql-schema/src/generate.field.spec.ts
- plugins/graphql-schema/vitest.config.ts
- plugins/job-handler/src/job.handler.e2e.spec.ts
- plugins/job-handler/vitest.config.ts
- plugins/json-extractor/src/parser.spec.ts
- plugins/json-extractor/vitest.config.ts
- plugins/json-schema/src/setup.factory.spec.ts
- plugins/json-schema/vitest.config.ts
- plugins/merge-connection/vitest.config.ts
- plugins/openapi-schema/src/setup.factory.openapi.spec.ts
- plugins/openapi-schema/vitest.config.ts
- plugins/pdf-extractor/vitest.config.ts
- plugins/psv-extractor/vitest.config.ts
- plugins/record-hook/src/RecordHook.e2e.spec.ts
- plugins/record-hook/vitest.config.ts
- plugins/rollout/vitest.config.ts
- plugins/space-configure/vitest.config.ts
- plugins/sql-ddl-converter/vitest.config.ts
- plugins/tsv-extractor/vitest.config.ts
- plugins/view-mapped/vitest.config.ts
- plugins/webhook-egress/src/webhook.egress.e2e.spec.ts
- plugins/webhook-egress/vitest.config.ts
- plugins/webhook-event-forwarder/vitest.config.ts
- plugins/xlsx-extractor/src/parser.spec.ts
- plugins/xlsx-extractor/src/utils.spec.ts
- plugins/xlsx-extractor/vitest.config.ts
- plugins/xml-extractor/src/parser.spec.ts
- plugins/xml-extractor/vitest.config.ts
- plugins/yaml-schema/src/setup.factory.spec.ts
- plugins/yaml-schema/vitest.config.ts
- plugins/zip-extractor/vitest.config.ts
- support/common-utils/src/async.map.spec.ts
- support/common-utils/vitest.config.ts
- utils/common/src/all.records.e2e.spec.ts
- utils/common/src/async.batch.spec.ts
- utils/common/src/logging.helper.spec.ts
- utils/common/src/slugify.spec.ts
- utils/common/vitest.config.ts
- utils/extractor/vitest.config.ts
- utils/fetch-schema/vitest.config.ts
- utils/file-buffer/vitest.config.ts
- utils/response-rejection/vitest.config.ts
- utils/testing/src/test.listener.spec.ts
- utils/testing/src/toBePendingMatcher.ts
- utils/testing/vitest.config.ts
- validate/boolean/src/validate.boolean.plugin.spec.ts
- validate/boolean/vitest.config.ts
- validate/date/src/validate.date.plugin.e2e.spec.ts
- validate/date/vitest.config.ts
- validate/email/src/validate.email.plugin.ts
🧰 Additional context used
🪛 Biome
enrich/gpx/src/enrich.gpx.plugin.ts
[error] 183-183: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.
(lint/suspicious/noGlobalIsNan)
[error] 183-183: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.
(lint/suspicious/noGlobalIsNan)
plugins/constraints/src/external.constraint.e2e.spec.ts
[error] 59-61: Disallow duplicate setup and teardown hooks.
Disallow beforeEach duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
🔇 Additional comments (24)
plugins/zip-extractor/src/zip.extractor.e2e.spec.ts (2)
6-20: LGTM: Clean migration of test imports
The imports have been properly updated to use Vitest test functions, and the new testing utilities are correctly imported.
26-27: LGTM: Improved test setup structure
The new approach using TestListener and setupDriver improves test maintainability and follows better OOP practices.
Let's verify the consistency of this pattern across other test files:
✅ Verification successful
TestListener and setupDriver pattern consistently used across test files
The verification shows that both TestListener and setupDriver are consistently used across all e2e test files in the codebase, including:
- All plugin test files (zip-extractor, webhook-egress, space-configure, etc.)
- Validation test files
- Core testing utilities
This confirms that the test setup changes follow the established testing patterns in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify consistent usage of TestListener and setupDriver
# across test files
# Check for consistent TestListener usage
rg -l "new TestListener\(\)" --type ts
# Check for consistent setupDriver usage
rg -l "setupDriver\(\)" --type ts
Length of output: 1258
plugins/space-configure/src/space.configure.e2e.spec.ts (3)
2-17: LGTM: Clean transition to Vitest imports
The import statements are well-organized and correctly import all necessary Vitest testing functions and new testing utilities.
51-53: LGTM: Well-structured test setup
The test setup properly utilizes Vitest's mocking functionality and the new testing utilities.
Line range hint 57-77: Previous review comment about consolidating hooks is still applicable
plugins/constraints/src/external.constraint.e2e.spec.ts (3)
6-19: LGTM: Test framework migration completed successfully
The imports have been properly updated to use Vitest and the new testing infrastructure.
Line range hint 66-95: LGTM: Test cases are well-structured
The test cases provide good coverage of the main functionality:
- Error assignment
- Error handling
- Value setting
🧰 Tools
🪛 Biome
[error] 59-61: Disallow duplicate setup and teardown hooks.
Disallow beforeEach duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
Line range hint 97-127: LGTM: Schema and test data are well-defined
The schema properly configures the external constraint, and the test data provides good test cases for validation.
plugins/webhook-event-forwarder/src/forward-webhook.e2e.spec.ts (3)
5-19: LGTM: Clean migration to Vitest imports.
The transition from Jest to Vitest imports is well-structured and includes all necessary test functions.
Line range hint 23-48: LGTM: Well-structured test setup with proper lifecycle management.
The test setup properly initializes and manages the TestListener and driver lifecycle, following e2e testing best practices.
Line range hint 1-116: Verify test coverage after Vitest migration.
The migration from Jest to Vitest has been implemented well. To ensure the migration hasn't impacted test coverage:
✅ Verification successful
Let me gather more information about the test coverage by checking the implementation file and any potential test files I might have missed.
Let me correct the file path and check the implementation file content.
Test coverage is complete and properly migrated to Vitest
The test file adequately covers all the main functionality of the webhook forwarder:
- Success case with JSON response
- Error case with non-200 status
- Callback handling in both scenarios
- Event topic verification
The migration from Jest to Vitest maintains the same level of test coverage, with no functionality gaps compared to the implementation file.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if all test cases from the Jest implementation are covered in Vitest
# Search for any remaining Jest-related code or comments that might indicate missing test cases
rg -i "jest\.|test\(|describe\(|it\(" --type ts plugins/webhook-event-forwarder/
Length of output: 391
Script:
#!/bin/bash
# Check for all test files and implementation files
fd -e ts -e js . plugins/webhook-event-forwarder/
# Get the content of the implementation file to understand what should be tested
cat plugins/webhook-event-forwarder/src/forward-webhook.ts
Length of output: 412
Script:
#!/bin/bash
# Get the content of the implementation file to understand what should be tested
cat plugins/webhook-event-forwarder/src/forward.webhook.ts
# Also check the index file to see if there are any additional exports that might need testing
cat plugins/webhook-event-forwarder/src/index.ts
Length of output: 1599
plugins/automap/src/automap.plugin.e2e.spec.ts (5)
4-7: LGTM: Clean transition to Vitest imports
The import changes correctly implement the transition from Jest to Vitest while maintaining the necessary testing utilities.
Also applies to: 11-20
26-34: LGTM: Well-structured test setup
The implementation of TestListener and driver setup follows best practices, with proper initialization in the beforeAll hook.
41-44: Race condition in teardown sequence
This is a duplicate of a previously identified issue. The driver shutdown should be awaited before deleting the space.
46-52: LGTM: Good test isolation
The beforeEach/afterEach hooks properly reset the listener state, ensuring clean test isolation.
55-55: LGTM: Clean migration to Vitest mocking
The transition from jest.fn() to vi.fn() is implemented correctly in both test suites.
Also applies to: 94-94
utils/testing/src/test.helpers.ts (3)
4-4: LGTM: Appropriate test lifecycle hooks imported from Vitest.
The change correctly imports the necessary lifecycle hooks as part of the Jest to Vitest migration.
15-15: Previous comment about environment ID validation remains applicable.
The concern about potential masking of configuration issues by converting undefined to "undefined" is still valid.
54-54: LGTM: Good refactoring of driver setup.
The change improves code organization by centralizing the driver setup logic.
enrich/gpx/src/enrich.gpx.plugin.ts (5)
1-2: LGTM! Import statements follow TypeScript best practices.
The changes properly separate type and value imports, which improves tree-shaking capabilities and type safety.
202-205: LGTM! Improved function signature formatting.
The multi-line parameter formatting enhances readability while maintaining functionality.
Also applies to: 212-212
229-231: Refer to the previous review comment about date string validation.
183-183: Refer to the previous review comment about unsafe isNaN usage.
🧰 Tools
🪛 Biome
[error] 183-183: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.
(lint/suspicious/noGlobalIsNan)
[error] 183-183: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.
(lint/suspicious/noGlobalIsNan)
Line range hint 1-262: Verify test file updates for Vitest compatibility.
Since this PR transitions from Jest to Vitest, ensure that the corresponding test files for this module have been updated to use Vitest syntax and configurations.
✅ Verification successful
Let me check the test file content to verify its Vitest compatibility.
Test files are properly updated for Vitest compatibility
The test file enrich.gpx.plugin.spec.ts has been correctly updated to use Vitest:
- Uses Vitest's
describe,expect, anditimports - Contains proper Vitest test syntax and assertions
- Has a valid Vitest configuration file that extends the shared config
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for test file updates and Vitest configuration
# Look for test files
echo "Looking for related test files..."
fd -e test.ts -e spec.ts . enrich/gpx/src
# Check for Vitest config
echo "Checking for Vitest configuration..."
fd vitest.config.ts . enrich/gpx
# Check for removed Jest config
echo "Verifying Jest config removal..."
fd jest.config.js jest.config.cjs . enrich/gpx
Length of output: 2678
Script:
#!/bin/bash
# Check content of the test file
cat enrich/gpx/src/enrich.gpx.plugin.spec.ts
# Also check vitest config
cat enrich/gpx/vitest.config.ts
Length of output: 3354
| const workspace = await api.workbooks.get(space.data.primaryWorkbookId!) | ||
|
|
||
| expect(workspace.data.name).toBe(setup.workbooks[0].name) | ||
| expect(workspace.data.labels).toMatchObject(setup.workbooks[0].labels) | ||
| expect(workspace.data.sheets[0].name).toBe( | ||
| setup.workbooks[0].sheets[0].name | ||
| expect(workspace.data.labels).toMatchObject(setup.workbooks[0].labels!) | ||
| expect(workspace.data.sheets![0].name).toBe( | ||
| setup.workbooks[0].sheets![0].name | ||
| ) | ||
| expect(workspace.data.sheets[0].config).toMatchObject( | ||
| setup.workbooks[0].sheets[0] | ||
| expect(workspace.data.sheets![0].config).toMatchObject( | ||
| setup.workbooks[0].sheets![0] | ||
| ) | ||
| expect(workspace.data.actions).toMatchObject(setup.workbooks[0].actions) | ||
| expect(workspace.data.actions).toMatchObject(setup.workbooks[0].actions!) |
There was a problem hiding this comment.
💡 Codebase verification
🛠️ Refactor suggestion
Based on the evidence gathered, I can now provide a final response:
Non-null assertions are potentially unsafe in this test
The codebase shows that:
primaryWorkbookIdis assigned an empty string as fallback in some casesworkbook.sheetscan be undefined (see automap plugin's null coalescing)- Other plugins implement explicit null checks for these properties
Recommendations:
- Replace non-null assertions with explicit null checks
- Add meaningful error messages for better debugging
- Use optional chaining and null coalescing for safer property access
- Consider adding type guards or assertions before the test expectations
Example:
const primaryWorkbookId = space.data.primaryWorkbookId
expect(primaryWorkbookId).toBeDefined()
const workspace = await api.workbooks.get(primaryWorkbookId)
const { sheets, labels, actions } = workspace.data
expect(sheets?.[0]).toBeDefined()
expect(labels).toBeDefined()
expect(actions).toBeDefined()
// Then proceed with specific assertions🔗 Analysis chain
Verify the safety of non-null assertions
The test uses multiple non-null assertions (!) which could mask potential runtime issues. Consider:
- Adding explicit null checks with meaningful error messages
- Breaking down complex object comparisons into more specific assertions
Let's verify if these properties can actually be null in other parts of the codebase:
Consider this safer approach:
- const workspace = await api.workbooks.get(space.data.primaryWorkbookId!)
+ const primaryWorkbookId = space.data.primaryWorkbookId
+ if (!primaryWorkbookId) {
+ throw new Error('Primary workbook ID is required but was not found')
+ }
+ const workspace = await api.workbooks.get(primaryWorkbookId)
+
+ const { name, labels, sheets, actions } = workspace.data
+ const expectedSheet = setup.workbooks[0].sheets?.[0]
+
+ expect(name).toBe(setup.workbooks[0].name)
+ expect(labels).toBeDefined()
+ expect(labels).toMatchObject(setup.workbooks[0].labels ?? [])
+
+ expect(sheets?.[0]).toBeDefined()
+ expect(sheets?.[0].name).toBe(expectedSheet?.name)
+ expect(sheets?.[0].config).toMatchObject(expectedSheet ?? {})
+
+ expect(actions).toBeDefined()
+ expect(actions).toMatchObject(setup.workbooks[0].actions ?? [])🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for cases where these properties might be undefined or null
rg -A 2 "primaryWorkbookId\s*[=:]\s*(null|undefined)"
rg -A 2 "workbooks\[\d+\]\.labels\s*[=:]\s*(null|undefined)"
rg -A 2 "workbooks\[\d+\]\.sheets\s*[=:]\s*(null|undefined)"
rg -A 2 "workbooks\[\d+\]\.actions\s*[=:]\s*(null|undefined)"
Length of output: 245
Script:
#!/bin/bash
# Let's try a different approach to find the type definitions and usage patterns
# Find type definitions for these properties
ast-grep --pattern 'interface $_ {
$$$
primaryWorkbookId$_
$$$
}'
# Search for actual usage of these properties in tests
rg -A 3 "primaryWorkbookId" --type ts
# Look for workbook related type definitions
ast-grep --pattern 'interface $_ {
$$$
(sheets|labels|actions)$_
$$$
}'
# Check test setup patterns
rg -A 5 "setup\.workbooks\[" --type ts
Length of output: 2342
Script:
#!/bin/bash
# Let's check the setup and test initialization to understand if these properties are guaranteed
# Find the test setup
rg -B 5 -A 10 "describe.*space\.configure" --type ts
# Look for workbook setup/initialization
rg -B 5 "setup\s*=\s*" --type ts
# Check if there are any null checks or default values in the implementation
rg -A 3 "workbooks\.get" --type ts
Length of output: 3359
carlbrugger
force-pushed
the
feat/vitest
branch
from
89e90ac to
282ec14
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
utils/common/src/slugify.spec.ts (1)
Line range hint
4-42: Well-structured test suite with comprehensive coverageThe test suite is well-organized with clear, focused test cases covering basic functionality, edge cases, and special scenarios.
Consider adding these test cases to make the suite even more robust:
it('should handle case sensitivity appropriately', () => { expect(slugify('Hello World')).toBe('hello-world') }) it('should handle unicode characters', () => { expect(slugify('héllo wörld')).toBe('hello-world') }) it('should handle numbers in strings', () => { expect(slugify('hello 123 world')).toBe('hello-123-world') }) it('should handle very long strings', () => { const longString = 'a'.repeat(1000) expect(slugify(longString)).toBe('a'.repeat(1000)) })plugins/automap/src/automap.plugin.e2e.spec.ts (1)
55-55: Consider enhancing test assertions and reducing timeoutWhile the transition to
vi.fn()is correct, consider these improvements:
- The 90-second timeout seems excessive for this operation
- The assertions could be more specific than just checking if the mock was called
Consider this enhancement:
- expect(mockFn).toHaveBeenCalled() + expect(mockFn).toHaveBeenCalledTimes(1) + const jobId = mockFn.mock.calls[0][0] + expect(typeof jobId).toBe('string')Also, consider reducing the timeout:
- }, 90_000) + }, 30_000)Also applies to: 94-94
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (61)
.github/workflows/lint.ymlis excluded by!**/*.ymlconfig/vitest/package.jsonis excluded by!**/*.jsonconvert/currency/package.jsonis excluded by!**/*.jsonconvert/translate/package.jsonis excluded by!**/*.jsonconvert/what3words/package.jsonis excluded by!**/*.jsonenrich/geocode/package.jsonis excluded by!**/*.jsonenrich/gpx/package.jsonis excluded by!**/*.jsonenrich/sentiment/package.jsonis excluded by!**/*.jsonenrich/summarize/package.jsonis excluded by!**/*.jsonexport/delimited-zip/package.jsonis excluded by!**/*.jsonexport/pivot-table/package.jsonis excluded by!**/*.jsonextract/html-table/package.jsonis excluded by!**/*.jsonextract/markdown/package.jsonis excluded by!**/*.jsonimport/faker/package.jsonis excluded by!**/*.jsonimport/llm-records/package.jsonis excluded by!**/*.jsonimport/rss/package.jsonis excluded by!**/*.jsonpackage-lock.jsonis excluded by!**/package-lock.json,!**/*.jsonpackage.jsonis excluded by!**/*.jsonplugins/autocast/package.jsonis excluded by!**/*.jsonplugins/automap/package.jsonis excluded by!**/*.jsonplugins/constraints/package.jsonis excluded by!**/*.jsonplugins/dedupe/package.jsonis excluded by!**/*.jsonplugins/delimiter-extractor/package.jsonis excluded by!**/*.jsonplugins/dxp-configure/package.jsonis excluded by!**/*.jsonplugins/export-workbook/package.jsonis excluded by!**/*.jsonplugins/foreign-db-extractor/package.jsonis excluded by!**/*.jsonplugins/graphql-schema/package.jsonis excluded by!**/*.jsonplugins/job-handler/package.jsonis excluded by!**/*.jsonplugins/json-extractor/package.jsonis excluded by!**/*.jsonplugins/json-schema/package.jsonis excluded by!**/*.jsonplugins/merge-connection/package.jsonis excluded by!**/*.jsonplugins/openapi-schema/package.jsonis excluded by!**/*.jsonplugins/pdf-extractor/package.jsonis excluded by!**/*.jsonplugins/psv-extractor/package.jsonis excluded by!**/*.jsonplugins/record-hook/package.jsonis excluded by!**/*.jsonplugins/rollout/package.jsonis excluded by!**/*.jsonplugins/space-configure/package.jsonis excluded by!**/*.jsonplugins/sql-ddl-converter/package.jsonis excluded by!**/*.jsonplugins/tsv-extractor/package.jsonis excluded by!**/*.jsonplugins/view-mapped/package.jsonis excluded by!**/*.jsonplugins/webhook-egress/package.jsonis excluded by!**/*.jsonplugins/webhook-event-forwarder/package.jsonis excluded by!**/*.jsonplugins/xlsx-extractor/package.jsonis excluded by!**/*.jsonplugins/xml-extractor/package.jsonis excluded by!**/*.jsonplugins/yaml-schema/package.jsonis excluded by!**/*.jsonplugins/zip-extractor/package.jsonis excluded by!**/*.jsonsupport/common-utils/package.jsonis excluded by!**/*.jsonturbo.jsonis excluded by!**/*.jsonutils/common/package.jsonis excluded by!**/*.jsonutils/extractor/package.jsonis excluded by!**/*.jsonutils/fetch-schema/package.jsonis excluded by!**/*.jsonutils/file-buffer/package.jsonis excluded by!**/*.jsonutils/response-rejection/package.jsonis excluded by!**/*.jsonutils/testing/package.jsonis excluded by!**/*.jsonvalidate/boolean/package.jsonis excluded by!**/*.jsonvalidate/date/package.jsonis excluded by!**/*.jsonvalidate/email/package.jsonis excluded by!**/*.jsonvalidate/isbn/package.jsonis excluded by!**/*.jsonvalidate/number/package.jsonis excluded by!**/*.jsonvalidate/phone/package.jsonis excluded by!**/*.jsonvalidate/string/package.jsonis excluded by!**/*.json
📒 Files selected for processing (157)
.changeset/five-hounds-wash.md(1 hunks)config/vitest/src/index.js(1 hunks)config/vitest/src/utils.js(1 hunks)convert/currency/jest.config.cjs(0 hunks)convert/currency/src/convert.currency.plugin.spec.ts(1 hunks)convert/currency/src/convert.currency.plugin.ts(7 hunks)convert/currency/vitest.config.ts(1 hunks)convert/translate/jest.config.cjs(0 hunks)convert/translate/src/convert.translate.plugin.spec.ts(2 hunks)convert/translate/vitest.config.ts(1 hunks)convert/what3words/jest.config.cjs(0 hunks)convert/what3words/vitest.config.ts(1 hunks)enrich/geocode/jest.config.cjs(0 hunks)enrich/geocode/src/enrich.geocode.plugin.spec.ts(6 hunks)enrich/geocode/src/index.ts(1 hunks)enrich/geocode/vitest.config.ts(1 hunks)enrich/gpx/jest.config.cjs(0 hunks)enrich/gpx/src/enrich.gpx.plugin.spec.ts(3 hunks)enrich/gpx/src/enrich.gpx.plugin.ts(4 hunks)enrich/gpx/vitest.config.ts(1 hunks)enrich/sentiment/jest.config.cjs(0 hunks)enrich/sentiment/src/enrich.sentiment.plugin.spec.ts(4 hunks)enrich/sentiment/src/enrich.sentiment.plugin.ts(2 hunks)enrich/sentiment/vitest.config.ts(1 hunks)enrich/summarize/jest.config.cjs(0 hunks)enrich/summarize/src/summarize.plugin.spec.ts(1 hunks)enrich/summarize/src/summarize.plugin.ts(2 hunks)enrich/summarize/vitest.config.ts(1 hunks)export/delimited-zip/jest.config.js(0 hunks)export/delimited-zip/vitest.config.ts(1 hunks)export/pivot-table/jest.config.cjs(0 hunks)export/pivot-table/vitest.config.ts(1 hunks)extract/html-table/jest.config.cjs(0 hunks)extract/html-table/vitest.config.ts(1 hunks)extract/markdown/jest.config.cjs(0 hunks)extract/markdown/src/parser.spec.ts(1 hunks)extract/markdown/vitest.config.ts(1 hunks)import/faker/jest.config.cjs(0 hunks)import/faker/src/faker.plugin.spec.ts(4 hunks)import/faker/src/faker.utils.ts(2 hunks)import/faker/vitest.config.ts(1 hunks)import/llm-records/jest.config.cjs(0 hunks)import/llm-records/vitest.config.ts(1 hunks)import/rss/jest.config.cjs(0 hunks)import/rss/vitest.config.ts(1 hunks)jest.config.js(0 hunks)jest.setup.js(0 hunks)plugins/autocast/jest.config.cjs(0 hunks)plugins/autocast/src/autocast.plugin.spec.ts(2 hunks)plugins/autocast/vitest.config.ts(1 hunks)plugins/automap/jest.config.cjs(0 hunks)plugins/automap/src/automap.plugin.e2e.spec.ts(2 hunks)plugins/automap/vitest.config.ts(1 hunks)plugins/constraints/jest.config.cjs(0 hunks)plugins/constraints/src/external.constraint.e2e.spec.ts(3 hunks)plugins/constraints/src/external.sheet.constraint.e2e.spec.ts(3 hunks)plugins/constraints/vitest.config.ts(1 hunks)plugins/dedupe/jest.config.cjs(0 hunks)plugins/dedupe/src/keep.first.logic.spec.ts(1 hunks)plugins/dedupe/src/keep.last.logic.spec.ts(1 hunks)plugins/dedupe/vitest.config.ts(1 hunks)plugins/delimiter-extractor/jest.config.cjs(0 hunks)plugins/delimiter-extractor/src/parser.spec.ts(2 hunks)plugins/delimiter-extractor/vitest.config.ts(1 hunks)plugins/dxp-configure/jest.config.cjs(0 hunks)plugins/dxp-configure/src/shim.target.spec.ts(1 hunks)plugins/dxp-configure/vitest.config.ts(1 hunks)plugins/export-workbook/jest.config.cjs(0 hunks)plugins/export-workbook/vitest.config.ts(1 hunks)plugins/foreign-db-extractor/jest.config.cjs(0 hunks)plugins/foreign-db-extractor/vitest.config.ts(1 hunks)plugins/graphql-schema/jest.config.cjs(0 hunks)plugins/graphql-schema/src/generate.field.spec.ts(3 hunks)plugins/graphql-schema/vitest.config.ts(1 hunks)plugins/job-handler/jest.config.cjs(0 hunks)plugins/job-handler/src/job.handler.e2e.spec.ts(4 hunks)plugins/job-handler/vitest.config.ts(1 hunks)plugins/json-extractor/jest.config.cjs(0 hunks)plugins/json-extractor/src/parser.spec.ts(1 hunks)plugins/json-extractor/vitest.config.ts(1 hunks)plugins/json-schema/jest.config.cjs(0 hunks)plugins/json-schema/src/setup.factory.spec.ts(1 hunks)plugins/json-schema/vitest.config.ts(1 hunks)plugins/merge-connection/jest.config.cjs(0 hunks)plugins/merge-connection/vitest.config.ts(1 hunks)plugins/openapi-schema/jest.config.cjs(0 hunks)plugins/openapi-schema/src/setup.factory.openapi.spec.ts(1 hunks)plugins/openapi-schema/vitest.config.ts(1 hunks)plugins/pdf-extractor/jest.config.cjs(0 hunks)plugins/pdf-extractor/vitest.config.ts(1 hunks)plugins/psv-extractor/jest.config.cjs(0 hunks)plugins/psv-extractor/vitest.config.ts(1 hunks)plugins/record-hook/jest.config.cjs(0 hunks)plugins/record-hook/rollup.config.mjs(1 hunks)plugins/record-hook/src/RecordHook.e2e.spec.ts(4 hunks)plugins/record-hook/vitest.config.ts(1 hunks)plugins/rollout/jest.config.cjs(0 hunks)plugins/rollout/vitest.config.ts(1 hunks)plugins/space-configure/jest.config.cjs(0 hunks)plugins/space-configure/src/space.configure.e2e.spec.ts(3 hunks)plugins/space-configure/vitest.config.ts(1 hunks)plugins/sql-ddl-converter/jest.config.cjs(0 hunks)plugins/sql-ddl-converter/vitest.config.ts(1 hunks)plugins/tsv-extractor/jest.config.cjs(0 hunks)plugins/tsv-extractor/vitest.config.ts(1 hunks)plugins/view-mapped/jest.config.cjs(0 hunks)plugins/view-mapped/vitest.config.ts(1 hunks)plugins/webhook-egress/jest.config.cjs(0 hunks)plugins/webhook-egress/src/webhook.egress.e2e.spec.ts(6 hunks)plugins/webhook-egress/vitest.config.ts(1 hunks)plugins/webhook-event-forwarder/jest.config.cjs(0 hunks)plugins/webhook-event-forwarder/src/forward-webhook.e2e.spec.ts(3 hunks)plugins/webhook-event-forwarder/vitest.config.ts(1 hunks)plugins/xlsx-extractor/jest.config.cjs(0 hunks)plugins/xlsx-extractor/src/parser.spec.ts(1 hunks)plugins/xlsx-extractor/src/utils.spec.ts(1 hunks)plugins/xlsx-extractor/vitest.config.ts(1 hunks)plugins/xml-extractor/jest.config.cjs(0 hunks)plugins/xml-extractor/src/parser.spec.ts(1 hunks)plugins/xml-extractor/vitest.config.ts(1 hunks)plugins/yaml-schema/jest.config.cjs(0 hunks)plugins/yaml-schema/src/setup.factory.spec.ts(2 hunks)plugins/yaml-schema/vitest.config.ts(1 hunks)plugins/zip-extractor/jest.config.cjs(0 hunks)plugins/zip-extractor/src/zip.extractor.e2e.spec.ts(2 hunks)plugins/zip-extractor/vitest.config.ts(1 hunks)support/common-utils/src/async.map.spec.ts(2 hunks)support/common-utils/vitest.config.ts(1 hunks)test/betterConsoleLog.js(0 hunks)test/dotenv-config.js(0 hunks)test/setup-global.js(0 hunks)test/toBePendingMatcher.ts(0 hunks)test/unit.cleanup.js(0 hunks)utils/common/jest.config.cjs(0 hunks)utils/common/src/all.records.e2e.spec.ts(7 hunks)utils/common/src/async.batch.spec.ts(6 hunks)utils/common/src/logging.helper.spec.ts(4 hunks)utils/common/src/slugify.spec.ts(1 hunks)utils/common/vitest.config.ts(1 hunks)utils/extractor/jest.config.cjs(0 hunks)utils/extractor/vitest.config.ts(1 hunks)utils/fetch-schema/jest.config.cjs(0 hunks)utils/fetch-schema/vitest.config.ts(1 hunks)utils/file-buffer/jest.config.cjs(0 hunks)utils/file-buffer/vitest.config.ts(1 hunks)utils/response-rejection/jest.config.cjs(0 hunks)utils/response-rejection/vitest.config.ts(1 hunks)utils/testing/jest.config.cjs(0 hunks)utils/testing/src/test.helpers.ts(3 hunks)utils/testing/src/test.listener.spec.ts(2 hunks)utils/testing/src/toBePendingMatcher.ts(1 hunks)utils/testing/vitest.config.ts(1 hunks)validate/boolean/jest.config.cjs(0 hunks)validate/boolean/src/validate.boolean.plugin.spec.ts(6 hunks)validate/boolean/vitest.config.ts(1 hunks)validate/date/jest.config.cjs(0 hunks)validate/date/src/validate.date.plugin.e2e.spec.ts(2 hunks)
⛔ Files not processed due to max files limit (18)
- validate/date/vitest.config.ts
- validate/email/jest.config.cjs
- validate/email/src/validate.email.plugin.ts
- validate/email/src/validate.email.utils.spec.ts
- validate/email/src/validate.email.utils.ts
- validate/email/vitest.config.ts
- validate/isbn/jest.config.cjs
- validate/isbn/src/validate.isbn.plugin.spec.ts
- validate/isbn/vitest.config.ts
- validate/number/jest.config.cjs
- validate/number/src/validate.number.plugin.spec.ts
- validate/number/vitest.config.ts
- validate/phone/jest.config.cjs
- validate/phone/src/validate.phone.utils.spec.ts
- validate/phone/vitest.config.ts
- validate/string/jest.config.cjs
- validate/string/src/validate.string.plugin.spec.ts
- validate/string/vitest.config.ts
💤 Files with no reviewable changes (57)
- convert/currency/jest.config.cjs
- convert/translate/jest.config.cjs
- convert/what3words/jest.config.cjs
- enrich/geocode/jest.config.cjs
- enrich/gpx/jest.config.cjs
- enrich/sentiment/jest.config.cjs
- enrich/summarize/jest.config.cjs
- export/delimited-zip/jest.config.js
- export/pivot-table/jest.config.cjs
- extract/html-table/jest.config.cjs
- extract/markdown/jest.config.cjs
- import/faker/jest.config.cjs
- import/llm-records/jest.config.cjs
- import/rss/jest.config.cjs
- jest.config.js
- jest.setup.js
- plugins/autocast/jest.config.cjs
- plugins/automap/jest.config.cjs
- plugins/constraints/jest.config.cjs
- plugins/dedupe/jest.config.cjs
- plugins/delimiter-extractor/jest.config.cjs
- plugins/dxp-configure/jest.config.cjs
- plugins/export-workbook/jest.config.cjs
- plugins/foreign-db-extractor/jest.config.cjs
- plugins/graphql-schema/jest.config.cjs
- plugins/job-handler/jest.config.cjs
- plugins/json-extractor/jest.config.cjs
- plugins/json-schema/jest.config.cjs
- plugins/merge-connection/jest.config.cjs
- plugins/openapi-schema/jest.config.cjs
- plugins/pdf-extractor/jest.config.cjs
- plugins/psv-extractor/jest.config.cjs
- plugins/record-hook/jest.config.cjs
- plugins/rollout/jest.config.cjs
- plugins/space-configure/jest.config.cjs
- plugins/sql-ddl-converter/jest.config.cjs
- plugins/tsv-extractor/jest.config.cjs
- plugins/view-mapped/jest.config.cjs
- plugins/webhook-egress/jest.config.cjs
- plugins/webhook-event-forwarder/jest.config.cjs
- plugins/xlsx-extractor/jest.config.cjs
- plugins/xml-extractor/jest.config.cjs
- plugins/yaml-schema/jest.config.cjs
- plugins/zip-extractor/jest.config.cjs
- test/betterConsoleLog.js
- test/dotenv-config.js
- test/setup-global.js
- test/toBePendingMatcher.ts
- test/unit.cleanup.js
- utils/common/jest.config.cjs
- utils/extractor/jest.config.cjs
- utils/fetch-schema/jest.config.cjs
- utils/file-buffer/jest.config.cjs
- utils/response-rejection/jest.config.cjs
- utils/testing/jest.config.cjs
- validate/boolean/jest.config.cjs
- validate/date/jest.config.cjs
🚧 Files skipped from review as they are similar to previous changes (94)
- .changeset/five-hounds-wash.md
- config/vitest/src/index.js
- config/vitest/src/utils.js
- convert/currency/src/convert.currency.plugin.spec.ts
- convert/currency/src/convert.currency.plugin.ts
- convert/currency/vitest.config.ts
- convert/translate/src/convert.translate.plugin.spec.ts
- convert/translate/vitest.config.ts
- convert/what3words/vitest.config.ts
- enrich/geocode/src/enrich.geocode.plugin.spec.ts
- enrich/geocode/src/index.ts
- enrich/geocode/vitest.config.ts
- enrich/gpx/src/enrich.gpx.plugin.spec.ts
- enrich/gpx/vitest.config.ts
- enrich/sentiment/src/enrich.sentiment.plugin.spec.ts
- enrich/sentiment/src/enrich.sentiment.plugin.ts
- enrich/sentiment/vitest.config.ts
- enrich/summarize/src/summarize.plugin.spec.ts
- enrich/summarize/src/summarize.plugin.ts
- enrich/summarize/vitest.config.ts
- export/delimited-zip/vitest.config.ts
- export/pivot-table/vitest.config.ts
- extract/html-table/vitest.config.ts
- extract/markdown/src/parser.spec.ts
- extract/markdown/vitest.config.ts
- import/faker/src/faker.plugin.spec.ts
- import/faker/src/faker.utils.ts
- import/faker/vitest.config.ts
- import/llm-records/vitest.config.ts
- import/rss/vitest.config.ts
- plugins/autocast/src/autocast.plugin.spec.ts
- plugins/autocast/vitest.config.ts
- plugins/automap/vitest.config.ts
- plugins/constraints/src/external.constraint.e2e.spec.ts
- plugins/constraints/src/external.sheet.constraint.e2e.spec.ts
- plugins/constraints/vitest.config.ts
- plugins/dedupe/src/keep.first.logic.spec.ts
- plugins/dedupe/src/keep.last.logic.spec.ts
- plugins/dedupe/vitest.config.ts
- plugins/delimiter-extractor/src/parser.spec.ts
- plugins/delimiter-extractor/vitest.config.ts
- plugins/dxp-configure/src/shim.target.spec.ts
- plugins/dxp-configure/vitest.config.ts
- plugins/export-workbook/vitest.config.ts
- plugins/foreign-db-extractor/vitest.config.ts
- plugins/graphql-schema/src/generate.field.spec.ts
- plugins/graphql-schema/vitest.config.ts
- plugins/job-handler/src/job.handler.e2e.spec.ts
- plugins/job-handler/vitest.config.ts
- plugins/json-extractor/src/parser.spec.ts
- plugins/json-extractor/vitest.config.ts
- plugins/json-schema/src/setup.factory.spec.ts
- plugins/json-schema/vitest.config.ts
- plugins/merge-connection/vitest.config.ts
- plugins/openapi-schema/src/setup.factory.openapi.spec.ts
- plugins/openapi-schema/vitest.config.ts
- plugins/pdf-extractor/vitest.config.ts
- plugins/psv-extractor/vitest.config.ts
- plugins/record-hook/src/RecordHook.e2e.spec.ts
- plugins/record-hook/vitest.config.ts
- plugins/rollout/vitest.config.ts
- plugins/space-configure/vitest.config.ts
- plugins/sql-ddl-converter/vitest.config.ts
- plugins/tsv-extractor/vitest.config.ts
- plugins/view-mapped/vitest.config.ts
- plugins/webhook-egress/src/webhook.egress.e2e.spec.ts
- plugins/webhook-egress/vitest.config.ts
- plugins/webhook-event-forwarder/vitest.config.ts
- plugins/xlsx-extractor/src/parser.spec.ts
- plugins/xlsx-extractor/src/utils.spec.ts
- plugins/xlsx-extractor/vitest.config.ts
- plugins/xml-extractor/src/parser.spec.ts
- plugins/xml-extractor/vitest.config.ts
- plugins/yaml-schema/src/setup.factory.spec.ts
- plugins/yaml-schema/vitest.config.ts
- plugins/zip-extractor/src/zip.extractor.e2e.spec.ts
- plugins/zip-extractor/vitest.config.ts
- support/common-utils/src/async.map.spec.ts
- support/common-utils/vitest.config.ts
- utils/common/src/all.records.e2e.spec.ts
- utils/common/src/async.batch.spec.ts
- utils/common/src/logging.helper.spec.ts
- utils/common/vitest.config.ts
- utils/extractor/vitest.config.ts
- utils/fetch-schema/vitest.config.ts
- utils/file-buffer/vitest.config.ts
- utils/response-rejection/vitest.config.ts
- utils/testing/src/test.helpers.ts
- utils/testing/src/test.listener.spec.ts
- utils/testing/src/toBePendingMatcher.ts
- utils/testing/vitest.config.ts
- validate/boolean/src/validate.boolean.plugin.spec.ts
- validate/boolean/vitest.config.ts
- validate/date/src/validate.date.plugin.e2e.spec.ts
🧰 Additional context used
🪛 Biome
enrich/gpx/src/enrich.gpx.plugin.ts
[error] 183-183: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.
(lint/suspicious/noGlobalIsNan)
[error] 183-183: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.
(lint/suspicious/noGlobalIsNan)
🔇 Additional comments (19)
plugins/record-hook/rollup.config.mjs (2)
13-14: Verify TypeScript definition handling.
TypeScript definitions are disabled (includeDefinitions: false), but UMD is enabled. This might affect TypeScript users of the plugin.
Let's verify the TypeScript usage:
#!/bin/bash
# Description: Check for TypeScript usage and existing type definitions
echo "Checking for TypeScript files in the plugin..."
fd -e ts -e tsx . plugins/record-hook/
echo "Checking for existing type references..."
rg -l "\"@flatfile/plugin-record-hook\"" --type typescript11-13: Verify the exclusion of Node.js and browser bundles.
The configuration explicitly disables Node.js and browser-specific bundles, only keeping the UMD format. This might impact users depending on their environment.
Let's verify if this plugin is used in Node.js or browser environments:
✅ Verification successful
Bundle configuration is appropriate for this package
The configuration excluding Node.js and browser-specific bundles while keeping UMD is actually correct. Here's why:
- The package.json shows proper exports configuration that handles both Node.js and browser environments through the "exports" field:
"exports": { ".": { "node": { "types": {...}, "import": "./dist/index.js", "require": "./dist/index.cjs" }, "browser": { "types": {...}, "import": "./dist/index.browser.js", "require": "./dist/index.browser.cjs" } } }
- The package uses tsup for the main build (which handles Node.js and browser builds) and Rollup is only used for the UMD bundle
- The package is actively used across many plugins (as seen in the imports) and the current setup properly supports all these use cases
</details>
<details>
<summary>🏁 Scripts executed</summary>
The following scripts were executed for the analysis:
Script:
```shell
#!/bin/bash
# Description: Check for Node.js and browser usage patterns
# Look for imports/requires of this plugin in both Node.js and browser contexts
echo "Checking for Node.js usage patterns..."
rg -l "require.*@flatfile/plugin-record-hook|import.*@flatfile/plugin-record-hook" --type js --type ts
echo "Checking for browser-specific usage (checking for script tags)..."
rg -l "<script.*@flatfile/plugin-record-hook" --type html
Length of output: 903
Script:
#!/bin/bash
# Let's try a different approach to check usage patterns
# 1. Check for any imports/requires of this plugin
echo "Checking for imports of record-hook plugin..."
rg "from.*plugin-record-hook|require.*plugin-record-hook"
# 2. Check package.json to understand the intended usage
echo -e "\nChecking package.json configuration..."
cat plugins/record-hook/package.json
# 3. Look for documentation or examples
echo -e "\nChecking for documentation or examples..."
fd README.md plugins/record-hook/
if [ -f plugins/record-hook/README.md ]; then
cat plugins/record-hook/README.md
fi
Length of output: 14704
utils/common/src/slugify.spec.ts (2)
1-2: LGTM: Clean and minimal imports
The imports are correctly structured for Vitest and include only the necessary testing utilities.
Line range hint 1-42: Verify complete transition from Jest to Vitest
While this file correctly uses Vitest, let's ensure the transition is complete across the codebase.
plugins/space-configure/src/space.configure.e2e.spec.ts (3)
2-17: LGTM: Clean migration of test framework imports.
The transition from Jest to Vitest is properly implemented with all necessary imports. The code organization is clean and follows best practices.
Line range hint 51-77: LGTM: Well-structured test setup with proper lifecycle management.
The test configuration demonstrates good practices:
- Clear initialization of test dependencies
- Proper cleanup after tests
- Effective use of the new TestListener and setupDriver utilities
84-94:
Replace non-null assertions with explicit checks for safer testing.
The current implementation uses multiple non-null assertions that could mask potential runtime issues. Consider breaking down the assertions into explicit checks.
- const workspace = await api.workbooks.get(space.data.primaryWorkbookId!)
+ const primaryWorkbookId = space.data.primaryWorkbookId
+ expect(primaryWorkbookId).toBeDefined()
+ const workspace = await api.workbooks.get(primaryWorkbookId)
+
+ const { name, labels, sheets, actions } = workspace.data
+ expect(labels).toBeDefined()
+ expect(sheets?.[0]).toBeDefined()
+ expect(actions).toBeDefined()
+
+ expect(name).toBe(setup.workbooks[0].name)
+ expect(labels).toMatchObject(setup.workbooks[0].labels ?? [])
+ expect(sheets[0].name).toBe(setup.workbooks[0].sheets?.[0]?.name)
+ expect(sheets[0].config).toMatchObject(setup.workbooks[0].sheets?.[0] ?? {})
+ expect(actions).toMatchObject(setup.workbooks[0].actions ?? [])plugins/webhook-event-forwarder/src/forward-webhook.e2e.spec.ts (4)
5-19: LGTM: Clean transition to Vitest and improved test setup!
The migration from Jest to Vitest is well-implemented, and the new test setup using TestListener and setupDriver provides better test isolation.
Also applies to: 23-24
42-42: Remove non-null assertion operator.
The existing comment about avoiding the non-null assertion operator (!) is still valid. Consider implementing the suggested runtime check for better error handling.
68-77: Improve fetch mocking approach with Vitest.
The existing comment about extracting the mock response setup into a helper function is still valid. This would reduce duplication and improve maintainability.
Also applies to: 94-99
Line range hint 67-116: LGTM: Well-structured test cases!
The test cases effectively verify both success and error scenarios for webhook forwarding. The assertions are comprehensive and the error handling is well-tested.
plugins/automap/src/automap.plugin.e2e.spec.ts (4)
4-20: LGTM: Clean import organization for Vitest transition
The imports are well-organized and properly reflect the transition to Vitest, with all imported functions being utilized in the test file.
26-34: LGTM: Well-structured test setup
The test setup follows best practices with proper initialization sequence:
- TestListener instantiation
- Driver setup and start
- Listener mounting
41-44: Fix race condition in teardown sequence
The potential race condition in the teardown sequence still needs to be addressed.
46-52: LGTM: Clean listener state management
The beforeEach/afterEach hooks ensure proper isolation between tests by resetting the listener state.
enrich/gpx/src/enrich.gpx.plugin.ts (4)
1-2: LGTM! Clean import organization.
The separation of type and value imports using TypeScript's import type syntax is a good practice that improves code clarity and bundle optimization.
183-183: Skipping comment as past review is still valid.
A previous review comment already addresses the unsafe isNaN usage issue.
🧰 Tools
🪛 Biome
[error] 183-183: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.
(lint/suspicious/noGlobalIsNan)
[error] 183-183: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.
(lint/suspicious/noGlobalIsNan)
202-212: LGTM! Improved function signature formatting.
The multi-line formatting of the function signature and config destructuring improves readability.
229-231: Skipping comment as past review is still valid.
A previous review comment already addresses the need for date string validation.
2 participants
Please explain how to summarize this PR for the Changelog:
Tell code reviewer how and what to test: