Skip to content

Conversation

@hua7450
Copy link
Collaborator

@hua7450 hua7450 commented Aug 29, 2025

🎯 Purpose

Fixes GitHub Actions test failures and aligns PR and push workflows to ensure consistent test execution. If PR tests pass, push tests will also pass - eliminating post-merge surprises.

🔧 Key Changes

1️⃣ Fixed Coverage Module Error

  • Problem: uv run coverage resulted in "command not found" errors
  • Solution: Changed to uv run python -m coverage for proper module execution
  • Files: .github/workflows/pr.yaml, .github/workflows/push.yaml

2️⃣ Unified Test Execution

Both PR and push workflows now run identical test commands:

  • ✅ Non-structural YAML tests (baseline with 2 batches, reform with 1 batch)
  • ✅ Structural YAML tests (contrib)
  • ✅ Python-based tests with pytest
  • ✅ Coverage report generation

3️⃣ Fixed Dependency Installation

  • Changed from uv sync --dev to uv sync --all-extras
  • Added explicit uv pip install coverage pytest to ensure test tools are available
  • Added pytest>=8.3.0 to dev dependencies in pyproject.toml

4️⃣ Python Version Update

  • Upgraded all workflows from Python 3.11 to Python 3.13

📝 Files Modified

  • .github/workflows/pr.yaml - PR workflow test commands
  • .github/workflows/push.yaml - Push workflow test commands
  • pyproject.toml - Added pytest to dev dependencies
  • changelog_entry.yaml - Documented changes

🔍 Command Changes

# Before
uv run coverage run -a --branch ...

# After  
uv run python -m coverage run -a --branch ...

✅ Benefits

  • Consistency: PR and push workflows use identical test commands
  • Reliability: No more "works in PR but fails on merge" scenarios
  • Clarity: Clear dependency management with explicit installations

hua7450 and others added 3 commits August 29, 2025 07:17
Exclude Claude Code configuration files from version control

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
The .claude directory should be a local configuration directory for Claude Code,
not a git submodule. It is now properly ignored via .gitignore.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
@hua7450 hua7450 force-pushed the this-better-works branch from 726676e to f30d0d2 Compare August 29, 2025 11:36
Copy link
Contributor

@MaxGhenis MaxGhenis left a comment

Choose a reason for hiding this comment

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

can you make these common actions their own files and bring them in? modularity will further ensure consistency

@hua7450
Copy link
Collaborator Author

hua7450 commented Aug 29, 2025

can you make these common actions their own files and bring them in? modularity will further ensure consistency

The main issue we have is the coverage check in github action. There are two ways to fix it.
Option 1: Stay with --system (current approach)

  • ✅ Coverage data can be combined
  • ✅ Simpler, proven to work
  • ⚠️ Memory cleanup between batches might be less effective

Option 2: Use venv but fix coverage

  • Would need to ensure all coverage files are written to the same location
  • Would need to persist coverage data between uv run invocations
  • More complex to set up correctly

@MaxGhenis
Copy link
Contributor

I mean reusing workflows across push and pr actions, see https://github.com/PolicyEngine/policyengine-us-data/tree/main/.github/workflows

hua7450 and others added 10 commits August 29, 2025 11:39
The uv sync --dev command wasn't properly installing dev dependencies.
Now using uv sync --all-extras and explicitly installing coverage and pytest.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
Implemented virtual environment approach with coverage fixes:
- Use uv sync --all-extras for dependency installation
- Set COVERAGE_FILE environment variable to ensure consistent data location
- Add --data-file=.coverage flag to all coverage commands
- Configure coverage settings in pyproject.toml for consistency
- Handle coverage combine gracefully in case of no parallel files

This approach maintains better memory isolation through virtual environments
while ensuring coverage data is properly collected and combined.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
The coverage collection wasn't working because test_batched.py spawns
subprocesses that don't inherit coverage tracking. This commit:

- Extracts test and coverage logic into a reusable workflow
- Properly configures coverage collection for each test batch
- Ensures coverage files are combined and reported correctly

The direct coverage test (test_coverage_direct.sh) proves that coverage
collection works when tests are run directly with coverage.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
@hua7450 hua7450 force-pushed the this-better-works branch from 2430640 to 6c0ab6e Compare August 29, 2025 15:40
@hua7450
Copy link
Collaborator Author

hua7450 commented Aug 29, 2025

Superseded by #6474

@hua7450 hua7450 closed this Aug 29, 2025
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