Skip to content

Commit be7981a

Browse files
josecelanojosecelano
committed
docs: [#149] update test organization to co-locate unit tests with code
- Change test strategy from separate tests/ folder to co-located tests - Use #[cfg(test)] modules within each file for unit tests - Rationale: All tests only depend on types within user_output module - Improves discoverability and maintainability - Follows project convention for unit tests Related to #102
1 parent cd6be7d commit be7981a

File tree

1 file changed

+389
-0
lines changed

1 file changed

+389
-0
lines changed

‎docs/issues/149-refactor-user-output-module-to-folder-structure.md‎

Lines changed: 389 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,389 @@
1+
# Refactor user_output.rs Module to Folder Structure
2+
3+
**Issue**: [#149](https://github.com/torrust/torrust-tracker-deployer/issues/149)
4+
**Parent Epic**: [#102](https://github.com/torrust/torrust-tracker-deployer/issues/102) - User Output Architecture Improvements
5+
**Related**:
6+
7+
- [User Output Module](https://github.com/torrust/torrust-tracker-deployer/blob/main/src/presentation/user_output.rs)
8+
- [Module Organization Guide](https://github.com/torrust/torrust-tracker-deployer/blob/main/docs/contributing/module-organization.md)
9+
10+
## Overview
11+
12+
The `src/presentation/user_output.rs` file has grown to **4,226 lines**, making it difficult to navigate and maintain. This task refactors the monolithic file into a well-organized folder module structure with focused, cohesive submodules that follow project conventions.
13+
14+
This refactoring improves:
15+
16+
- **Discoverability**: Clear separation makes it easier to find specific functionality
17+
- **Maintainability**: Smaller, focused files are easier to understand and modify
18+
- **Testability**: Tests can be organized alongside their related code
19+
- **Collaboration**: Multiple developers can work on different aspects simultaneously
20+
- **Code Review**: Smaller, focused changes are easier to review
21+
22+
## Goals
23+
24+
- [ ] Convert `user_output.rs` to `user_output/` folder module
25+
- [ ] Separate concerns into logical submodules
26+
- [ ] Maintain backward compatibility (no public API changes)
27+
- [ ] Ensure all tests pass without modification
28+
- [ ] Follow project module organization conventions
29+
- [ ] Improve code discoverability and navigation
30+
31+
## 🏗️ Architecture Requirements
32+
33+
**DDD Layer**: Presentation
34+
**Module Path**: `src/presentation/user_output/`
35+
**Pattern**: Folder module with focused submodules
36+
37+
### Module Structure Requirements
38+
39+
- [ ] Follow folder module conventions (see [Module Organization Guide](../contributing/module-organization.md))
40+
- [ ] Use `mod.rs` as the public API entry point with re-exports
41+
- [ ] Group related functionality in focused submodules
42+
- [ ] Organize tests alongside their code (when appropriate)
43+
- [ ] Maintain clear public/private boundaries
44+
45+
### Architectural Constraints
46+
47+
- [ ] **No public API changes** - All existing public types, traits, and functions must remain accessible
48+
- [ ] **Backward compatibility** - Existing code using `UserOutput` must work without changes
49+
- [ ] **Test compatibility** - All existing tests must pass without modification
50+
- [ ] **Import paths** - Existing imports like `use crate::presentation::user_output::UserOutput;` must continue to work
51+
52+
### Anti-Patterns to Avoid
53+
54+
- ❌ Breaking public API surface
55+
- ❌ Circular dependencies between submodules
56+
- ❌ Leaking internal implementation details through public re-exports
57+
- ❌ Creating submodules that are too granular (prefer cohesion)
58+
59+
## Specifications
60+
61+
### Current State Analysis
62+
63+
The `user_output.rs` file (4,226 lines) contains:
64+
65+
**Core Components** (~800 lines):
66+
67+
- `Theme` struct and implementations
68+
- `VerbosityLevel` enum
69+
- `Channel` enum
70+
- `UserOutput` main struct
71+
72+
**Traits and Abstractions** (~300 lines):
73+
74+
- `OutputMessage` trait
75+
- `FormatterOverride` trait
76+
- `OutputSink` trait
77+
78+
**Message Types** (~600 lines):
79+
80+
- `ProgressMessage`
81+
- `SuccessMessage`
82+
- `WarningMessage`
83+
- `ErrorMessage`
84+
- `ResultMessage`
85+
- `StepsMessage` and `StepsMessageBuilder`
86+
- `InfoBlockMessage` and `InfoBlockMessageBuilder`
87+
88+
**Infrastructure** (~500 lines):
89+
90+
- `VerbosityFilter` private component
91+
- Type-safe wrappers: `StdoutWriter`, `StderrWriter`
92+
- `StandardSink`, `CompositeSink`, `FileSink`, `TelemetrySink`
93+
94+
**Testing Infrastructure** (~200 lines):
95+
96+
- `test_support` module with `TestWriter` and `TestUserOutput`
97+
98+
**Tests** (~1,800 lines):
99+
100+
- Comprehensive test suite covering all functionality
101+
102+
### Proposed Folder Structure
103+
104+
```text
105+
src/presentation/user_output/
106+
├── mod.rs # Public API, re-exports, module documentation
107+
├── core.rs # UserOutput main struct and core impl
108+
├── theme.rs # Theme struct and predefined themes
109+
├── verbosity.rs # VerbosityLevel enum and VerbosityFilter
110+
├── channel.rs # Channel enum
111+
├── traits.rs # OutputMessage, FormatterOverride, OutputSink traits
112+
├── messages/ # Message type implementations
113+
│ ├── mod.rs
114+
│ ├── progress.rs # ProgressMessage
115+
│ ├── success.rs # SuccessMessage
116+
│ ├── warning.rs # WarningMessage
117+
│ ├── error.rs # ErrorMessage
118+
│ ├── result.rs # ResultMessage
119+
│ ├── steps.rs # StepsMessage and builder
120+
│ └── info_block.rs # InfoBlockMessage and builder
121+
├── sinks/ # OutputSink implementations
122+
│ ├── mod.rs
123+
│ ├── standard.rs # StandardSink
124+
│ ├── composite.rs # CompositeSink
125+
│ ├── file.rs # FileSink
126+
│ ├── telemetry.rs # TelemetrySink
127+
│ └── writers.rs # StdoutWriter, StderrWriter wrappers
128+
├── formatters/ # FormatterOverride implementations
129+
│ ├── mod.rs
130+
│ └── json.rs # JsonFormatter
131+
└── test_support.rs # TestWriter, TestUserOutput (kept as submodule)
132+
```
133+
134+
### Module Responsibility Matrix
135+
136+
| Module | Responsibility | Public Items | Lines (approx) |
137+
| ----------------- | ------------------------------------ | ------------------------------------------------------------ | -------------- |
138+
| `mod.rs` | Public API and documentation | Re-exports all public items | ~150 |
139+
| `core.rs` | Main `UserOutput` struct and methods | `UserOutput` | ~400 |
140+
| `theme.rs` | Theme configuration | `Theme` | ~150 |
141+
| `verbosity.rs` | Verbosity levels and filtering | `VerbosityLevel`, `VerbosityFilter` (private) | ~200 |
142+
| `channel.rs` | Output channel routing | `Channel` | ~50 |
143+
| `traits.rs` | Core abstractions | `OutputMessage`, `FormatterOverride`, `OutputSink` | ~150 |
144+
| `messages/*.rs` | Message implementations | All message types and builders | ~700 |
145+
| `sinks/*.rs` | Output sink implementations | `StandardSink`, `CompositeSink`, `FileSink`, `TelemetrySink` | ~500 |
146+
| `formatters/*.rs` | Formatter implementations | `JsonFormatter` | ~100 |
147+
| `test_support.rs` | Testing utilities | `TestWriter`, `TestUserOutput` | ~200 |
148+
149+
### Key Design Decisions
150+
151+
#### 1. Folder Module Pattern
152+
153+
Use a folder with `mod.rs` as the entry point, following Rust conventions:
154+
155+
- `mod.rs` contains public API and re-exports
156+
- Internal modules are private by default
157+
- Public types/traits are explicitly re-exported
158+
- Module-level documentation lives in `mod.rs`
159+
160+
#### 2. Message Organization
161+
162+
Group message types in a `messages/` subfolder:
163+
164+
- Each message type gets its own file
165+
- Builders stay with their message types
166+
- `messages/mod.rs` re-exports all message types
167+
168+
#### 3. Sink Organization
169+
170+
Group sink implementations in a `sinks/` subfolder:
171+
172+
- Each sink type gets its own file
173+
- Type-safe writers (`StdoutWriter`, `StderrWriter`) in `writers.rs`
174+
- `sinks/mod.rs` re-exports all sink types
175+
176+
#### 4. Test Organization
177+
178+
**Keep unit tests co-located with their modules** using `#[cfg(test)]`:
179+
180+
- Tests stay in the same file as the code they test (e.g., `theme.rs` contains `Theme` and its tests)
181+
- Use `#[cfg(test)]` modules to group tests within each file
182+
- Only create a separate `tests/` folder if integration tests are needed (tests that import from multiple modules)
183+
- Current tests are all unit tests (only use types from within `user_output` module), so they should stay co-located
184+
185+
**Rationale**: Unit tests that only depend on types from a single module should be kept close to the code they test. This improves discoverability and makes it easier to maintain tests alongside implementation changes.
186+
187+
#### 5. Test Infrastructure
188+
189+
The `test_support.rs` module provides utilities for testing:
190+
191+
- `TestWriter` - Captures output for verification
192+
- `TestUserOutput` - Test fixture with captured stdout/stderr
193+
194+
This module should remain accessible for other modules that need to test user output functionality.
195+
196+
#### 6. Backward Compatibility
197+
198+
Maintain existing import paths:
199+
200+
```rust
201+
// These must continue to work
202+
use crate::presentation::user_output::UserOutput;
203+
use crate::presentation::user_output::{Theme, VerbosityLevel};
204+
use crate::presentation::user_output::test_support::TestUserOutput;
205+
```
206+
207+
## Implementation Plan
208+
209+
### Phase 1: Create Folder Structure and Core Modules (1-2 hours)
210+
211+
- [ ] Create `src/presentation/user_output/` directory
212+
- [ ] Create `mod.rs` with module documentation and structure outline
213+
- [ ] Extract `Theme` to `theme.rs` with tests in `#[cfg(test)]` module
214+
- [ ] Extract `VerbosityLevel` and `VerbosityFilter` to `verbosity.rs` with tests in `#[cfg(test)]` module
215+
- [ ] Extract `Channel` to `channel.rs` with tests in `#[cfg(test)]` module
216+
- [ ] Update `mod.rs` with re-exports for backward compatibility
217+
- [ ] Verify existing code still compiles and all tests pass
218+
219+
### Phase 2: Extract Traits and Core Logic (1-2 hours)
220+
221+
- [ ] Extract `OutputMessage`, `FormatterOverride`, `OutputSink` to `traits.rs`
222+
- [ ] Extract main `UserOutput` struct and methods to `core.rs`
223+
- [ ] Update `mod.rs` with re-exports
224+
- [ ] Verify all functionality works
225+
226+
### Phase 3: Organize Message Types (2-3 hours)
227+
228+
- [ ] Create `messages/` subdirectory
229+
- [ ] Extract each message type to its own file with tests in `#[cfg(test)]` modules:
230+
- [ ] `progress.rs` - `ProgressMessage` with tests
231+
- [ ] `success.rs` - `SuccessMessage` with tests
232+
- [ ] `warning.rs` - `WarningMessage` with tests
233+
- [ ] `error.rs` - `ErrorMessage` with tests
234+
- [ ] `result.rs` - `ResultMessage` with tests
235+
- [ ] `steps.rs` - `StepsMessage`, `StepsMessageBuilder`, and tests
236+
- [ ] `info_block.rs` - `InfoBlockMessage`, `InfoBlockMessageBuilder`, and tests
237+
- [ ] Create `messages/mod.rs` with re-exports
238+
- [ ] Update parent `mod.rs` with message re-exports
239+
- [ ] Verify all message tests pass
240+
241+
### Phase 4: Organize Sink Implementations (1-2 hours)
242+
243+
- [ ] Create `sinks/` subdirectory
244+
- [ ] Extract `StdoutWriter` and `StderrWriter` to `writers.rs` with tests in `#[cfg(test)]` module
245+
- [ ] Extract sink implementations to individual files with tests:
246+
- [ ] `standard.rs` - `StandardSink` with tests
247+
- [ ] `composite.rs` - `CompositeSink` with tests
248+
- [ ] `file.rs` - `FileSink` with tests
249+
- [ ] `telemetry.rs` - `TelemetrySink` with tests
250+
- [ ] Create `sinks/mod.rs` with re-exports
251+
- [ ] Update parent `mod.rs` with sink re-exports
252+
- [ ] Verify all sink tests pass
253+
254+
### Phase 5: Organize Formatters (30 minutes)
255+
256+
- [ ] Create `formatters/` subdirectory
257+
- [ ] Extract `JsonFormatter` to `json.rs` with tests in `#[cfg(test)]` module
258+
- [ ] Create `formatters/mod.rs` with re-exports
259+
- [ ] Update parent `mod.rs` with formatter re-exports
260+
- [ ] Verify formatter tests pass
261+
262+
### Phase 6: Organize Test Infrastructure (30 minutes)
263+
264+
- [ ] Extract `test_support` module to `test_support.rs`
265+
- [ ] Verify `test_support` is accessible for testing in other modules
266+
- [ ] Verify all co-located unit tests still pass
267+
- [ ] Check that test utilities (TestWriter, TestUserOutput) work correctly
268+
269+
### Phase 7: Documentation and Cleanup (30 minutes)
270+
271+
- [ ] Update module-level documentation in `mod.rs`
272+
- [ ] Add module documentation to each submodule
273+
- [ ] Update imports in `mod.rs` for clarity
274+
- [ ] Remove old `user_output.rs` file
275+
- [ ] Verify documentation builds: `cargo doc --no-deps`
276+
277+
### Phase 8: Final Verification (30 minutes)
278+
279+
- [ ] Run pre-commit checks: `./scripts/pre-commit.sh`
280+
- [ ] Verify no public API changes
281+
- [ ] Verify all tests pass
282+
- [ ] Verify no unused dependencies: `cargo machete`
283+
- [ ] Check for any remaining TODO comments
284+
- [ ] Manual smoke test of basic functionality
285+
286+
## Acceptance Criteria
287+
288+
> **Note for Contributors**: These criteria define what the PR reviewer will check. Use this as your pre-review checklist before submitting the PR to minimize back-and-forth iterations.
289+
290+
**Quality Checks**:
291+
292+
- [ ] Pre-commit checks pass: `./scripts/pre-commit.sh`
293+
- [ ] All tests pass without modification: `cargo test`
294+
- [ ] Documentation builds successfully: `cargo doc --no-deps`
295+
- [ ] No unused dependencies: `cargo machete`
296+
297+
**Structural Criteria**:
298+
299+
- [ ] `user_output.rs` is converted to `user_output/` folder
300+
- [ ] `mod.rs` serves as the public API entry point
301+
- [ ] All submodules are appropriately sized (< 500 lines each)
302+
- [ ] Clear separation of concerns across submodules
303+
- [ ] Test organization mirrors module structure
304+
305+
**Backward Compatibility**:
306+
307+
- [ ] All existing public types remain accessible
308+
- [ ] All existing import paths continue to work
309+
- [ ] No changes required in code using `UserOutput`
310+
- [ ] All existing tests pass without modification
311+
- [ ] Public API surface is identical
312+
313+
**Code Quality**:
314+
315+
- [ ] Each module has clear, focused responsibility
316+
- [ ] Module documentation explains purpose and usage
317+
- [ ] Follows project module organization conventions
318+
- [ ] No circular dependencies between submodules
319+
- [ ] Private implementation details not leaked
320+
321+
**Testing**:
322+
323+
- [ ] Tests organized by component
324+
- [ ] Test coverage maintained (no reduction)
325+
- [ ] Test infrastructure (`test_support`) easily accessible
326+
- [ ] Integration tests verify cross-component functionality
327+
328+
## Related Documentation
329+
330+
- [Module Organization Guide](../contributing/module-organization.md) - Module organization conventions
331+
- [User Output Epic #102](https://github.com/torrust/torrust-tracker-deployer/issues/102) - Parent epic
332+
- [Codebase Architecture](../codebase-architecture.md) - Overall architecture principles
333+
- [Testing Conventions](../contributing/testing/) - Testing best practices
334+
335+
## Notes
336+
337+
### Rationale for Refactoring
338+
339+
The current 4,226-line file violates the Single Responsibility Principle and makes navigation difficult. Breaking it into focused modules:
340+
341+
1. **Improves Discoverability**: Clear file names make it easy to find specific functionality
342+
2. **Reduces Cognitive Load**: Developers work with smaller, focused files
343+
3. **Enables Parallel Development**: Multiple developers can work on different aspects
344+
4. **Simplifies Testing**: Tests can be organized alongside related code
345+
5. **Follows Best Practices**: Aligns with Rust community conventions
346+
347+
### Design Considerations
348+
349+
**Why Not More Granular?**
350+
351+
The proposed structure balances granularity with cohesion:
352+
353+
- **Messages**: Each message type is simple enough to stand alone
354+
- **Sinks**: Each sink has distinct responsibility (console, file, composite, telemetry)
355+
- **Core**: Keeps related `UserOutput` methods together
356+
357+
**Why Separate `messages/` and `sinks/`?**
358+
359+
These represent different architectural concerns:
360+
361+
- **Messages**: Define the "what" (content and formatting)
362+
- **Sinks**: Define the "where" (output destinations)
363+
364+
#### Test Organization Strategy
365+
366+
Tests are organized in a dedicated `tests/` folder to:
367+
368+
- Keep test code separate from production code
369+
- Mirror the module structure for easy navigation
370+
- Group integration tests separately from unit tests
371+
372+
### Migration Safety
373+
374+
This refactoring is **zero-risk** for existing code:
375+
376+
- All public items remain in the same namespace via re-exports
377+
- No API changes required
378+
- Existing tests validate correctness
379+
- Can be done incrementally and tested at each step
380+
381+
### Estimated Total Time
382+
383+
**8-10 hours** total:
384+
385+
- Core refactoring: 6-8 hours
386+
- Testing and verification: 2 hours
387+
- Documentation: 1 hour
388+
389+
Can be done in multiple commits/sessions without breaking existing functionality.

0 commit comments

Comments
 (0)