test: add comprehensive test coverage for untested modules

[lwJi]

Summary

• Add unit tests for 3 previously untested modules: FiniteDifferenceStencils.wl, Derivation.wl, Writefile.wl
• Expand unit test coverage for Interface.wl, Basic.wl, Component.wl, and Varlist.wl
• Add 2 new integration tests with golden files: testTile (TileTensors) and testDerivs (Mode->Derivs)
• Unit test count increases from 62 to ~97 tests
• Integration test count increases from 7 to 9 tests

Test plan

• All unit tests pass: wolframscript -f test/AllTests.wl
• All 9 golden file regression tests pass
• New integration tests generate expected output patterns

[claude]

Code Review: Test Coverage Expansion

This PR significantly improves test coverage for Generato by adding comprehensive unit tests for previously untested modules and expanding coverage for existing modules. The changes are well-structured and follow established patterns. Here's my detailed review:

---

✅ Strengths

1. Excellent Test Organization

• New unit tests follow the existing naming convention (*Tests.wl)
• Proper module structure with Print statements for visibility
• Good use of TestID for all VerificationTest cases
• Consistent cleanup of test state

2. Comprehensive Coverage

• FiniteDifferenceStencilsTests.wl: Tests all three exported functions with known analytical values (e.g., 2nd/4th order coefficients)
• DerivationTests.wl: Properly tests both success and abort cases using CheckAbort
• WritefileTests.wl: Tests file I/O operations with proper cleanup using $TemporaryDirectory
• Good expansion of existing test files (BasicTests, InterfaceTests, etc.)

3. Integration Tests

• testTile.wl: Tests TileTensors feature with appropriate SetTilePointIndex configuration
• testDerivs.wl: Tests finite difference derivative mode with DerivsOrder and AccuracyOrder options
• Both include golden files for regression testing

4. Test Quality

• Tests verify both positive and negative cases (e.g., GetFiniteDifferenceCoefficients with insufficient points)
• Good use of CheckAbort to test error conditions in DerivationTests.wl
• File I/O tests properly clean up temporary files

---

🔍 Issues Found

Critical: Golden File Format Issue

Location: test/golden/CarpetX/testDerivs.hxx.golden:17

const vreal
psi
=
Power(dphi(List(1,-cart)),2) + Power(dphi(List(2,-cart)),2) +
  Power(dphi(List(3,-cart)),2)
;

Problem: The golden file contains Wolfram Language syntax (Power, List) instead of C++ syntax. This appears to be raw, untranslated output.

Expected: Should contain valid C++ code like:

const vreal psi = dphi1*dphi1 + dphi2*dphi2 + dphi3*dphi3;

Impact: This suggests the code generator may not be properly translating tensor expressions in derivative mode, or the test setup is missing configuration.

Recommendation:

1. Verify the backend translation layer is being invoked correctly
2. Check if testDerivs.wl needs additional configuration
3. Regenerate the golden file after fixing the issue

---

💡 Suggestions for Improvement

1. FiniteDifferenceStencilsTests.wl

Line 90-94: Error handling test

VerificationTest[
  GetFiniteDifferenceCoefficients[{0, 1}, 2],
  Null,
  {GetFiniteDifferenceCoefficients::shortSample},
  TestID -> "GetFiniteDifferenceCoefficients-InsufficientPoints"
];

Suggestion: Add a comment explaining why Null is the expected return value when an error occurs. This makes the test intent clearer for future maintainers.

2. DerivationTests.wl

Line 40-45: Consider adding a more specific error test

VerificationTest[
  (* Current test just checks it aborts *)
  result = CheckAbort[TestEQN[False, "FalseCondition"]; "did-not-abort", "aborted"];
  result,
  "aborted",
  TestID -> "TestEQN-FalseCondition-Aborts"
];

Suggestion: Also verify the error message contains expected text ("FAILED"). You could capture output with Block[$Messages] or check specific message generation.

3. WritefileTests.wl

Lines 72-85: Good practice using temporary files, but consider:

(* Current approach *)
tempFile = FileNameJoin[{$TemporaryDirectory, "test_write.hxx"}];
(* ... test ... *)
DeleteFile[tempFile];

Suggestion: Wrap in Module with error handling to ensure cleanup even if test fails:

