Skip to content

Conversation

@deanq
Copy link
Member

@deanq deanq commented Nov 14, 2025

Problem

flash init was creating inconsistent file structures depending on how the package was installed:

  • In-place init (myapp): Created all files including hidden files (.env, .gitignore, .flashignore)
  • Named init (newapp): Missing hidden files

Root Cause

Two issues:

  1. Missing package data configuration - Template files weren't included in wheel distributions
  2. Unreliable file copying - rglob("*") pattern doesn't reliably find hidden files

The bug was masked during development because editable installs (uv add --editable) bypass packaging and read files directly from source. Only wheel installs exposed the issue.

Solution

1. Core Fix (cc9a910)

Package data configuration:

[tool.setuptools.package-data]
tetra_rp = [
    "cli/utils/skeleton_template/**/*",
    "cli/utils/skeleton_template/**/.*",  # Explicit pattern for hidden files
]

Refactored file copying:

  • Replaced rglob("*") with iterdir() recursive approach
  • Extracted IGNORE_PATTERNS constant and _should_ignore() helper
  • Added explicit type annotations for mypy compliance
  • Handles both text and binary files

2. Test Suite (6957f57)

Added 28 comprehensive unit tests with 90% coverage:

  • Ignore pattern matching (9 tests)
  • File conflict detection (5 tests)
  • Project skeleton creation (10 tests)
  • End-to-end workflows (4 tests)

3. Validation Infrastructure (856af02)

Created automated wheel validation to prevent future packaging issues:

  • scripts/validate-wheel.sh - Validates all 11 template files in wheel
  • make validate-wheel - Local testing command
  • CI/CD integration - Runs on all PRs and pushes

4. Documentation (e2bb20f)

Added TESTING.md explaining:

  • Development vs release testing approaches
  • Why editable installs hide packaging problems
  • How to test with full packaging validation
  • Pre-release checklist

Testing

Local validation:

make validate-wheel

Results:

  • All 11 template files present in wheel ✓
  • All files created by flash init including hidden files ✓
  • 28/28 unit tests passing ✓
  • Code quality checks passing ✓

CI/CD:

  • Wheel validation runs automatically on PR
  • Catches packaging issues before merge

Impact

  • Fixes missing hidden files (.env, .gitignore, .flashignore) in wheel distributions
  • Prevents future packaging issues through automated validation
  • Documents testing best practices for the team

Related

Fixes #AE-1249

…reliable hidden file handling

Root cause: Missing package data configuration and unreliable file copying
caused hidden files (.env, .gitignore, .flashignore) to be missing from
wheel distributions.

Changes:
- Add [tool.setuptools.package-data] with explicit hidden file pattern (**/.*)
  to include all template files in wheel distributions
- Refactor skeleton.py to use iterdir() recursive approach instead of rglob()
  for more reliable hidden file handling
- Extract IGNORE_PATTERNS constant and _should_ignore() helper function
- Add explicit type annotations (List[Path], List[str]) for mypy compliance
- Handle both text and binary files during template copying

The issue was only caught during wheel testing, not editable installs, since
editable installs bypass packaging and read files directly from source.

Fixes: flash init missing hidden files in wheel distributions
…ation

Add 28 unit tests covering:
- Ignore pattern matching (9 tests)
- File conflict detection (5 tests)
- Project skeleton creation (10 tests)
- End-to-end workflows (4 tests)

Key test coverage:
- Hidden file handling (.env, .gitignore, .flashignore)
- Template variable substitution ({{project_name}})
- Force overwrite behavior
- Ignored files not copied (__pycache__, *.pyc, etc.)
- Edge cases (read-only files, missing template dir)

Coverage: 90% of skeleton module

All tests pass and use pytest best practices with clear arrange-act-assert
structure and descriptive test names.
Add validation script that verifies wheel packaging before release:

validate-wheel.sh:
- Builds wheel and verifies all 11 template files are present
- Creates clean test environment and installs wheel
- Tests flash init functionality end-to-end
- Validates all files created including hidden files
- Exits with error if any files missing

Integration:
- Add make validate-wheel target for local testing
- Integrate into CI/CD pipeline (.github/workflows/ci.yml)
- Runs automatically on all PRs and pushes to main

This catches packaging issues that editable installs miss, preventing bugs
like missing template files from reaching production.

Related: #AE-1249
Add comprehensive testing guide that documents:

Quick Reference:
- Fast editable install for development (bypasses packaging)
- Complete wheel install for testing with packaging validation
- Commands for unit tests, quality checks, and wheel validation

Key Insights:
- Editable installs hide packaging problems by reading files directly from source
- Wheel testing required to catch package-data configuration issues
- Comparison table showing which testing methods catch packaging issues

Pre-Release Checklist:
- make test-unit, make quality-check, make validate-wheel
- Test both flash init modes (in-place and named)

Documents the root cause of the skeleton template bug and how to prevent
similar issues in the future through proper wheel testing workflow.
@deanq deanq requested review from Copilot and jhcipar November 14, 2025 20:40
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes a packaging bug where flash init failed to create hidden template files (.env, .gitignore, .flashignore) in wheel distributions. The issue stemmed from missing package data configuration and unreliable file copying logic using rglob("*").

Key Changes:

  • Added explicit package data configuration to include hidden files in wheel distributions
  • Refactored file copying to use recursive iterdir() instead of rglob("*")
  • Implemented comprehensive test suite with 28 unit tests achieving 90% coverage
  • Created automated wheel validation infrastructure to prevent future packaging issues

Reviewed Changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
pyproject.toml Added package-data configuration with explicit patterns for hidden files
src/tetra_rp/cli/utils/skeleton.py Refactored file operations with ignore patterns, type annotations, and recursive directory copying
tests/unit/test_skeleton.py Added comprehensive test suite covering ignore patterns, conflicts, skeleton creation, and hidden file handling
scripts/validate-wheel.sh Created validation script to verify template files in wheel and test flash init output
Makefile Added validate-wheel target for local testing
.github/workflows/ci.yml Integrated wheel validation into CI/CD pipeline
TESTING.md Documented testing approaches and pre-release checklist

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Changes based on PR review comments:

1. Extract nested functions to module level for better testability
   - Split copy_directory_recursive into _copy_directory_recursive and _copy_template_file
   - Pass created_files as explicit parameter instead of closure mutation
   - Improves code organization and makes functions independently testable

2. Make Python version detection more flexible in validate-wheel.sh
   - Replace hardcoded python3.11/3.12 checks with fallback chain
   - Supports python3.9 through python3.13 with graceful fallback
   - Future-proof for upcoming Python versions

3. Improve test monkeypatch approach
   - Replace fragile Path.__truediv__ patch with __file__ patch
   - More targeted and less likely to affect other operations
   - Cleaner and more maintainable test code

4. Fix hardcoded user-specific path in documentation
   - Replace ~/Github/python/tetra-rp with /path/to/tetra-rp
   - Makes documentation applicable to all developers

All tests passing (248/248), quality checks passing.
@deanq deanq requested a review from Copilot November 14, 2025 20:56
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@deanq deanq merged commit c3bf137 into main Nov 14, 2025
7 checks passed
@deanq deanq deleted the deanq/ae-1249-flash-init-incomplete-skeleton-bug branch November 14, 2025 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants