Refine DBF loader tests to generate fixtures at runtime#2
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughAdds DBF metadata types, a DBF table loader, and a table catalog for directory enumeration. Updates tooling to include a dbfinfo command to display metadata. Documents metadata surfacing in architecture.md. Introduces tests generating synthetic DBF files to validate header parsing, sidecar detection, and catalog ordering. Minor .csproj formatting. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller as Caller
participant Loader as DbfTableLoader
participant FS as File System
Caller->>Loader: Load(filePath)
Loader->>FS: Open file (read-only)
Loader->>FS: Read 32-byte header
Loader->>Loader: Parse version/date/count/lengths/LDID
alt header has tail
Loader->>FS: Read remaining header
end
loop Field descriptors (32 bytes)
Loader->>Loader: ReadFieldDescriptor(...)
end
Loader->>FS: Probe sidecars (DBT/FPT/NDX/NTX/MDX)
Loader-->>Caller: DbfTableDescriptor
sequenceDiagram
autonumber
actor User as User
participant CLI as Tools Program (dbfinfo)
participant Catalog as TableCatalog
participant Loader as DbfTableLoader
participant FS as File System
User->>CLI: dbfinfo <path>
alt path is directory
CLI->>Catalog: EnumerateTables(directory)
Catalog->>FS: Enumerate *.dbf
loop each .dbf
Catalog->>Loader: Load(dbfPath)
Loader-->>Catalog: DbfTableDescriptor
end
Catalog-->>CLI: List<DbfTableDescriptor> (sorted)
else path is .dbf file
CLI->>Loader: Load(filePath)
Loader-->>CLI: DbfTableDescriptor
end
CLI->>User: Print table metadata and fields
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (7)
Comment |
There was a problem hiding this comment.
Pull Request Overview
This PR refactors DBF loader tests to generate fixture data at runtime instead of relying on static fixture files. It replaces static DBF fixture files with runtime-generated test tables and sidecars, updates tests to use temporary workspaces, and adds a new dbfinfo command-line tool.
- Introduces runtime DBF fixture generation through
DbfFixtureWorkspaceand helper classes - Adds new
TableCatalogclass for discovering and enumerating DBF files in directories - Extends the command-line tool with a
dbfinfocommand for inspecting DBF metadata
Reviewed Changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/XBase.Core.Tests/DbfTableLoaderTests.cs | Complete rewrite using runtime fixture generation instead of static files |
| src/XBase.Tools/Program.cs | Added dbfinfo command with argument queue processing for DBF inspection |
| src/XBase.Core/Table/TableCatalog.cs | New class for discovering and enumerating DBF tables in directories |
| src/XBase.Core/Table/DbfTableLoader.cs | New DBF file loader with header parsing and sidecar detection |
| src/XBase.Core/Table/DbfMetadata.cs | New metadata classes for DBF table descriptors and field schemas |
| architecture.md | Updated documentation to reflect new loader and catalog components |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| public string CreateSidecar(string fileName) | ||
| { | ||
| string path = Path.Combine(DirectoryPath, fileName); | ||
| File.WriteAllBytes(path, Array.Empty<byte>()); |
There was a problem hiding this comment.
Using Array.Empty<byte>() for creating empty files is unnecessary. Use File.Create(path).Dispose() or using var _ = File.Create(path); instead to avoid allocating an empty byte array.
| File.WriteAllBytes(path, Array.Empty<byte>()); | |
| File.Create(path).Dispose(); |
| byte[] buffer = new byte[dataLength]; | ||
| stream.Write(buffer); |
There was a problem hiding this comment.
For large record counts, this could allocate significant memory. Consider using stream.SetLength() or writing in smaller chunks to avoid large memory allocations for test fixtures.
| byte[] buffer = new byte[dataLength]; | |
| stream.Write(buffer); | |
| // Write in chunks to avoid large allocations | |
| const int chunkSize = 4096; | |
| byte[] buffer = new byte[Math.Min(chunkSize, dataLength)]; | |
| int bytesRemaining = dataLength; | |
| while (bytesRemaining > 0) | |
| { | |
| int bytesToWrite = Math.Min(buffer.Length, bytesRemaining); | |
| stream.Write(buffer, 0, bytesToWrite); | |
| bytesRemaining -= bytesToWrite; | |
| } |
| foreach (DbfTableDescriptor table in tables) | ||
| { | ||
| fileLookup.TryGetValue(table.Name, out string? actualPath); | ||
| PrintTable(table, actualPath ?? Path.Combine(target, table.Name + ".dbf")); |
There was a problem hiding this comment.
The fallback path construction using Path.Combine(target, table.Name + \".dbf\") may not match the actual file if casing differs. Consider using the file enumeration result directly or handling case sensitivity consistently.
| PrintTable(table, actualPath ?? Path.Combine(target, table.Name + ".dbf")); | |
| if (actualPath != null) | |
| { | |
| PrintTable(table, actualPath); | |
| } | |
| else | |
| { | |
| Console.Error.WriteLine($"Warning: File for table '{table.Name}' not found in directory '{target}'."); | |
| PrintTable(table, null); | |
| } |
| foreach (string extension in IndexExtensions) | ||
| { | ||
| string candidate = tableName + extension; | ||
| if (lookup.TryGetValue(candidate, out string? actualName) && actualName is not null) | ||
| { | ||
| if (!indexFileNames.Contains(actualName, StringComparer.OrdinalIgnoreCase)) |
There was a problem hiding this comment.
Using Contains() with StringComparer.OrdinalIgnoreCase on a List has O(n) complexity. Consider using a HashSet<string> with the comparer for O(1) lookups when checking for duplicates.
| foreach (string extension in IndexExtensions) | |
| { | |
| string candidate = tableName + extension; | |
| if (lookup.TryGetValue(candidate, out string? actualName) && actualName is not null) | |
| { | |
| if (!indexFileNames.Contains(actualName, StringComparer.OrdinalIgnoreCase)) | |
| HashSet<string> indexFileNamesSet = new(StringComparer.OrdinalIgnoreCase); | |
| foreach (string extension in IndexExtensions) | |
| { | |
| string candidate = tableName + extension; | |
| if (lookup.TryGetValue(candidate, out string? actualName) && actualName is not null) | |
| { | |
| if (indexFileNamesSet.Add(actualName)) |
Summary
Testing
https://chatgpt.com/codex/tasks/task_e_68dc735dfe7483229e0993b3b52d15df
Summary by CodeRabbit
New Features
Documentation
Tests