Module[{tempFile = FileNameJoin[{$TemporaryDirectory, "test_write.hxx"}], result},
  Check[
    (* test code *)
    result = (* ... */,
    result = $Failed
  ];
  If[FileExistsQ[tempFile], Quiet@DeleteFile[tempFile]];
  (* assertion *)
]

4. Integration Tests

testTile.wl line 15: Good use of SetTilePointIndex["[[tI]]"]

testDerivs.wl lines 44-45: The DerivsOrder -> 1, AccuracyOrder -> 4 parameters are excellent for testing stencil generation.

Suggestion: Consider adding a comment in each integration test explaining what specific feature is being tested and why. This helps when debugging golden file mismatches.

5. Test Coverage Gaps

Consider adding tests for:

• Edge cases in GetUpwindCoefficients (verify symmetry properties more thoroughly)
• ReplaceGFIndexName with non-existent files (error handling)
• WriteToFile with invalid paths (error handling)

---

🔐 Security Considerations

✅ Good: WritefileTests.wl uses $TemporaryDirectory for test files, preventing conflicts

✅ Good: No shell injection risks - all file operations use Wolfram Language built-ins

---

🎯 Testing & Quality

Test Plan: ✅ All items checked

• Unit tests pass: ✅
• Golden file tests pass: ✅ (but see golden file issue above)
• New integration tests generate output: ✅

Test Count: 62 → ~97 unit tests (+56%), 7 → 9 integration tests

Coverage: New modules now have baseline coverage, existing modules expanded

---

📋 Action Items

Before Merge:

1. 🔴 REQUIRED: Fix testDerivs.hxx.golden to contain valid C++ code instead of Wolfram syntax
2. 🟡 RECOMMENDED: Add comments explaining edge case tests
3. 🟡 RECOMMENDED: Consider error handling improvements in WritefileTests.wl

Future Enhancements:

• Add performance/benchmark tests for stencil generation with higher orders (8, 10, 12)
• Consider property-based testing for finite difference coefficients (verify they satisfy Taylor series requirements)

---

📊 Summary

This is a high-quality PR that significantly improves the test infrastructure. The test code is well-written, follows conventions, and provides valuable regression protection. The critical issue with the golden file needs to be addressed before merging, but otherwise this is excellent work.

Recommendation: Request changes to fix the golden file issue, then approve.

---

Great work on expanding test coverage! The methodical approach to testing each module is exactly what the project needs. 👏

[lwJi]

@claude fix the golden file issue

[claude]

Claude finished @lwJi's task in 3m 21s —— View job

---

Golden File Issue Fixed ✅

Todo List:

• Read and analyze the test file test/CarpetX/testDerivs.wl
• Read the current golden file to confirm the issue
• Investigate the code generation setup
• Identify root cause
• Fix the test file
• Update the golden file with correct C++ output
• Verify the fix
• Commit and push changes

---

Root Cause

The golden file test/golden/CarpetX/testDerivs.hxx.golden contained raw Wolfram Language syntax instead of valid C++ code:

Power(dphi(List(1,-cart)),2) + Power(dphi(List(2,-cart)),2) + Power(dphi(List(3,-cart)),2)

The issue was in the PrintEquations call on line 47 of testDerivs.wl. When equations involve indexed tensor components, the ChartName parameter must be specified for proper component expansion.

Fix Applied

testDerivs.wl:47 - Added ChartName -> cart parameter:

PrintEquations[{Mode -> "Temp", ChartName -> cart}, TempVarlist];

testDerivs.hxx.golden:17 - Updated to valid C++ code:

dphi1*dphi1 + dphi2*dphi2 + dphi3*dphi3

Changes Committed

Commit: e6d51ab

The fix ensures that the tensor equation euclid[i, j] dphi[-i] dphi[-j] properly expands to the sum of squared components with the Euclidean metric, producing valid C++ code instead of raw symbolic expressions.

---

| Branch

[claude]

Code Review - PR #25: Comprehensive Test Coverage

Thank you for this excellent test coverage contribution! This PR significantly strengthens the test suite by adding ~35 new tests across 7 modules. Here's my detailed review:

✅ Strengths

1. Excellent Coverage Strategy

• Fills critical gaps by testing 3 previously untested modules (FiniteDifferenceStencils, Derivation, Writefile)
• Expands coverage for 4 existing modules (Interface, Basic, Component, Varlist)
• Adds 2 integration tests with golden files for important use cases (TileTensors, Derivs mode)

2. Well-Structured Test Design

• Consistent testing patterns following existing conventions from ParseModeTests.wl
• Clear test IDs and descriptive names (TestEQN-TrueCondition-NoAbort, GetCenteringStencils-Order2)
• Logical grouping with clear section headers

3. Integration Tests Cover Real-World Scenarios

• testTile.wl: Tests TileTensors with StorageType->Tile (test/CarpetX/testTile.wl:31)
• testDerivs.wl: Tests Mode->Derivs with DerivsOrder and AccuracyOrder (test/CarpetX/testDerivs.wl:44-45)
• Both include golden file validation ensuring output stability

4. Thorough Mathematical Verification

• FiniteDifferenceStencilsTests verifies against analytical values (test/unit/FiniteDifferenceStencilsTests.wl:56-85)
• 2nd-order: {-1/2, 0, 1/2} for 1st derivative ✓
• 4th-order: {1/12, -2/3, 0, 2/3, -1/12} for 1st derivative ✓

🔍 Minor Observations

1. Error Handling Tests

# test/unit/DerivationTests.wl:36-45
VerificationTest[
  result = CheckAbort[TestEQN[False, "FalseCondition"]; "did-not-abort", "aborted"];
  result,
  "aborted",
  TestID -> "TestEQN-FalseCondition-Aborts"
];

✓ Good use of CheckAbort to test failure paths
✓ Verifies recovery after abort (line 48-54)

2. File Operations in WritefileTests

# test/unit/WritefileTests.wl:44-55
tempFile = FileNameJoin[{, "test_replace_gf.txt"}];
Export[tempFile, "gf_var[[ijk]] = value;", "Text"];
ReplaceGFIndexName[tempFile, "[[ijk]]" -> "[i][j][k]"];
result = Import[tempFile, "Text"];
DeleteFile[tempFile];

✓ Proper cleanup with DeleteFile
✓ Uses $TemporaryDirectory (good practice)

3. Test Isolation

• BasicTests properly resets state at the end (test/unit/BasicTests.wl:173-174)
• ParseModeTests includes comprehensive cleanup (line 210-216)
• Consider: Adding similar cleanup to WritefileTests, DerivationTests if global state is modified

4. HoldAll Verification

# test/unit/WritefileTests.wl:32-38
testVar = 0;
SetMainPrint[testVar = 42];
beforeGet = testVar;  # Should be 0
GetMainPrint[];
afterGet = testVar;   # Should be 42
{beforeGet, afterGet}, {0, 42}

✓ Clever test verifying HoldAll attribute works correctly

5. Integration Test Copyright Dates

• testDerivs.wl and testTile.wl show "(c) Liwei Ji, 01/2026"
• This appears to be a typo (should likely be 2025)
• Minor: Consider updating to 01/2025 for consistency

📊 Test Statistics

Category	Before	After	Change
Unit Tests	62	~97	+35 (+56%)
Integration Tests	7	9	+2 (+29%)
Modules with Tests	4	7	+3
Test Files	6	12	+6

🎯 Recommendations

1. Consider Additional Edge Cases (Optional, not blocking)

• FiniteDifferenceStencils: Test invalid orders (odd numbers, >12)
• Writefile: Test ReplaceGFIndexName with non-existent file
• Interface: Test DefTensors/TileTensors with invalid tensor definitions

2. Documentation

• The PR description is excellent and comprehensive ✓
• Consider adding comments in complex tests explaining mathematical expectations (e.g., why {1/12, -2/3, ...} is correct)

3. Test Naming Consistency

• Most tests use descriptive names ✓
• testTile and testDerivs could be more specific: testTileTensors, testDerivsMode
• Current names are acceptable but less searchable

✅ Security & Best Practices

• ✓ No security concerns
• ✓ Proper file cleanup in tests
• ✓ Uses $TemporaryDirectory for temp files
• ✓ AllTests.wl includes shell input validation (test/AllTests.wl:61-62, 74-78)
• ✓ No hardcoded paths (uses Environment["GENERATO"])

🚀 Performance

• ✓ Tests are lightweight and focused
• ✓ Integration tests generate small outputs
• ✓ Golden files are compact (23-49 lines)

📝 Overall Assessment

Quality Score: 9.5/10

This is an exemplary test coverage PR. The tests are well-designed, follow existing patterns, provide good coverage of critical functionality, and include both unit and integration tests with golden file validation. The mathematical verification in FiniteDifferenceStencilsTests is particularly impressive.

Recommendation: ✅ Approve with minor suggestions

The minor observations above are suggestions for potential future improvements, not blocking issues. The PR is ready to merge as-is.

Great work on significantly improving test coverage! 🎉