GOODBOY008 opened a new issue, #717:
URL: https://github.com/apache/fesod/issues/717
## Summary
This proposal outlines a comprehensive refactoring of FesodSheet's test
suite to improve maintainability, reduce code duplication, and adopt modern
JUnit 5 testing patterns.
**Reference PR**: #716
---
## Why? Current Issues
### 1. Code Duplication (~40% redundant code)
```
Current State:
├── SimpleDataTest.java → 3 nearly identical test methods (XLSX, XLS,
CSV)
├── ConverterDataTest.java → Same pattern repeated
├── FillDataTest.java → Same pattern repeated
└── ... (15+ test classes)
```
Each format requires separate test methods with copy-pasted logic:
```java
// Current: 3 methods doing the same thing
@Test void t01ReadAndWrite07() { readAndWrite(file07); }
@Test void t02ReadAndWrite03() { readAndWrite(file03); }
@Test void t03ReadAndWriteCsv() { readAndWrite(fileCsv); }
```
### 2. Listeners Mixed with Assertions (Violates SRP)
```java
// Current: Listener contains test assertions
public class SimpleDataListener extends AnalysisEventListener<SimpleData> {
@Override
public void doAfterAllAnalysed(AnalysisContext context) {
Assertions.assertEquals(10, list.size()); // ❌ Test logic in
listener
}
}
```
### 3. No Shared Test Infrastructure
- Each test package has its own data classes
- No reusable format providers
- No common assertion utilities for Excel-specific validations
### 4. Scattered Model Classes
- `ConverterReadData.java` and `ConverterWriteData.java` are nearly identical
- `ImageData.java` duplicated across packages
---
## How? Solution Approach
### New Testkit Infrastructure (`testkit/` package)
```
testkit/
├── base/
│ └── AbstractExcelTest.java # Format providers, temp file management
├── enums/
│ └── ExcelFormat.java # XLSX/XLS/CSV with capability metadata
├── listeners/
│ └── CollectingReadListener.java # Reusable data collector (no assertions)
└── assertions/
└── ExcelAssertions.java # Fluent Excel file assertions
```
### Parameterized Tests (JUnit 5)
```java
// After: Single parameterized method
@ParameterizedTest(name = "Format: {0}")
@MethodSource("allFormats")
void shouldWriteAndReadCorrectly(ExcelFormat format) {
File file = createTempFile("test", format);
List<SimpleData> result = roundTrip(file, SimpleData.class, testData);
assertThat(result).hasSize(10);
}
```
### Separation of Concerns
```java
// After: Listener only collects data
CollectingReadListener<T> listener = new CollectingReadListener<>();
FesodSheet.read(file, clazz, listener).sheet().doRead();
// Assertions in test method
assertThat(listener.getData()).hasSize(10);
```
---
## Target Structure
```
src/test/java/org/apache/fesod/sheet/
├── testkit/ # Shared test infrastructure
├── model/ # Shared data models
├── unit/ # Pure unit tests (no I/O)
├── integration/ # Integration tests (file I/O)
└── [legacy packages] # Deprecated, to be removed
```
---
## Benefits
| Metric | Before | After | Improvement |
|--------|--------|-------|-------------|
| Test Methods | ~300 | ~100 | **-67%** |
| Listener Classes | 25+ | 1 | **-96%** |
| Lines of Test Code | ~8,000 | ~5,000 | **-37%** |
| Format Coverage | Manual | Automatic | **100%** |
---
## Migration Strategy
| Phase | Scope | Status |
|-------|-------|--------|
| Phase 1 | Testkit Infrastructure | ✅ PR #716 |
| Phase 2 | simple/, converter/ | ✅ PR #716 |
| Phase 3 | fill/, style/, head/ | 🔲 Future |
| Phase 4 | template/, annotation/ | 🔲 Future |
| Phase 5 | Remaining packages | 🔲 Future |
| Phase 6 | Legacy cleanup | 🔲 Future |
---
## Discussion Points
1. **Backward Compatibility**: Keep deprecated tests running during
migration?
2. **Package Naming**: `integration/` vs feature-based (`read/`, `write/`)?
3. **AssertJ Dependency**: Added for fluent assertions - acceptable?
4. **Timeline**: Phased migration or big-bang?
---
## References
- Implementation PR: #716
- Full plan: `docs/TEST_REFACTORING_PLAN.md`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
