PTV-1904 add github actions

[briehl]

Add github action for the simple sdkless unit test suite.

We can't do the full integration tests because that needs a whole lot more computer (and time) than we get here.

[briehl]

Claude review:

PR Review: PTV-1904 add github actions

Overview

This PR adds GitHub Actions CI/CD automation to run the sdkless unit test suite on pull requests. This provides automated testing feedback without requiring the full integration test infrastructure.

Changes Summary

Files Added:

• .github/workflows/sdkless_test.yml - GitHub Actions workflow configuration

No files deleted or modified

Detailed Review

Positive Aspects:

1. ✅ Automation for Quick Feedback - Runs lightweight unit tests on every PR

   • Triggers on PR open, synchronize, and ready_for_review events
   • Provides fast feedback loop (no need for full integration tests)
   • Helps catch regressions early

2. ✅ Appropriate Scope - Clear rationale:

   • Full integration tests require significant compute resources and time
   • This workflow tests the sdkless unit suite which is fast and focused
   • Good balance between coverage and resource usage

3. ✅ Standard Actions - Uses stable, widely-adopted GitHub Actions:

   • actions/checkout@v4 - Latest stable version
   • actions/setup-python@v4 - Latest stable version

4. ✅ Clear Naming - Workflow name "SDK-less unit tests" matches purpose

5. ✅ Comprehensive Dependencies - Installs all necessary packages:

   • Testing: pytest, pytest-cov
   • Type checking: mypy
   • Linting: flake8
   • Code coverage: coverall
   • Project dependencies: pandas, requests, jsonrpcbase
   • Python version 3.10 (good choice for modern Python)

Observations & Questions

1. Python Version - Uses Python 3.10:

   • Is this the minimum supported version?
   • Should this test against multiple Python versions (3.10, 3.11, 3.12)?
   • Consider adding a matrix strategy if broader compatibility is needed

2. Pipenv Usage - The workflow installs packages via pip, then runs:

   run: pipenv run make test-sdkless

Question: Does pipenv run work without a lock file being created?
Concern: Installing via pip AND pipenv might cause dependency version conflicts
Suggestion: Either use pip install -r requirements.txt OR properly initialize pipenv with pipenv install --dev
Makefile Assumption - Assumes make test-sdkless target exists:
Verify that the Makefile has this target defined
What does this target do specifically?
No Caching - Workflow doesn't cache Python dependencies:
Could add actions/cache@v3 for pip cache to speed up runs
Example:

• uses: actions/cache@v3
  with:
  path: ~/.cache/pip
  key: ${{ runner.os }}-pip-${{ hashFiles('**/requirements.txt') }}
  No Coverage Reports - pytest-cov is installed but unclear if coverage reports are generated:
  Is coverage being tracked or reported?
  Could integrate with Codecov or similar for tracking
  Flake8 in Dependencies - Listed but unclear if it's run:
  Is make test-sdkless using it?
  Or was it added for potential future use?
  Potential Concerns
  Pipenv/Pip Mismatch - Using both pip and pipenv could cause issues. The workflow should either:
  Use pipenv install --dev --deploy to use lockfile (requires Pipfile.lock in git)
  Or use standard pip with requirements.txt/requirements-dev.txt
  The current approach of pip install followed by pipenv run is unconventional
  No Error Handling - If any step fails, it will stop, which is correct, but ensure Makefile targets properly exit with error codes.
  Hard-coded Package List - If dependencies change, the workflow needs manual updates. Consider using a requirements file instead:
• name: Install pip packages
  run: pip install -r requirements-dev.txt
  Recommendations
  Verify Pipenv Setup - Confirm that Pipfile and Pipfile.lock exist and are properly configured. If not, simplify to use standard pip with requirements files.
  Add Caching - Speed up builds with pip caching (add 10-30 seconds to small builds, saves significant time on large projects).
  Optional: Python Matrix - If multi-version support is important:
  strategy:
  matrix:
  python-version: ['3.10', '3.11', '3.12']
  Document Coverage - If coverage tracking is desired, integrate with a coverage service or add a report step.
  Verdict
  ✅ APPROVED - This is a solid first step toward CI/CD automation. The workflow is straightforward and will provide valuable feedback on pull requests. The scope is appropriate given the resource constraints mentioned. Minor concerns:
  Verify that the pipenv + pip combination works as intended
  Consider dependency management improvements (requirements.txt vs inline pip install)
  Optional: Add caching and multi-version Python testing for future robustness
  The workflow will provide meaningful CI feedback and is ready to merge with the recommendation to verify the Pipenv setup works as expected.

[briehl]

Added caching and removed pipenv - that was just used locally to manage environment. Note also issue #98 for adding a Python package manager. Also issue #103 for adding codecov.

[briehl]

After some reworking - added a requirements-gha.txt file to use as a cache point, and removed the use of pipenv in the gha run - here's an update Claude review:

PR Review (Final): PTV-1904 add github actions

Overview

PR 96 has been updated with the latest improvements. The workflow now includes proper dependency caching with correct cache key reference and uses a requirements file for clean dependency management.

Changes Summary

Files Modified:

• .github/workflows/sdkless_test.yml - GitHub Actions workflow for sdkless unit tests

Final Configuration:

• Pip caching with correct cache key
• Standard pip installation using requirements file
• Direct Makefile target invocation

Detailed Review of Final Implementation

Strengths:

1. ✅ Correct Pip Caching:

   - name: Cache pip install
     uses: actions/cache@v3
     with:
       path: ~/.cache/pip
       key: ${{ runner.os }}-pip-${{ hashFiles('**/requirements-gha.txt') }}

Cache key correctly references requirements-gha.txt
Cache will be properly invalidated when dependencies change
Will significantly speed up subsequent runs
✅ Clean Requirements-Based Installation:
run: pip install -r requirements-gha.txt
Uses a dedicated requirements file for GitHub Actions
Much cleaner than inline package list
Easier to maintain and track dependencies
Single source of truth for CI dependencies
✅ Verified Dependencies Match:
All packages from requirements-gha.txt are properly installed
Includes: flake8, jsonrpcbase, mypy, pandas, pytest, pytest-cov, requests
Matches exactly what the Makefile test target needs
✅ Clean Makefile Integration:
Workflow delegates testing logic to make test-sdkless
Makefile handles mypy and pytest configuration
Proper separation of concerns
✅ Appropriate Trigger Events:
Runs on PR open, synchronize, and ready_for_review
Good balance of feedback frequency
✅ Verified Test Target:
test-sdkless target exists in Makefile (line 74)
Runs mypy type checking and pytest with coverage
Properly excludes integration tests
Technical Details
Workflow Steps:
Checkout code (uses actions/checkout@v4)
Setup Python 3.10 (uses actions/setup-python@v4)
Setup pip cache (uses actions/cache@v3)
Install dependencies from requirements-gha.txt
Run sdkless tests via Makefile target
Cache Strategy:
OS-based key for platform-specific cache separation
Hash-based invalidation when requirements-gha.txt changes
Standard ~/.cache/pip location for pip packages
Verdict
✅ APPROVED - This is an excellent, production-ready GitHub Actions workflow. All previous concerns have been properly addressed. Strengths:
Correct cache key implementation
Clean dependency management via requirements file
Proper test scope (sdkless only, excluding integration tests)
Good workflow structure and trigger events
Well-integrated with existing Makefile
No issues detected. The PR is ready to merge with confidence.