Compare commits
1 Commits
edd516755a
...
ba37b199de
| Author | SHA1 | Date | |
|---|---|---|---|
| ba37b199de |
@@ -1,388 +0,0 @@
|
||||
# Testing Strategy Handoff Document
|
||||
|
||||
**Project:** shapes - Java Shape Editor
|
||||
**Date:** March 27, 2026
|
||||
**Author:** AI Coding Orchestrator
|
||||
|
||||
---
|
||||
|
||||
## 1. Problem Statement
|
||||
|
||||
The objective was to explore the project, identify gaps in test coverage, and propose a testing strategy to improve code quality and reliability.
|
||||
|
||||
### Initial Questions
|
||||
- What is the current test coverage?
|
||||
- What classes/packages are tested vs. untested?
|
||||
- What is the recommended testing approach moving forward?
|
||||
|
||||
---
|
||||
|
||||
## 2. Project Overview
|
||||
|
||||
### Tech Stack
|
||||
- **Language:** Java 16
|
||||
- **Build Tool:** Maven
|
||||
- **Testing Framework:** JUnit 5 (Jupiter)
|
||||
- **Logging:** SLF4J + Logback
|
||||
|
||||
### Package Structure
|
||||
```
|
||||
src/main/java/ovh/gasser/newshapes/
|
||||
├── App.java # Main application entry
|
||||
├── Selection.java # Selection management
|
||||
├── ShapeVisitor.java # Visitor interface
|
||||
├── SVGExporter.java # SVG export logic
|
||||
├── HTMLExporter.java # HTML export logic
|
||||
├── shapes/
|
||||
│ ├── Shape.java # Shape interface
|
||||
│ ├── AbstractShape.java # Base abstract class
|
||||
│ ├── SRectangle.java # Rectangle shape
|
||||
│ ├── SCircle.java # Circle shape
|
||||
│ ├── STriangle.java # Triangle shape
|
||||
│ ├── SText.java # Text shape
|
||||
│ ├── SCollection.java # Shape collection (composite)
|
||||
│ └── ResizeHandle.java # Resize handle enum
|
||||
├── attributes/
|
||||
│ ├── Attributes.java # Base attributes interface
|
||||
│ ├── SelectionAttributes.java # Selection state
|
||||
│ └── ColorAttributes.java # Fill/stroke colors
|
||||
├── ui/
|
||||
│ ├── Controller.java # Main controller
|
||||
│ ├── ShapesView.java # View component
|
||||
│ ├── ShapeDraftman.java # Drawing component
|
||||
│ ├── listeners/
|
||||
│ │ ├── MenuAddListener.java
|
||||
│ │ ├── MenuEditListener.java
|
||||
│ │ └── SelectionListener.java
|
||||
│ └── visitors/
|
||||
│ ├── SVGDraftman.java # SVG visitor implementation
|
||||
│ └── HTMLDraftman.java # HTML visitor implementation
|
||||
└── util/
|
||||
└── Streamable.java # Stream support interface
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 3. Initial Exploration Findings
|
||||
|
||||
### 3.1 Existing Test Coverage (Before Work)
|
||||
|
||||
| Test File | Coverage |
|
||||
|-----------|----------|
|
||||
| AbstractShapeTest.java | Partial (6 tests) |
|
||||
| SRectangleTest.java | ✅ |
|
||||
| SCircleTest.java | ✅ |
|
||||
| STriangleTest.java | ✅ |
|
||||
| STextTest.java | ✅ |
|
||||
| SVGExporterTest.java | ✅ |
|
||||
| HTMLExporterTest.java | ✅ |
|
||||
|
||||
**Total: 36 tests across 7 test classes**
|
||||
|
||||
### 3.2 Classes with NO Tests
|
||||
|
||||
| Priority | Class | Reason for Testing Need |
|
||||
|----------|-------|------------------------|
|
||||
| 🔴 High | SCollection | Core composite pattern, child management, bounds calculation |
|
||||
| 🔴 High | Selection | Critical selection management, listener notifications |
|
||||
| 🔴 High | Streamable | Interface with default method used throughout |
|
||||
| 🟡 Medium | SelectionAttributes | Core data type |
|
||||
| 🟡 Medium | ColorAttributes | Core data type |
|
||||
| 🟡 Medium | SVGDraftman | Complex string generation |
|
||||
| 🟡 Medium | HTMLDraftman | Complex string generation |
|
||||
| 🟢 Low | Controller | UI logic, integration points |
|
||||
| 🟢 Low | ShapeDraftman | UI rendering |
|
||||
| 🟢 Low | ShapesView | UI component |
|
||||
|
||||
### 3.3 Issues in Existing Tests
|
||||
|
||||
1. **Shallow coverage** - Most shape tests only verify creation and bounds
|
||||
2. **No edge cases** - Missing: null handling, negative coordinates, zero dimensions
|
||||
3. **Reflection usage** - SCircleTest uses reflection to check color (fragile)
|
||||
4. **No contract tests** - Shape interface has no tests verifying invariants
|
||||
5. **Inconsistent clone testing** - Some tests verify deep copy, others don't
|
||||
|
||||
---
|
||||
|
||||
## 4. Work Completed
|
||||
|
||||
### 4.1 Tests Added
|
||||
|
||||
Three new test files created with 21 new tests:
|
||||
|
||||
#### SCollectionTest.java (12 tests)
|
||||
```java
|
||||
src/test/java/ovh/gasser/newshapes/shapes/SCollectionTest.java
|
||||
```
|
||||
|
||||
| Test Method | Purpose |
|
||||
|-------------|---------|
|
||||
| testCreateWithShapes | Verify SCollection.of() creates collection with shapes |
|
||||
| testAdd | Verify add() adds a shape to the collection |
|
||||
| testRemove | Verify remove() removes a shape from the collection |
|
||||
| testIterator | Verify iterator() iterates over children |
|
||||
| testStream | Verify stream() returns a stream of children |
|
||||
| testGetBoundsEmptyCollection | Verify getBounds() returns Rectangle(WIN_SIZE) when empty |
|
||||
| testGetBoundsWithChildren | Verify getBounds() returns union of all children's bounds |
|
||||
| testTranslate | Verify translate() moves all children |
|
||||
| testClone | Verify clone() creates deep copy with SelectionAttributes |
|
||||
| testToString | Verify toString() contains SCollection |
|
||||
| testAddAttributesPropagatesToChildren | Verify ColorAttributes are propagated to children |
|
||||
| testGetAttributesReturnsChildColor | Verify getAttributes(ColorAttributes.ID) returns first child's color |
|
||||
|
||||
#### SelectionTest.java (7 tests)
|
||||
```java
|
||||
src/test/java/ovh/gasser/newshapes/SelectionTest.java
|
||||
```
|
||||
|
||||
| Test Method | Purpose |
|
||||
|-------------|---------|
|
||||
| testIsEmptyInitially | Verify isEmpty() returns true initially |
|
||||
| testAdd | Verify add() adds shape and marks it selected |
|
||||
| testAddAll | Verify addAll() adds multiple shapes |
|
||||
| testClear | Verify clear() removes all shapes and marks them unselected |
|
||||
| testGetSelectedShapesReturnsCopy | Verify getSelectedShapes() returns immutable copy |
|
||||
| testAddListener | Verify addListener() and listener notification |
|
||||
| testNullAddAllDoesNothing | Verify addAll(null) doesn't throw |
|
||||
|
||||
#### StreamableTest.java (2 tests)
|
||||
```java
|
||||
src/test/java/ovh/gasser/newshapes/util/StreamableTest.java
|
||||
```
|
||||
|
||||
| Test Method | Purpose |
|
||||
|-------------|---------|
|
||||
| testStreamReturnsStreamOfElements | Verify stream() returns all elements |
|
||||
| testStreamEmptyCollection | Verify stream() works on empty collection |
|
||||
|
||||
### 4.2 Test Results
|
||||
|
||||
```
|
||||
Tests run: 57, Failures: 0, Errors: 0, Skipped: 0
|
||||
BUILD SUCCESS
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 5. Oracle Strategic Review
|
||||
|
||||
Consulted @oracle for architectural guidance on testing strategy.
|
||||
|
||||
### 5.1 Assessment
|
||||
|
||||
**Overall Grade: B- (Adequate but shallow)**
|
||||
|
||||
| Area | Coverage | Quality |
|
||||
|------|----------|---------|
|
||||
| Shapes | ~60% | Basic happy-path only |
|
||||
| SCollection | Good | Decent coverage including edge cases |
|
||||
| Selection | Good | Tests core behavior + listeners |
|
||||
| Exporters | Good | Structure + format validation |
|
||||
| AbstractShape | Good | Tests polymorphic behavior |
|
||||
|
||||
### 5.2 Recommended Priority
|
||||
|
||||
| Priority | Area | Rationale |
|
||||
|----------|------|-----------|
|
||||
| P0 | Attributes tests | SelectionAttributes, ColorAttributes - core data types |
|
||||
| P1 | Visitor tests | SVGDraftman/HTMLDraftman - complex string generation |
|
||||
| P1 | contains(Point) | Critical hit-testing behavior |
|
||||
| P2 | UI tests | Controller integration points |
|
||||
| P2 | Edge cases | Zero dimensions, negative coords, null handling |
|
||||
| P3 | ResizeHandle | All 8 handles should be tested |
|
||||
|
||||
### 5.3 CI/CD Recommendations
|
||||
|
||||
1. **Add JaCoCo** - Enforce 80% code coverage
|
||||
2. **Parallel tests** - Enable parallel test execution in surefire
|
||||
3. **GitHub Actions** - Add CI pipeline
|
||||
|
||||
---
|
||||
|
||||
## 6. Visitor Testing Approach (@oracle)
|
||||
|
||||
### 6.1 Why Direct Visitor Tests?
|
||||
|
||||
Current exporter tests obscure visitor logic in the full pipeline. Direct tests provide:
|
||||
- **Isolation** - Tests fail at specific visitor method
|
||||
- **Faster feedback** - No complex shape construction needed
|
||||
- **Edge case targeting** - Test specific attribute combinations
|
||||
|
||||
### 6.2 Recommended Structure
|
||||
|
||||
```
|
||||
src/test/java/ovh/gasser/newshapes/ui/visitors/
|
||||
├── SVGDraftmanTest.java # Direct visitor tests
|
||||
└── HTMLDraftmanTest.java
|
||||
```
|
||||
|
||||
### 6.3 Assertions Strategy
|
||||
|
||||
| Use Exact Match For | Use Contains/Partial For |
|
||||
|---------------------|---------------------------|
|
||||
| Element tags (`<rect>`, `<circle>`) | Dynamic values (coords, colors) |
|
||||
| Fixed attributes (xmlns, DOCTYPE) | CSS output ordering |
|
||||
| Structure markers | |
|
||||
|
||||
### 6.4 Edge Cases to Cover
|
||||
|
||||
- Null ColorAttributes
|
||||
- Filled only / stroked only / both / neither
|
||||
- Empty collections
|
||||
- Nested SCollection
|
||||
- Text font styles (ITALIC, BOLD, ITALIC+BOLD)
|
||||
|
||||
### 6.5 Bug Found 🐛
|
||||
|
||||
**HTMLDraftman.visitTriangle()** line 69:
|
||||
```java
|
||||
htmlOutput.printf("<div class=\"triangle%d\"></div>\n", this.hashCode()); // BUG
|
||||
```
|
||||
Should be `sTriangle.hashCode()`. Direct visitor tests would catch this.
|
||||
|
||||
---
|
||||
|
||||
## 7. Remaining Work
|
||||
|
||||
### 7.1 High Priority
|
||||
|
||||
| # | Task | Estimated Effort |
|
||||
|---|------|-----------------|
|
||||
| 1 | Add SelectionAttributes test | 1 hour |
|
||||
| 2 | Add ColorAttributes test | 1 hour |
|
||||
| 3 | Create ShapeContractTest (parameterized) | 2 hours |
|
||||
|
||||
### 7.2 Medium Priority
|
||||
|
||||
| # | Task | Estimated Effort |
|
||||
|---|------|-----------------|
|
||||
| 4 | Add SVGDraftmanTest | 2 hours |
|
||||
| 5 | Add HTMLDraftmanTest | 2 hours |
|
||||
| 6 | Add contains(Point) tests | 2-3 hours |
|
||||
|
||||
### 7.3 Lower Priority
|
||||
|
||||
| # | Task | Estimated Effort |
|
||||
|---|------|-----------------|
|
||||
| 7 | Add ResizeHandle tests | 1 hour |
|
||||
| 8 | Add edge case tests (null, negative, zero) | 2 hours |
|
||||
| 9 | Fix HTMLDraftman.visitTriangle() bug | 30 min |
|
||||
| 10 | Set up JaCoCo + CI | 1 hour |
|
||||
|
||||
---
|
||||
|
||||
## 8. Appendix
|
||||
|
||||
### A. Current Test Files
|
||||
|
||||
```
|
||||
src/test/java/ovh/gasser/newshapes/
|
||||
├── shapes/
|
||||
│ ├── AbstractShapeTest.java # 6 tests
|
||||
│ ├── SRectangleTest.java # 4 tests
|
||||
│ ├── SCircleTest.java # 5 tests
|
||||
│ ├── STriangleTest.java # 3 tests
|
||||
│ ├── STextTest.java # 5 tests
|
||||
│ └── SCollectionTest.java # 12 tests (NEW)
|
||||
├── exporters/
|
||||
│ ├── SVGExporterTest.java # 7 tests
|
||||
│ └── HTMLExporterTest.java # 6 tests
|
||||
├── SelectionTest.java # 7 tests (NEW)
|
||||
└── util/
|
||||
└── StreamableTest.java # 2 tests (NEW)
|
||||
```
|
||||
|
||||
### B. Maven Dependencies
|
||||
|
||||
```xml
|
||||
<dependency>
|
||||
<groupId>org.junit.jupiter</groupId>
|
||||
<artifactId>junit-jupiter-api</artifactId>
|
||||
<version>5.10.0</version>
|
||||
<scope>test</scope>
|
||||
</dependency>
|
||||
<dependency>
|
||||
<groupId>org.junit.jupiter</groupId>
|
||||
<artifactId>junit-jupiter-engine</artifactId>
|
||||
<version>5.10.0</version>
|
||||
<scope>test</scope>
|
||||
</dependency>
|
||||
```
|
||||
|
||||
### C. JaCoCo Configuration (Recommended)
|
||||
|
||||
```xml
|
||||
<plugin>
|
||||
<groupId>org.jacoco</groupId>
|
||||
<artifactId>jacoco-maven-plugin</artifactId>
|
||||
<version>0.8.11</version>
|
||||
<executions>
|
||||
<execution>
|
||||
<goals>
|
||||
<goal>prepare-agent</goal>
|
||||
<goal>report</goal>
|
||||
<goal>check</goal>
|
||||
</goals>
|
||||
</execution>
|
||||
</executions>
|
||||
<configuration>
|
||||
<rules>
|
||||
<rule>
|
||||
<element>CLASS</element>
|
||||
<limits>
|
||||
<limit>
|
||||
<counter>LINE</counter>
|
||||
<value>COVEREDRATIO</value>
|
||||
<minimum>0.80</minimum>
|
||||
</limit>
|
||||
</limits>
|
||||
</rule>
|
||||
</rules>
|
||||
</configuration>
|
||||
</plugin>
|
||||
```
|
||||
|
||||
### D. Shape Contract Test (Recommended Pattern)
|
||||
|
||||
```java
|
||||
class ShapeContractTest {
|
||||
@ParameterizedTest
|
||||
@MethodSource("ovh.gasser.newshapes.shapes.ShapeFactory#allShapes")
|
||||
void testCloneIsIndependent(Shape s) { ... }
|
||||
|
||||
@ParameterizedTest
|
||||
@MethodSource("ovh.gasser.newshapes.shapes.ShapeFactory#allShapes")
|
||||
void testGetBoundsReturnsCopy(Shape s) { ... }
|
||||
|
||||
@ParameterizedTest
|
||||
@MethodSource("ovh.gasser.newshapes.shapes.ShapeFactory#allShapes")
|
||||
void testTranslateMutatesInPlace(Shape s) { ... }
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 9. Summary
|
||||
|
||||
| Metric | Before | After |
|
||||
|--------|--------|-------|
|
||||
| Total Tests | 36 | 57 |
|
||||
| Test Classes | 7 | 10 |
|
||||
| Core Classes Tested | 8 | 11 |
|
||||
| Coverage Grade | C | B- |
|
||||
|
||||
**Key Achievements:**
|
||||
- Identified and filled gaps in core business logic (SCollection, Selection, Streamable)
|
||||
- Received architectural guidance from @oracle
|
||||
- Discovered potential bug in HTMLDraftman
|
||||
- Established clear roadmap for remaining work
|
||||
|
||||
**Next Steps:**
|
||||
1. Add Attributes tests (SelectionAttributes, ColorAttributes)
|
||||
2. Add visitor tests (SVGDraftman, HTMLDraftman)
|
||||
3. Add Shape contract tests
|
||||
4. Set up CI/CD with JaCoCo coverage gate
|
||||
|
||||
---
|
||||
|
||||
*End of Handoff Document*
|
||||
Reference in New Issue
Block a user