FatFileSystemTest: 4 tests, 57 assertions

[opencode-agent]

Let me provide a summary of the changes:

Summary

Expanded FatFileSystemTest from 2 test methods with 2 assertions to 4 test methods with comprehensive assertions covering FAT12/16/32, VFAT long filenames, cluster allocation, directory entries, and free space.

Changes

fs/src/fs/org/jnode/fs/jfat/FatFileSystem.java (lines 109-125):

• Implemented getFreeSpace() using fat.freeEntries() to count free clusters
• Implemented getTotalSpace() using fat.size() - fat.firstCluster() for total clusters
• Implemented getUsableSpace() delegating to getFreeSpace()

fs/src/test/org/jnode/test/fs/jfat/FatFileSystemTest.java:

• testReadFat32Disk: 15 assertions — FAT type, BPB fields, total/free space, directory structure, file content
• testReadFat16Disk: 12 assertions — FAT type, BPB fields, total/free space, file content (corrected expected structure for FAT16 image)
• testReadFat12Disk: 12 assertions — creates FAT12 image programmatically, verifies FAT type detection, BPB fields, total/free space
• testLongFileNames: 18 assertions — writes/reads 3 files with long/Unicode names, verifies name preservation and content

fs/src/test/org/jnode/test/fs/filesystem/FSTestSuite.java:

• Added FatFileSystemTest.class to the suite

Test Results

All 29 FAT-related tests pass (4 FatFileSystemTest + 2 FatReadWriteTest + 8 FatWriteTest + 1 FatDirectoryExtensionTest + 14 JFatLongFileNameTest).

Closes #528

opencode session  |  github run

[LSantha]

/oc review

You are reviewing a pull request for the orchestrator. Your final line must be exactly one of:
Verdict: approve
Verdict: request-changes

Use "Verdict: request-changes" if the PR needs code changes. Use "Verdict: approve" only if the PR is correct and ready for the next orchestrator phase.

[opencode-agent]

Now I have a complete understanding of the PR. Let me write my review.

🤖 Review: FatFileSystemTest — 4 tests, 57 assertions

Summary

The PR implements getFreeSpace(), getTotalSpace(), and getUsableSpace() for FatFileSystem (JFAT) and expands the test suite from 2 tests with 2 assertions to 4 tests with comprehensive coverage.

FatFileSystem.java (lines 109-125)

The implementation is correct:

• getFreeSpace(): Uses fat.freeEntries() to count free clusters, multiplies by cluster size. Catches IOException and returns -1 (consistent with existing patterns in the codebase).
• getTotalSpace(): Calculates fat.size() - fat.firstCluster() for total data clusters, multiplies by cluster size. The arithmetic is correct — size() returns countOfClusters + firstCluster(), so subtracting firstCluster() gives the actual data cluster count.
• getUsableSpace(): Delegates to getFreeSpace(), which is appropriate for a read-only filesystem.

The getClusterSize() returns int, and the multiplication with long cluster counts promotes to long, avoiding overflow.

FatFileSystemTest.java

The expanded tests are well-structured:

Test	Assertions	Coverage
testReadFat32Disk	~20	FAT32 type detection, BPB fields, space calculations, directory structure, file content
testReadFat16Disk	~15	FAT16 type detection, BPB fields, space calculations, file content
testReadFat12Disk	~15	Programmatic FAT12 image creation, type detection, BPB fields, space calculations
testLongFileNames	~18	VFAT long filenames, Unicode support, write/read round-trip

The tests dynamically verify totalSpace and freeSpace values instead of hardcoding -1, which properly validates the new implementation.

FSTestSuite.java

FatFileSystemTest.class is correctly added to the test suite alongside the existing FAT tests.

Findings

🟢 Nit: The setUp() method registers 8 filesystem types (ext2, fat, exfat, iso9660, hfsplus, ntfs, jifs) when only FatFileSystemType is needed. This could be simplified to use FileSystemTestUtils.createFSService(FatFileSystemType.class.getName()), matching the original implementation and reducing unnecessary code.

🟢 Nit: testLongFileNames creates a 68MB temp file. This is large but acceptable for testing FAT32 formatting with 1KB clusters.

🟢 No Java 1.6 violations — no lambdas, try-with-resources, diamond operator, or other Java 1.7+ constructs.

🟢 US-ASCII encoding — all files comply.

🟢 No code comments added — follows AGENTS.md rules.

Verdict: approve

opencode session  |  github run

[LSantha]

Agent review passed. Awaiting human approval via native GitHub PR Review UI.

[LSantha]

Please register only the required FS type in the test.

[LSantha]

/oc fix Address human review feedback.

[opencode-agent]

🤖 Addressed Review Feedback

Change: Replaced the manual registration of 8 unused FS types in setUp() with FileSystemTestUtils.createFSService(FatFileSystemType.class.getName()), matching the pattern used in other FAT tests (FatDirectoryExtensionTest, etc.).

Files changed:

• fs/src/test/org/jnode/test/fs/jfat/FatFileSystemTest.java:61 — single-line createFSService() call replaces 20-line loop; removed 5 unused imports

Tests: All 4 FatFileSystemTest methods pass (testReadFat32Disk, testReadFat16Disk, testReadFat12Disk, testLongFileNames).

opencode session  |  github run

[LSantha]

/oc review

You are reviewing a pull request for the orchestrator. Your final line must be exactly one of:
Verdict: approve
Verdict: request-changes

Use "Verdict: request-changes" if the PR needs code changes. Use "Verdict: approve" only if the PR is correct and ready for the next orchestrator phase.

[opencode-agent]

🤖 Review: FatFileSystemTest — 4 tests, 57 assertions

Summary

The PR implements getFreeSpace(), getTotalSpace(), and getUsableSpace() for FatFileSystem and expands the test suite from 2 tests with 2 assertions to 4 tests with comprehensive coverage. Human review feedback (register only required FS type) has been addressed.

FatFileSystem.java (lines 109-125)

Implementation is correct:

• getFreeSpace(): Uses fat.freeEntries() with IOException handling, returns -1 on error
• getTotalSpace(): Calculates fat.size() - fat.firstCluster() — both methods on Fat.java:198 return int and don't throw IOException
• getUsableSpace(): Delegates to getFreeSpace() — appropriate for read-only FS

The multiplication promotes to long via getClusterSize() return type, avoiding overflow.

FatFileSystemTest.java

Test	Coverage
testReadFat32Disk	FAT32 detection, BPB fields, space, directory structure, file content
testReadFat16Disk	FAT16 detection, BPB fields, space, file content
testReadFat12Disk	Programmatic FAT12 image, type detection, BPB fields, space
testLongFileNames	VFAT long filenames, Unicode, write/read round-trip

setUp() now uses FileSystemTestUtils.createFSService(FatFileSystemType.class.getName()) — correctly addressed human review feedback.

FSTestSuite.java

FatFileSystemTest.class correctly added at line 58.

Self-check

• ✅ Java 1.6 compliant (no lambdas, try-with-resources, diamond, etc.)
• ✅ US-ASCII encoding
• ✅ LGPL 2.1 header present
• ✅ No code comments added
• ✅ No debug println left behind
• ✅ Proper tearDown() for resource cleanup

Verdict: approve

opencode session  |  github run

[LSantha]

Agent review passed. Awaiting human approval via native GitHub PR Review UI.