-
Notifications
You must be signed in to change notification settings - Fork 7k
Remove old py versions #440
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
38 commits
Select commit
Hold shift + click to select a range
ab82cbe
Removed old Python versions
cdorsman eae9c78
Removed 3.10 from tox and upgraded requirements-dev.txt becasue of hi…
cdorsman 41c9cb2
3.13 changed to 3.12
cdorsman 3ffb3cb
Adjusted lint_python workflow
cdorsman 68fdae6
Added continue-on-error: true. So that if the workflow stop comes in…
cdorsman 3115935
Added workflow to check per PR
cdorsman 8b94006
Moved workflow
cdorsman 928fdcf
Changed name workflow
cdorsman a48d192
Changed job name
cdorsman b14815c
Added approval for non-Python files and removed continue-on-error
cdorsman ce40583
Optimzed lint_pr.yml
cdorsman cbfd21b
Added fix for PyTest
cdorsman ed9d604
Let pytest only test on changed python design patterns
cdorsman 2acd613
Optimized Tox
cdorsman a7d2745
Allow tox execute it's checks
cdorsman c509128
Tox optimization 2
cdorsman 9c75596
Optimized check
cdorsman e3fa2e0
Ignore setup.py from linting unless it is changes
cdorsman 4f16b18
Fixed bug
cdorsman c905677
Testing a idea
cdorsman b3be9b2
Revert idea
cdorsman 4c78f08
added __init__.py to tests/ for tox
cdorsman 4b32f1b
Let tox only test on Python files that are in the PR.
cdorsman d777606
Adjusted .coveragerc
cdorsman bdb6c0a
added usedevelop = true to tox.ini
cdorsman b9a9c02
Change cov from patterns to main
cdorsman 09b2e42
Rewrote check.
cdorsman 29a56b4
retry fixing coverage
cdorsman 5a30b1c
Change cov to main
cdorsman 75267d8
Added coverage run to execute pytest
cdorsman 9153332
changed cov to patterns
cdorsman 9166124
created pyproject.toml and moved old config to backup folder
cdorsman 5e78e5b
Testing
cdorsman 88b3744
Changed opts to doctest
cdorsman e6a3df8
Fix for error Unknown config option: randomly_seed
cdorsman 0cdee77
Trying fix for No data was collected. (no-data-collected)
cdorsman a84c688
Changed source from patterns to ./
cdorsman 2644109
Changed source from patterns to ./
cdorsman File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,286 @@ | ||
name: lint_pull_request | ||
on: [pull_request, push] | ||
jobs: | ||
check_changes: | ||
runs-on: ubuntu-24.04 | ||
outputs: | ||
has_python_changes: ${{ steps.changed-files.outputs.has_python_changes }} | ||
files: ${{ steps.changed-files.outputs.files }} | ||
steps: | ||
- uses: actions/checkout@v3 | ||
with: | ||
fetch-depth: 0 # To get all history for git diff commands | ||
|
||
- name: Get changed Python files | ||
id: changed-files | ||
run: | | ||
if [ "${{ github.event_name }}" == "pull_request" ]; then | ||
# For PRs, compare against base branch | ||
CHANGED_FILES=$(git diff --name-only --diff-filter=ACMRT origin/${{ github.base_ref }} HEAD | grep '\.py$' | grep -v "^setup\.py$" || echo "") | ||
# Check if setup.py specifically changed | ||
SETUP_PY_CHANGED=$(git diff --name-only --diff-filter=ACMRT origin/${{ github.base_ref }} HEAD | grep "^setup\.py$" || echo "") | ||
if [ ! -z "$SETUP_PY_CHANGED" ]; then | ||
CHANGED_FILES="$CHANGED_FILES $SETUP_PY_CHANGED" | ||
fi | ||
else | ||
# For pushes, use the before/after SHAs | ||
CHANGED_FILES=$(git diff --name-only --diff-filter=ACMRT ${{ github.event.before }} ${{ github.event.after }} | grep '\.py$' | grep -v "^setup\.py$" || echo "") | ||
# Check if setup.py specifically changed | ||
SETUP_PY_CHANGED=$(git diff --name-only --diff-filter=ACMRT ${{ github.event.before }} ${{ github.event.after }} | grep "^setup\.py$" || echo "") | ||
if [ ! -z "$SETUP_PY_CHANGED" ]; then | ||
CHANGED_FILES="$CHANGED_FILES $SETUP_PY_CHANGED" | ||
fi | ||
fi | ||
|
||
# Check if any Python files were changed and set the output accordingly | ||
if [ -z "$CHANGED_FILES" ]; then | ||
echo "No Python files changed" | ||
echo "has_python_changes=false" >> $GITHUB_OUTPUT | ||
echo "files=" >> $GITHUB_OUTPUT | ||
else | ||
echo "Changed Python files: $CHANGED_FILES" | ||
echo "has_python_changes=true" >> $GITHUB_OUTPUT | ||
echo "files=$CHANGED_FILES" >> $GITHUB_OUTPUT | ||
fi | ||
|
||
- name: PR information | ||
if: ${{ github.event_name == 'pull_request' }} | ||
run: | | ||
if [[ "${{ steps.changed-files.outputs.has_python_changes }}" == "true" ]]; then | ||
echo "This PR contains Python changes that will be linted." | ||
else | ||
echo "This PR contains no Python changes, but still requires manual approval." | ||
fi | ||
|
||
lint: | ||
needs: check_changes | ||
if: ${{ needs.check_changes.outputs.has_python_changes == 'true' }} | ||
runs-on: ubuntu-24.04 | ||
strategy: | ||
fail-fast: false | ||
matrix: | ||
tool: [flake8, format, mypy, pytest, pyupgrade, tox] | ||
steps: | ||
# Additional check to ensure we have Python files before proceeding | ||
- name: Verify Python changes | ||
run: | | ||
if [[ "${{ needs.check_changes.outputs.has_python_changes }}" != "true" ]]; then | ||
echo "No Python files were changed. Skipping linting." | ||
exit 0 | ||
fi | ||
|
||
- uses: actions/checkout@v3 | ||
with: | ||
fetch-depth: 0 | ||
|
||
- uses: actions/setup-python@v4 | ||
with: | ||
python-version: 3.12 | ||
|
||
- uses: actions/cache@v3 | ||
with: | ||
path: ~/.cache/pip | ||
key: ${{ runner.os }}-pip-${{ hashFiles('requirements-dev.txt') }} | ||
restore-keys: | | ||
${{ runner.os }}-pip- | ||
|
||
- name: Install dependencies | ||
run: | | ||
python -m pip install --upgrade pip | ||
pip install -r requirements-dev.txt | ||
|
||
# Flake8 linting | ||
- name: Lint with flake8 | ||
if: ${{ matrix.tool == 'flake8' }} | ||
id: flake8 | ||
run: | | ||
echo "Linting files: ${{ needs.check_changes.outputs.files }}" | ||
flake8 ${{ needs.check_changes.outputs.files }} --count --show-source --statistics | ||
|
||
# Format checking with isort and black | ||
- name: Format check | ||
if: ${{ matrix.tool == 'format' }} | ||
id: format | ||
run: | | ||
echo "Checking format with isort for: ${{ needs.check_changes.outputs.files }}" | ||
isort --profile black --check ${{ needs.check_changes.outputs.files }} | ||
echo "Checking format with black for: ${{ needs.check_changes.outputs.files }}" | ||
black --check ${{ needs.check_changes.outputs.files }} | ||
|
||
# Type checking with mypy | ||
- name: Type check with mypy | ||
if: ${{ matrix.tool == 'mypy' }} | ||
id: mypy | ||
run: | | ||
echo "Type checking: ${{ needs.check_changes.outputs.files }}" | ||
mypy --ignore-missing-imports ${{ needs.check_changes.outputs.files }} | ||
|
||
# Run tests with pytest | ||
- name: Run tests with pytest | ||
if: ${{ matrix.tool == 'pytest' }} | ||
id: pytest | ||
run: | | ||
echo "Running pytest discovery..." | ||
python -m pytest --collect-only -v | ||
|
||
# First run any test files that correspond to changed files | ||
echo "Running tests for changed files..." | ||
changed_files="${{ needs.check_changes.outputs.files }}" | ||
|
||
# Extract module paths from changed files | ||
modules=() | ||
for file in $changed_files; do | ||
# Convert file path to module path (remove .py and replace / with .) | ||
if [[ $file == patterns/* ]]; then | ||
module_path=${file%.py} | ||
module_path=${module_path//\//.} | ||
modules+=("$module_path") | ||
fi | ||
done | ||
|
||
# Run tests for each module | ||
for module in "${modules[@]}"; do | ||
echo "Testing module: $module" | ||
python -m pytest -xvs tests/ -k "$module" || true | ||
done | ||
|
||
# Then run doctests on the changed files | ||
echo "Running doctests for changed files..." | ||
for file in $changed_files; do | ||
if [[ $file == *.py ]]; then | ||
echo "Running doctest for $file" | ||
python -m pytest --doctest-modules -v $file || true | ||
fi | ||
done | ||
|
||
# Check Python version compatibility | ||
- name: Check Python version compatibility | ||
if: ${{ matrix.tool == 'pyupgrade' }} | ||
id: pyupgrade | ||
run: pyupgrade --py312-plus ${{ needs.check_changes.outputs.files }} | ||
|
||
# Run tox | ||
- name: Run tox | ||
if: ${{ matrix.tool == 'tox' }} | ||
id: tox | ||
run: | | ||
echo "Running tox integration for changed files..." | ||
changed_files="${{ needs.check_changes.outputs.files }}" | ||
|
||
# Create a temporary tox configuration that extends the original one | ||
echo "[tox]" > tox_pr.ini | ||
echo "envlist = py312" >> tox_pr.ini | ||
echo "skip_missing_interpreters = true" >> tox_pr.ini | ||
|
||
echo "[testenv]" >> tox_pr.ini | ||
echo "setenv =" >> tox_pr.ini | ||
echo " COVERAGE_FILE = .coverage.{envname}" >> tox_pr.ini | ||
echo "deps =" >> tox_pr.ini | ||
echo " -r requirements-dev.txt" >> tox_pr.ini | ||
echo "allowlist_externals =" >> tox_pr.ini | ||
echo " pytest" >> tox_pr.ini | ||
echo " coverage" >> tox_pr.ini | ||
echo " python" >> tox_pr.ini | ||
echo "commands =" >> tox_pr.ini | ||
|
||
# Check if we have any implementation files that changed | ||
pattern_files=0 | ||
test_files=0 | ||
|
||
for file in $changed_files; do | ||
if [[ $file == patterns/* ]]; then | ||
pattern_files=1 | ||
elif [[ $file == tests/* ]]; then | ||
test_files=1 | ||
fi | ||
done | ||
|
||
# Only run targeted tests, no baseline | ||
echo " # Run specific tests for changed files" >> tox_pr.ini | ||
|
||
has_tests=false | ||
|
||
# Add coverage-focused test commands | ||
for file in $changed_files; do | ||
if [[ $file == *.py ]]; then | ||
# Run coverage tests for implementation files | ||
if [[ $file == patterns/* ]]; then | ||
module_name=$(basename $file .py) | ||
|
||
# Get the pattern type (behavioral, structural, etc.) | ||
if [[ $file == patterns/behavioral/* ]]; then | ||
pattern_dir="behavioral" | ||
elif [[ $file == patterns/creational/* ]]; then | ||
pattern_dir="creational" | ||
elif [[ $file == patterns/structural/* ]]; then | ||
pattern_dir="structural" | ||
elif [[ $file == patterns/fundamental/* ]]; then | ||
pattern_dir="fundamental" | ||
elif [[ $file == patterns/other/* ]]; then | ||
pattern_dir="other" | ||
else | ||
pattern_dir="" | ||
fi | ||
|
||
echo " # Testing $file" >> tox_pr.ini | ||
|
||
# Check if specific test exists | ||
if [ -n "$pattern_dir" ]; then | ||
test_path="tests/${pattern_dir}/test_${module_name}.py" | ||
echo " if [ -f \"${test_path}\" ]; then echo \"Test file ${test_path} exists: true\" && coverage run -m pytest -xvs --cov=patterns --cov-append ${test_path}; else echo \"Test file ${test_path} exists: false\"; fi" >> tox_pr.ini | ||
|
||
# Also try to find any test that might include this module | ||
echo " coverage run -m pytest -xvs --cov=patterns --cov-append tests/${pattern_dir}/ -k \"${module_name}\" --no-header" >> tox_pr.ini | ||
fi | ||
|
||
# Run doctests for the file | ||
echo " coverage run -m pytest --doctest-modules -v --cov=patterns --cov-append $file" >> tox_pr.ini | ||
|
||
has_tests=true | ||
fi | ||
|
||
# Run test files directly if modified | ||
if [[ $file == tests/* ]]; then | ||
echo " coverage run -m pytest -xvs --cov=patterns --cov-append $file" >> tox_pr.ini | ||
has_tests=true | ||
fi | ||
fi | ||
done | ||
|
||
# If we didn't find any specific tests to run, mention it | ||
if [ "$has_tests" = false ]; then | ||
echo " python -c \"print('No specific tests found for changed files. Consider adding tests.')\"" >> tox_pr.ini | ||
# Add a minimal test to avoid failure, but ensure it generates coverage data | ||
echo " coverage run -m pytest -xvs --cov=patterns --cov-append -k \"not integration\" --no-header" >> tox_pr.ini | ||
fi | ||
|
||
# Add coverage report command | ||
echo " coverage combine" >> tox_pr.ini | ||
echo " coverage report -m" >> tox_pr.ini | ||
|
||
# Run tox with the custom configuration | ||
echo "Running tox with custom PR configuration..." | ||
echo "======================== TOX CONFIG ========================" | ||
cat tox_pr.ini | ||
echo "===========================================================" | ||
tox -c tox_pr.ini | ||
|
||
summary: | ||
needs: [check_changes, lint] | ||
# Run summary in all cases, regardless of whether lint job ran | ||
if: ${{ always() }} | ||
runs-on: ubuntu-24.04 | ||
steps: | ||
- uses: actions/checkout@v3 | ||
|
||
- name: Summarize results | ||
run: | | ||
echo "## Pull Request Lint Results" >> $GITHUB_STEP_SUMMARY | ||
if [[ "${{ needs.check_changes.outputs.has_python_changes }}" == "true" ]]; then | ||
echo "Linting has completed for all Python files changed in this PR." >> $GITHUB_STEP_SUMMARY | ||
echo "See individual job logs for detailed results." >> $GITHUB_STEP_SUMMARY | ||
else | ||
echo "No Python files were changed in this PR. Linting was skipped." >> $GITHUB_STEP_SUMMARY | ||
fi | ||
echo "" >> $GITHUB_STEP_SUMMARY | ||
echo "⚠️ **Note:** This PR still requires manual approval regardless of linting results." >> $GITHUB_STEP_SUMMARY |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
[run] | ||
branch = True | ||
|
||
[report] | ||
; Regexes for lines to exclude from consideration | ||
exclude_also = | ||
; Don't complain about missing debug-only code: | ||
def __repr__ | ||
if self\.debug | ||
|
||
; Don't complain if tests don't hit defensive assertion code: | ||
raise AssertionError | ||
raise NotImplementedError | ||
|
||
; Don't complain if non-runnable code isn't run: | ||
if 0: | ||
if __name__ == .__main__.: | ||
|
||
; Don't complain about abstract methods, they aren't run: | ||
@(abc\.)?abstractmethod | ||
|
||
ignore_errors = True | ||
|
||
[html] | ||
directory = coverage_html_report |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't believe I never thought to try this approach before. love this.