Skip to content

Commit 31227bc

Browse files
committed
Merge #19: refactor: [#14] Phase 2.3 - Three-Layer Testing Architecture & Documentation Improvements
0a36bbc fix: [#14] correct configure-env test logic to eliminate CI warning (Jose Celano) efd79da fix: [#14] add .gitkeep to preserve application/config/templates directory structure (Jose Celano) e505846 docs: add table of contents to copilot instructions (Jose Celano) db45c94 refactor: [#14] separate testing concerns across three-layer architecture (Jose Celano) d3fe01e docs: [#14] fix table formatting in copilot instructions (Jose Celano) 8c75916 refactor: [#14] reorganize Makefile into infrastructure/application layers (Jose Celano) 732a297 feat: [#14] improve cloud-init completion detection robustness (Jose Celano) 8ba6858 refactor: [#14] centralize log_section function and improve Docker Compose deployment (Jose Celano) 42c52c4 refactor: [#14] move general utility functions from test-e2e.sh to shell-utils.sh (Jose Celano) Pull request description: ## 🎯 Overview This PR completes **Phase 2.3** of the twelve-factor app refactoring initiative (#14), implementing a strict **three-layer testing architecture** and comprehensive documentation improvements to ensure maintainable separation of concerns. ## 🚀 Key Achievements ### ✅ **Three-Layer Testing Architecture Implementation** Implemented strict separation of testing concerns across three layers to prevent future mixing of concerns and improve maintainability: **1. Project-Wide/Global Layer** (`tests/` folder) - **Command**: `make test-ci` - **Scope**: Cross-cutting concerns and orchestration - **Responsibilities**: Global syntax validation, Makefile validation, orchestrates other layers **2. Infrastructure Layer** (`infrastructure/` folder) - **Command**: `make infra-test-ci` - **Scope**: Infrastructure-specific validation ONLY - **Responsibilities**: Terraform/OpenTofu syntax, cloud-init templates, infrastructure scripts **3. Application Layer** (`application/` folder) - **Command**: `make app-test-ci` - **Scope**: Application-specific validation ONLY - **Responsibilities**: Docker Compose syntax, application configuration, deployment scripts ### ✅ **Enhanced Documentation & Contributor Experience** - **Table of Contents**: Added comprehensive TOC to `.github/copilot-instructions.md` for improved navigation - **AI Assistant Guidelines**: Enhanced with explicit testing layer separation warnings and examples - **Contributor Documentation**: Updated with three-layer architecture requirements and best practices ### ✅ **Shell Utilities Consolidation** - **Centralized Functions**: Moved reusable shell functions to `scripts/shell-utils.sh` - **Consistent Logging**: Standardized `log_section` function across all scripts - **Sudo Cache Management**: Improved user experience for infrastructure operations ### ✅ **Makefile Organization** - **Layer Separation**: Clear distinction between infrastructure and application commands - **Orchestration**: Project-wide test orchestrator that respects layer boundaries - **Backward Compatibility**: Legacy commands maintained with proper deprecation notices ## 📁 Files Changed ### Core Architecture Files - **`Makefile`**: Refactored test targets for three-layer architecture - **`tests/test-ci.sh`**: New project-wide orchestrator for global concerns - **`infrastructure/tests/test-ci.sh`**: Refactored to focus only on infrastructure concerns - **`application/tests/test-ci.sh`**: New application-layer CI test script ### Documentation & Guidelines - **`.github/copilot-instructions.md`**: Added TOC and enhanced AI assistant guidelines - **`docs/refactoring/`**: Added comprehensive refactoring documentation - **`tests/README.md`**: Updated to reflect three-layer architecture ### Utilities & Scripts - **`scripts/shell-utils.sh`**: Consolidated utility functions with sudo cache management - **Infrastructure scripts**: Updated to use centralized utilities - **Cloud-init templates**: Improved completion detection robustness ## 🔧 Benefits Delivered ### **Maintainable Architecture** - ✅ **Clear separation**: Each layer focuses only on its own concerns - ✅ **Prevent regression**: Strong guidelines prevent future mixing of concerns - ✅ **Better debugging**: Issues can be isolated to specific layers - ✅ **Faster CI**: Each layer can be tested independently when needed ### **Enhanced Developer Experience** - ✅ **Better navigation**: TOC makes comprehensive documentation more accessible - ✅ **Clear guidelines**: AI assistants and contributors have explicit boundaries - ✅ **Improved workflows**: Centralized utilities improve script consistency - ✅ **Quality assurance**: All changes pass complete CI validation ### **Future-Proof Foundation** - ✅ **Scalable testing**: Architecture supports adding new layers if needed - ✅ **Cultural enforcement**: Documentation prevents regression to mixed concerns - ✅ **Technical enforcement**: Make targets enforce proper layer separation ## 🧪 Testing & Validation ### **Complete CI Validation** ```bash make test-ci # ✅ All project-wide tests pass ├── Global concerns (syntax, structure, Makefile) ✅ ├── make infra-test-ci (Infrastructure layer only) ✅ └── make app-test-ci (Application layer only) ✅ ``` ### **Layer Separation Validation** - ✅ **Infrastructure tests**: Only test infrastructure concerns (no global linting calls) - ✅ **Application tests**: Only test application concerns (CI-friendly environment handling) - ✅ **Global tests**: Handle cross-cutting concerns and orchestration only ### **Documentation Quality** - ✅ **Markdown linting**: All documentation passes markdownlint validation - ✅ **Link validation**: Table of contents links work correctly in GitHub - ✅ **Comprehensive coverage**: All major workflow aspects documented ## 🔗 Relationship to Issue #14 This PR addresses **Phase 2.3** objectives from the twelve-factor refactoring plan: - ✅ **Testing Infrastructure**: Three-layer architecture prevents future technical debt - ✅ **Documentation Improvements**: Enhanced contributor guidelines and navigation - ✅ **Shell Utilities**: Consolidated and improved script utilities - ✅ **Foundation Completion**: Solid base for remaining twelve-factor implementation **Next Phase**: Configuration management system implementation (template-based configurations) ## 🔍 Review Focus Areas ### **Testing Architecture** - Verify that each test layer only handles its own concerns - Confirm `make test-ci` properly orchestrates all layers - Check that AI assistant guidelines prevent future violations ### **Documentation Quality** - Review table of contents navigation and completeness - Validate AI assistant guidelines are clear and actionable - Confirm refactoring documentation provides good context ### **Code Quality** - Verify shell utilities follow project standards - Check that Makefile changes maintain backward compatibility - Confirm all linting and syntax validation passes --- **Ready for Review** ✅ | **All CI Tests Pass** ✅ | **Documentation Complete** ✅ ACKs for top commit: josecelano: ACK 0a36bbc Tree-SHA512: fba11e0c5baff4fc1798be7dd1567b507fc9601ccd2fe619b33ee388d44513cb9cce97a5dd5f3c91f69c61413fc6cdb5c12ca5c72063bc10271b75b11f841177
2 parents 71e04ea + 0a36bbc commit 31227bc

19 files changed

+837
-303
lines changed

.github/copilot-instructions.md

Lines changed: 131 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,26 @@
11
# Torrust Tracker Demo - Contributor Guide
22

3+
## Table of Contents
4+
5+
- [🎯 Project Overview](#-project-overview)
6+
- [Current Major Initiative](#current-major-initiative)
7+
- [📁 Repository Structure](#-repository-structure)
8+
- [Key Components](#key-components)
9+
- [🛠️ Development Workflow](#️-development-workflow)
10+
- [Quick Start for Contributors](#quick-start-for-contributors)
11+
- [Main Commands](#main-commands)
12+
- [📋 Conventions and Standards](#-conventions-and-standards)
13+
- [Twelve-Factor App Principles](#twelve-factor-app-principles)
14+
- [Git Workflow](#git-workflow)
15+
- [Code Quality Standards](#code-quality-standards)
16+
- [Testing Requirements](#testing-requirements)
17+
- [Security Guidelines](#security-guidelines)
18+
- [🚀 Getting Started](#-getting-started)
19+
- [For New Contributors](#for-new-contributors)
20+
- [For Infrastructure Changes](#for-infrastructure-changes)
21+
- [For AI Assistants](#for-ai-assistants)
22+
- [📖 Additional Resources](#-additional-resources)
23+
324
## 🎯 Project Overview
425

526
**Torrust Tracker Demo** is the complete production deployment configuration for running a live [Torrust Tracker](https://github.com/torrust/torrust-tracker) instance. This repository provides:
@@ -132,30 +153,30 @@ cd torrust-tracker-demo
132153
make install-deps
133154

134155
# 3. Setup SSH key for VMs
135-
make setup-ssh-key
156+
make infra-config-local
136157

137158
# 4. Test twelve-factor deployment workflow locally
138159
make infra-apply # Provision infrastructure (platform setup)
139160
make app-deploy # Deploy application (Build + Release + Run stages)
140-
make health-check # Validate deployment
141-
make ssh # Connect to VM
161+
make app-health-check # Validate deployment
162+
make vm-ssh # Connect to VM
142163
make infra-destroy # Cleanup
143164

144165
# 5. Run tests
145-
make test # Full infrastructure test
146-
make test-syntax # Syntax validation only
166+
make test-e2e # Full infrastructure test
167+
make lint # Syntax validation only
147168
```
148169

149170
### Main Commands
150171

151172
#### Twelve-Factor Workflow (Recommended)
152173

153-
| Command | Purpose |
154-
| ------------------- | ------------------------------------------------- |
155-
| `make infra-apply` | Provision infrastructure (platform setup) |
156-
| `make app-deploy` | Deploy application (Build + Release + Run stages) |
157-
| `make app-redeploy` | Redeploy application (Release + Run stages only) |
158-
| `make health-check` | Validate deployment health |
174+
| Command | Purpose |
175+
| ----------------------- | ------------------------------------------------- |
176+
| `make infra-apply` | Provision infrastructure (platform setup) |
177+
| `make app-deploy` | Deploy application (Build + Release + Run stages) |
178+
| `make app-redeploy` | Redeploy application (Release + Run stages only) |
179+
| `make app-health-check` | Validate deployment health |
159180

160181
#### Infrastructure Management
161182

@@ -171,19 +192,18 @@ make test-syntax # Syntax validation only
171192

172193
#### VM Access and Debugging
173194

174-
| Command | Purpose |
175-
| ----------------- | --------------------------------- |
176-
| `make ssh` | Connect to deployed VM |
177-
| `make console` | Access VM console (text-based) |
178-
| `make vm-console` | Access VM graphical console (GUI) |
195+
| Command | Purpose |
196+
| --------------------- | --------------------------------- |
197+
| `make vm-ssh` | Connect to deployed VM |
198+
| `make vm-console` | Access VM console (text-based) |
199+
| `make vm-gui-console` | Access VM graphical console (GUI) |
179200

180201
#### Testing and Validation
181202

182-
| Command | Purpose |
183-
| ------------------ | --------------------------------------- |
184-
| `make test` | Run complete infrastructure tests |
185-
| `make test-syntax` | Run syntax validation only |
186-
| `make lint` | Run all linting (alias for test-syntax) |
203+
| Command | Purpose |
204+
| --------------- | --------------------------------- |
205+
| `make test-e2e` | Run complete infrastructure tests |
206+
| `make lint` | Run syntax validation only |
187207

188208
#### Legacy Commands (Deprecated)
189209

@@ -245,7 +265,7 @@ The twelve-factor **Build, Release, Run** stages apply to the application deploy
245265
1. **Initial Setup**: `make infra-apply``make app-deploy`
246266
2. **Code Changes**: `make app-redeploy` (skips infrastructure)
247267
3. **Infrastructure Changes**: `make infra-apply``make app-redeploy`
248-
4. **Validation**: `make health-check`
268+
4. **Validation**: `make app-health-check`
249269
5. **Cleanup**: `make infra-destroy`
250270

251271
### Git Workflow
@@ -349,6 +369,66 @@ The project includes a comprehensive linting script that validates all file type
349369
- **No secrets**: Never commit SSH keys, passwords, or tokens
350370
- **Documentation**: Update docs for any infrastructure changes
351371

372+
#### Three-Layer Testing Architecture
373+
374+
**CRITICAL**: This project uses a strict three-layer testing architecture.
375+
**Never mix concerns across layers**:
376+
377+
**1. Project-Wide/Global Layer** (`tests/` folder)
378+
379+
- **Command**: `make test-ci`
380+
- **Scope**: Cross-cutting concerns that span all layers
381+
- **Responsibilities**:
382+
- Global syntax validation (`make lint` / `./scripts/lint.sh`)
383+
- Project structure validation (`tests/test-unit-project.sh`)
384+
- Makefile validation
385+
- Orchestrates infrastructure and application layer tests
386+
387+
**2. Infrastructure Layer** (`infrastructure/` folder)
388+
389+
- **Command**: `make infra-test-ci`
390+
- **Scope**: Infrastructure-specific validation ONLY
391+
- **Responsibilities**:
392+
- Terraform/OpenTofu syntax validation
393+
- Cloud-init template validation
394+
- Infrastructure script validation
395+
- Infrastructure configuration validation
396+
- **NEVER**: Call global commands like `make lint` from infrastructure tests
397+
398+
**3. Application Layer** (`application/` folder)
399+
400+
- **Command**: `make app-test-ci`
401+
- **Scope**: Application-specific validation ONLY
402+
- **Responsibilities**:
403+
- Docker Compose syntax validation
404+
- Application configuration validation
405+
- Deployment script validation
406+
- Grafana dashboard validation
407+
- **NEVER**: Call global commands like `make lint` from application tests
408+
409+
**Testing Hierarchy:**
410+
411+
```text
412+
make test-ci (Project-wide orchestrator)
413+
├── Global concerns (syntax, structure, Makefile)
414+
├── make infra-test-ci (Infrastructure layer only)
415+
└── make app-test-ci (Application layer only)
416+
```
417+
418+
**Common Mistake to Avoid:**
419+
420+
- ❌ Adding `make lint` calls inside `infrastructure/tests/test-ci.sh`
421+
- ❌ Testing application concerns from infrastructure tests
422+
- ❌ Testing infrastructure concerns from application tests
423+
- ✅ Keep each layer focused on its own responsibilities only
424+
- ✅ Use `make test-ci` for complete validation that orchestrates all layers
425+
426+
**GitHub Actions Integration:**
427+
428+
- Uses `make test-ci` (not `make infra-test-ci`) to ensure all layers are tested
429+
- Runs without virtualization (CI-compatible)
430+
- Maintains separation of concerns for maintainable testing
431+
352432
#### End-to-End Smoke Testing
353433

354434
For verifying the functionality of the tracker from an end-user's perspective (e.g., simulating announce/scrape requests), refer to the **Smoke Testing Guide**. This guide explains how to use the official `torrust-tracker-client` tools to perform black-box testing against a running tracker instance without needing a full BitTorrent client.
@@ -408,8 +488,8 @@ The project implements intelligent sudo cache management to improve the user exp
408488

409489
```bash
410490
make install-deps # Install dependencies
411-
make setup-ssh-key # Configure SSH access
412-
make test-prereq # Verify setup
491+
make infra-config-local # Configure SSH access
492+
make infra-test-prereq # Verify setup
413493
```
414494

415495
3. **Install recommended VS Code extensions**:
@@ -439,7 +519,7 @@ The project implements intelligent sudo cache management to improve the user exp
439519
```bash
440520
make infra-apply # Deploy test VM
441521
make app-deploy # Deploy application
442-
make ssh # Verify access
522+
make vm-ssh # Verify access
443523
make infra-destroy # Clean up
444524
```
445525

@@ -448,7 +528,7 @@ The project implements intelligent sudo cache management to improve the user exp
448528
### For Infrastructure Changes
449529

450530
1. **Local testing first**: Always test infrastructure changes locally
451-
2. **Validate syntax**: Run `make test-syntax` before committing
531+
2. **Validate syntax**: Run `make lint` before committing
452532
3. **Document changes**: Update relevant documentation
453533
4. **Test twelve-factor workflow**: Ensure both infrastructure provisioning and application deployment work
454534
```bash
@@ -467,6 +547,27 @@ When providing assistance:
467547
- Test infrastructure changes locally before suggesting them
468548
- Provide clear explanations and documentation
469549
- Consider the migration to Hetzner infrastructure in suggestions
550+
- **CRITICAL**: Respect the three-layer testing architecture (see Testing Requirements above)
551+
552+
#### Testing Layer Separation (CRITICAL)
553+
554+
**NEVER** mix testing concerns across the three layers:
555+
556+
- **Infrastructure tests** (`infrastructure/tests/`) should ONLY test infrastructure concerns
557+
- **Application tests** (`application/tests/`) should ONLY test application concerns
558+
- **Global tests** (`tests/`) handle cross-cutting concerns and orchestration
559+
560+
**Common violations to avoid:**
561+
562+
- ❌ Adding `make lint` calls in `infrastructure/tests/test-ci.sh`
563+
- ❌ Testing Docker Compose from infrastructure layer
564+
- ❌ Testing Terraform from application layer
565+
- ❌ Any layer calling commands from other layers directly
566+
567+
**Always use the proper orchestrator:**
568+
569+
- Use `make test-ci` for complete testing (orchestrates all layers)
570+
- Use layer-specific commands only when targeting that specific layer
470571

471572
#### Command Execution Context
472573

@@ -482,15 +583,15 @@ Be mindful of the execution context for different types of commands. The project
482583

483584
When executing commands on the remote VM, be aware of limitations with interactive sessions.
484585

485-
- **Problem**: The VS Code integrated terminal may not correctly handle commands that initiate a new interactive shell, such as `ssh torrust@<VM_IP>` or `make ssh`. The connection may succeed, but subsequent commands sent to that shell may not execute as expected, and their output may not be captured.
586+
- **Problem**: The VS Code integrated terminal may not correctly handle commands that initiate a new interactive shell, such as `ssh torrust@<VM_IP>` or `make vm-ssh`. The connection may succeed, but subsequent commands sent to that shell may not execute as expected, and their output may not be captured.
486587

487588
- **Solution**: Prefer executing commands non-interactively whenever possible. Instead of opening a persistent SSH session, pass the command directly to `ssh`.
488589

489590
- **Don't do this**:
490591

491592
```bash
492593
# 1. Log in
493-
make ssh
594+
make vm-ssh
494595
# 2. Run command (this might fail or output won't be seen)
495596
df -H
496597
```
@@ -569,7 +670,7 @@ This ensures that the command is executed and its output is returned to the prim
569670
**Pre-commit Testing Requirement**: ALWAYS run the CI test suite before committing any changes:
570671

571672
```bash
572-
make test-ci
673+
make infra-test-ci
573674
```
574675

575676
This command runs all unit tests that don't require a virtual machine, including:
@@ -581,7 +682,7 @@ This command runs all unit tests that don't require a virtual machine, including
581682
582683
Only commit if all CI tests pass. If any tests fail, fix the issues before committing.
583684
584-
**Note**: End-to-end tests (`make test`) are excluded from pre-commit requirements due to their longer execution time (~5-8 minutes), but running them before pushing is strongly recommended for comprehensive validation.
685+
**Note**: End-to-end tests (`make test-e2e`) are excluded from pre-commit requirements due to their longer execution time (~5-8 minutes), but running them before pushing is strongly recommended for comprehensive validation.
585686
586687
**Best Practice**: Always ask "Would you like me to commit these changes?" before performing any git state-changing operations.
587688

0 commit comments

Comments
 (0)