Skip to content

Commit ea36005

Browse files
josecelanojosecelano
committed
Merge #158: feat: [#121] Skip slow tests for Copilot agent with enhanced messaging
48c3293 docs: [#121] add comprehensive environment variable naming guide (Jose Celano) d73db7e feat: [#121] skip slow tests for Copilot agent with enhanced messaging (Jose Celano) Pull request description: ## Summary This PR addresses issue #121 by implementing a solution to prevent GitHub Copilot coding agent from timing out during pre-commit checks. ## Changes ### 1. Environment Variable Renaming **Changed:** `TORRUST_TD_SKIP_E2E_TESTS` → `TORRUST_TD_SKIP_SLOW_TESTS` **Rationale:** - Action-based naming (`SKIP_SLOW_TESTS`) is clearer than condition-based (`RUNNING_IN_AGENT_ENV`) - More descriptive of what actually happens - Not tied to a specific environment (more flexible) - Could be used by developers locally for faster pre-commit checks ### 2. Expanded Test Skipping **Now skips in fast mode:** - E2E provision and destroy tests (~44s) - E2E configuration tests (~48s) - Code coverage check (~1m 29s) - **Total time saved: ~3 minutes (54.8% reduction)** **Still runs in fast mode:** - cargo machete (~0.08s) - All linters (~19s) - Unit tests - 1529 tests (~1m 16s) - cargo doc (~44s) ### 3. Enhanced Pre-commit Messaging When `TORRUST_TD_SKIP_SLOW_TESTS=true`, the script now displays: ``` ⚠️ Running in fast mode (skipping slow tests) The following tests are SKIPPED to stay within the 5-minute timeout limit: • E2E provision and destroy tests (~44 seconds) • E2E configuration tests (~48 seconds) • Code coverage check (~1 minute 29 seconds) 💡 These tests will run automatically in CI after PR creation. If you want to run them manually before committing, use these commands: cargo run --bin e2e-provision-and-destroy-tests # ~44s cargo run --bin e2e-config-tests # ~48s cargo cov-check # ~1m 29s Fast mode execution time: ~3 minutes 48 seconds ``` **Benefits:** - Clear list of what was skipped - Exact commands to run each test separately (all < 5 min) - AI agents can choose to run them individually if time permits - Informative guidance about CI safety net ### 4. Comprehensive Documentation Added detailed timing tables to `docs/contributing/copilot-agent/pre-commit-config.md`: **Individual Task Timings:** | Task | Time | % of Total | Skipped? | |------|------|-----------|----------| | cargo machete | 0.08s | 0.04% | ❌ No | | All linters | 18.75s | 5.7% | ❌ No | | Unit tests | 1m 16s | 22.9% | ❌ No | | cargo doc | 44s | 13.4% | ❌ No | | E2E provision | 44s | 13.4% | ✅ Yes | | E2E config | 48s | 14.4% | ✅ Yes | | Coverage | 1m 29s | 26.9% | ✅ Yes | **Unit Tests Breakdown:** - Shows detailed breakdown of all 1529 tests across 8 test suites - Explains why unit tests are NOT skipped despite taking 1m 16s ### 5. Documentation Organization - Created `docs/contributing/copilot-agent/` folder - Moved `copilot-agent-firewall.md` → `copilot-agent/firewall.md` - Added `copilot-agent/README.md` index - Added `copilot-agent/pre-commit-config.md` comprehensive setup guide - Updated all cross-references ## Performance Results - **Full mode:** ~5m 30s (exceeds Copilot's ~5-6 min timeout ⚠️) - **Fast mode:** ~3m 48s (31% faster, ~2 min safety margin ✅) - **Unit tests:** 1m 16s (kept in fast mode for quality assurance) ## Testing ✅ Fast mode tested: `TORRUST_TD_SKIP_SLOW_TESTS=true ./scripts/pre-commit.sh` ✅ Full mode tested: `./scripts/pre-commit.sh` ✅ All markdown linting passes ✅ Messaging displays correctly with timing information ## Next Steps for Repository Admins **Important:** Update GitHub environment variable configuration: 1. Go to Settings > Environments > copilot 2. **Change variable name:** `TORRUST_TD_SKIP_E2E_TESTS` → `TORRUST_TD_SKIP_SLOW_TESTS` 3. Keep value as `true` ## CI Safety Net Even though slow tests are skipped in pre-commit for Copilot agent, they still run: - In GitHub Actions workflows on PR creation - In the full CI pipeline before merging This ensures no regressions slip through while keeping Copilot agent functional. ## Related - Closes #121 - Community Discussion: https://github.com/orgs/community/discussions/178998 ACKs for top commit: josecelano: ACK 48c3293 Tree-SHA512: 9ba430ec13812da1d3ef3ff9b57b55252bdbec9638f3784fc48a37a593d70ee2a0d3c19320a32593143d486545c822f02936a11537a3f58f547e398b1c73d880
2 parents d219624 + 48c3293 commit ea36005

9 files changed

Lines changed: 779 additions & 28 deletions

File tree

‎.github/copilot-instructions.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,8 @@ These principles should guide all development decisions, code reviews, and featu
103103

104104
11. **When writing Markdown documentation**: Be aware of GitHub Flavored Markdown's automatic linking behavior. Read [`docs/contributing/github-markdown-pitfalls.md`](../docs/contributing/github-markdown-pitfalls.md) for critical patterns to avoid. **NEVER use hash-number patterns for enumeration or step numbering** - this creates unintended links to GitHub issues/PRs. Use ordered lists or alternative formats instead.
105105

106+
12. **When creating new environment variables**: Read [`docs/contributing/environment-variables-naming.md`](../docs/contributing/environment-variables-naming.md) for comprehensive guidance on naming conventions (condition-based vs action-based), decision frameworks, and best practices. Also review [`docs/decisions/environment-variable-prefix.md`](../docs/decisions/environment-variable-prefix.md) to ensure all project environment variables use the `TORRUST_TD_` prefix for proper namespacing and avoiding conflicts with system or user variables.
107+
106108
## 🧪 Build & Test
107109

108110
- **Setup Dependencies**: `cargo run --bin dependency-installer install` (sets up required development tools)

‎docs/contributing/README.md‎

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,24 +4,25 @@ This guide will help you understand our development practices and contribution w
44

55
## 📋 Quick Reference
66

7-
| Topic | File |
8-
| ------------------------------------ | ------------------------------------------------------------ |
9-
| DDD layer placement (architecture) | [ddd-layer-placement.md](./ddd-layer-placement.md) |
10-
| PR review guide for reviewers | [pr-review-guide.md](./pr-review-guide.md) |
11-
| Creating roadmap issues | [roadmap-issues.md](./roadmap-issues.md) |
12-
| Branching conventions | [branching.md](./branching.md) |
13-
| Commit process and pre-commit checks | [commit-process.md](./commit-process.md) |
14-
| Code quality and linting | [linting.md](./linting.md) |
15-
| Module organization and imports | [module-organization.md](./module-organization.md) |
16-
| Error handling principles | [error-handling.md](./error-handling.md) |
17-
| Working with Tera templates | [templates.md](./templates.md) |
18-
| Debugging techniques | [debugging.md](./debugging.md) |
19-
| Spell checking and dictionaries | [spelling.md](./spelling.md) |
20-
| Known issues and expected behaviors | [known-issues.md](./known-issues.md) |
21-
| Logging best practices | [logging-guide.md](./logging-guide.md) |
22-
| GitHub Markdown pitfalls | [github-markdown-pitfalls.md](./github-markdown-pitfalls.md) |
23-
| GitHub Copilot agent firewall | [copilot-agent-firewall.md](./copilot-agent-firewall.md) |
24-
| Testing conventions and practices | [testing/](./testing/) |
7+
| Topic | File |
8+
| ------------------------------------ | -------------------------------------------------------------------- |
9+
| DDD layer placement (architecture) | [ddd-layer-placement.md](./ddd-layer-placement.md) |
10+
| PR review guide for reviewers | [pr-review-guide.md](./pr-review-guide.md) |
11+
| Creating roadmap issues | [roadmap-issues.md](./roadmap-issues.md) |
12+
| Branching conventions | [branching.md](./branching.md) |
13+
| Commit process and pre-commit checks | [commit-process.md](./commit-process.md) |
14+
| Code quality and linting | [linting.md](./linting.md) |
15+
| Module organization and imports | [module-organization.md](./module-organization.md) |
16+
| Error handling principles | [error-handling.md](./error-handling.md) |
17+
| Working with Tera templates | [templates.md](./templates.md) |
18+
| Environment variables naming | [environment-variables-naming.md](./environment-variables-naming.md) |
19+
| Debugging techniques | [debugging.md](./debugging.md) |
20+
| Spell checking and dictionaries | [spelling.md](./spelling.md) |
21+
| Known issues and expected behaviors | [known-issues.md](./known-issues.md) |
22+
| Logging best practices | [logging-guide.md](./logging-guide.md) |
23+
| GitHub Markdown pitfalls | [github-markdown-pitfalls.md](./github-markdown-pitfalls.md) |
24+
| GitHub Copilot agent configuration | [copilot-agent/](./copilot-agent/) |
25+
| Testing conventions and practices | [testing/](./testing/) |
2526

2627
## 🚀 Getting Started
2728

‎docs/contributing/copilot-agent/README.md‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
# GitHub Copilot Agent Configuration
2+
3+
This directory contains documentation for configuring and working with GitHub Copilot coding agent in the Torrust Tracker Deployer project.
4+
5+
## Documents
6+
7+
### [Firewall Configuration](./firewall.md)
8+
9+
Describes the firewall configuration for GitHub Copilot coding agent, including:
10+
11+
- Custom allowlist domains (e.g., `opentofu.org`)
12+
- Recommended allowlist coverage
13+
- Setup instructions for repository administrators
14+
15+
### [Pre-commit Configuration](./pre-commit-config.md)
16+
17+
Explains how to configure environment variables to skip slow tests during Copilot agent pre-commit checks:
18+
19+
- Why this is needed (timeout issues)
20+
- How to set up `TORRUST_TD_SKIP_SLOW_TESTS` environment variable
21+
- Pre-commit timing breakdown (individual tasks and unit test breakdown)
22+
- What gets skipped (E2E tests and coverage checks)
23+
- Testing and verification instructions
24+
25+
## Related Issues
26+
27+
- [GitHub Issue #121](https://github.com/torrust/torrust-tracker-deployer/issues/121) - Install Git Pre-Commit Hooks for Copilot Agent
28+
- [Community Discussion](https://github.com/orgs/community/discussions/178998) - Copilot timeout during long-running pre-commit checks
29+
30+
## References
31+
32+
- [GitHub Copilot Coding Agent Documentation](https://docs.github.com/en/copilot/using-github-copilot/using-copilot-coding-agent-to-work-on-tasks)
33+
- [Customizing Copilot Agent Environment](https://docs.github.com/en/copilot/how-tos/use-copilot-agents/coding-agent/customize-the-agent-environment)

docs/contributing/copilot-agent-firewall.md renamed to docs/contributing/copilot-agent/firewall.md

File renamed without changes.

‎docs/contributing/copilot-agent/pre-commit-config.md‎

Lines changed: 175 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,175 @@
1+
# Setting up TORRUST_TD_SKIP_SLOW_TESTS for GitHub Copilot Agent
2+
3+
This document explains how to configure the `TORRUST_TD_SKIP_SLOW_TESTS` environment variable for GitHub Copilot coding agent to skip slow tests during pre-commit checks.
4+
5+
## Why This Is Needed
6+
7+
GitHub Copilot coding agent has a hardcoded ~5-6 minute timeout for command execution. Our full pre-commit verification (including E2E tests and coverage checks) takes ~5.5 minutes, causing the agent to timeout and retry infinitely.
8+
9+
**Related Issues:**
10+
11+
- GitHub Issue: #121
12+
- Community Discussion: https://github.com/orgs/community/discussions/178998
13+
14+
## Solution
15+
16+
We use an environment variable (`TORRUST_TD_SKIP_SLOW_TESTS=true`) to skip slow tests (E2E tests and code coverage) when Copilot agent runs pre-commit checks. This keeps checks under the timeout limit while maintaining full verification for local development.
17+
18+
**Note:** The variable name follows action-based naming (describes behavior, not context). For a comprehensive discussion on condition-based vs action-based environment variable naming, see [Environment Variables Naming Guide](../environment-variables-naming.md). All Torrust Tracker Deployer environment variables use the `TORRUST_TD_` prefix as documented in [Environment Variable Prefix ADR](../../decisions/environment-variable-prefix.md).
19+
20+
## Pre-commit Timing Breakdown
21+
22+
Understanding where time is spent helps explain why we skip certain tests for the agent:
23+
24+
### Individual Task Timings
25+
26+
| Task | Time | % of Total | Category | Skipped in Fast Mode? |
27+
| ------------- | ----------- | ---------- | -------- | --------------------- |
28+
| cargo machete | 0.08s | 0.04% | Instant | ❌ No |
29+
| All linters | 18.75s | 5.7% | Fast | ❌ No |
30+
| Unit tests | 1m 16s | 22.9% | Medium | ❌ No |
31+
| cargo doc | 44s | 13.4% | Medium | ❌ No |
32+
| E2E provision | 44s | 13.4% | Medium | ✅ **Yes** |
33+
| E2E config | 48s | 14.4% | Medium | ✅ **Yes** |
34+
| Coverage | 1m 29s | 26.9% | Slowest | ✅ **Yes** |
35+
| **TOTAL** | **~5m 30s** | **100%** | - | - |
36+
37+
**Fast Mode Total: ~3m 48s** (31% time reduction, ~2 minute safety margin below timeout)
38+
39+
### Unit Tests Breakdown (cargo test)
40+
41+
The unit tests (`cargo test`) complete in **1m 16s** and include:
42+
43+
| Test Suite | Time | Tests | Description |
44+
| ---------------------- | ----------- | -------- | ------------------------------------ |
45+
| Unit tests (lib) | 12.24s | 1200 | Core library unit tests |
46+
| e2e_create_command | 13.45s | 4 | End-to-end create command workflow |
47+
| e2e_destroy_command | 0.65s | 4 | End-to-end destroy command workflow |
48+
| file_lock_multiprocess | 6.05s | 8 | Multi-process file locking tests |
49+
| logging_integration | 0.13s | 11 | Logging system integration tests |
50+
| ssh_client_integration | 11.31s | 9 | SSH client integration tests |
51+
| template_integration | 0.01s | 4 | Template rendering integration tests |
52+
| Doc tests | 15.44s | 289 | Documentation example tests |
53+
| **TOTAL** | **~1m 16s** | **1529** | All test suites |
54+
55+
**Key Insight:** Even though unit tests take 1m 16s (22.9% of total), they're **NOT skipped** in fast mode because:
56+
57+
- They validate correctness across the entire codebase
58+
- Many are fast unit tests (12.24s for 1200 tests)
59+
- Integration tests provide critical coverage
60+
- Doc tests ensure documentation examples work
61+
62+
**What We Skip:** E2E tests (1m 32s) and coverage (1m 29s) are skipped because:
63+
64+
- They're the slowest checks (54.8% of total time combined)
65+
- E2E tests run in CI workflows after PR creation
66+
- Coverage runs in CI and provides informational metrics
67+
- Skipping them provides ~3 minute time savings
68+
69+
## How to Configure
70+
71+
### Step 1: Navigate to Repository Settings
72+
73+
1. Go to your GitHub repository: https://github.com/torrust/torrust-tracker-deployer
74+
2. Click **Settings** (requires admin access)
75+
3. In the left sidebar, click **Environments**
76+
77+
### Step 2: Configure the Copilot Environment
78+
79+
1. Click on the `copilot` environment (it should already exist)
80+
2. Scroll down to **Environment variables**
81+
3. Click **Add environment variable**
82+
83+
### Step 3: Add the Variable
84+
85+
1. **Name**: `TORRUST_TD_SKIP_SLOW_TESTS`
86+
2. **Value**: `true`
87+
3. Click **Add variable**
88+
89+
## How It Works
90+
91+
### For Local Development (Full Verification)
92+
93+
When developers run `./scripts/pre-commit.sh` locally:
94+
95+
- `TORRUST_TD_SKIP_SLOW_TESTS` is **not set** (defaults to `false`)
96+
- All checks run, including E2E tests and coverage (~5.5 minutes)
97+
- Full quality verification maintained
98+
99+
### For Copilot Agent (Fast Verification)
100+
101+
When Copilot agent runs the pre-commit hook:
102+
103+
- `TORRUST_TD_SKIP_SLOW_TESTS=true` is injected from the `copilot` environment
104+
- E2E tests and coverage checks are **skipped** (~3.8 minutes total)
105+
- CI workflows will still run all tests after the PR is created
106+
107+
## Verification
108+
109+
To verify the configuration is working:
110+
111+
1. Check that the variable exists in Settings > Environments > copilot
112+
2. Wait for Copilot agent to create a PR
113+
3. Check the session logs - you should see: `⚠️ Running in fast mode (skipping slow tests)`
114+
115+
## What Gets Skipped
116+
117+
When `TORRUST_TD_SKIP_SLOW_TESTS=true`:
118+
119+
**Skipped (saves ~3 minutes):**
120+
121+
- ❌ E2E provision and destroy tests (~44s)
122+
- ❌ E2E configuration tests (~48s)
123+
- ❌ Code coverage check (~1m 29s)
124+
125+
**Still runs (maintains quality):**
126+
127+
- ✅ Cargo machete - unused dependencies (~0.08s)
128+
- ✅ All linters - markdown, YAML, Rust, shellcheck (~19s)
129+
- ✅ Unit tests - 1529 tests across all suites (~1m 16s)
130+
- ✅ Cargo documentation build (~44s)
131+
132+
**Total time: ~3m 48s** (vs ~5m 30s in full mode)
133+
134+
## CI Safety Net
135+
136+
Even though slow tests are skipped in pre-commit for Copilot agent, they still run:
137+
138+
- In GitHub Actions workflows on PR creation
139+
- In the full CI pipeline before merging
140+
141+
This ensures no regressions slip through while keeping Copilot agent functional.
142+
143+
## Running Skipped Tests Manually
144+
145+
The pre-commit script will inform you about skipped tests and provide commands to run them:
146+
147+
**For AI agents:** You can run these commands separately (each completes in < 5 minutes):
148+
149+
```bash
150+
# Run E2E provision tests (~44s)
151+
cargo run --bin e2e-provision-and-destroy-tests
152+
153+
# Run E2E config tests (~48s)
154+
cargo run --bin e2e-config-tests
155+
156+
# Run coverage check (~1m 29s)
157+
cargo cov-check
158+
```
159+
160+
**For developers:** Test the fast mode locally:
161+
162+
```bash
163+
TORRUST_TD_SKIP_SLOW_TESTS=true ./scripts/pre-commit.sh
164+
```
165+
166+
To run the full verification locally (default):
167+
168+
```bash
169+
./scripts/pre-commit.sh
170+
```
171+
172+
## References
173+
174+
- [GitHub Docs: Setting environment variables in Copilot's environment](https://docs.github.com/en/copilot/how-tos/use-copilot-agents/coding-agent/customize-the-agent-environment#setting-environment-variables-in-copilots-environment)
175+
- [Community Discussion: Copilot timeout issue](https://github.com/orgs/community/discussions/178998)

0 commit comments

Comments
 (0)