Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Enable pylint on tests #119279

Merged
merged 2 commits into from
Jun 20, 2024
Merged

Enable pylint on tests #119279

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a1bd100
Enable pylint on tests
epenet Jun 17, 2024
9f20323
Remove jobs==1
epenet Jun 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions .github/workflows/ci.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,51 @@ jobs:
python --version
pylint --ignore-missing-annotations=y homeassistant/components/${{ needs.info.outputs.integrations_glob }}

pylint-tests:
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really need an additional job for it? Can we not extend the pylint job, or is the execution time that bad?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

pylint is one of the longer jobs... about 10 minutes for main and 10 minutes for tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've created #120022 to compare the run times

Copy link
Contributor Author

Choose a reason for hiding this comment

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

2 jobs: 8m 18s + 5m 58s

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With a single job... still running after 19 minutes
https://github.com/home-assistant/core/actions/runs/9598111761/job/26468675380?pr=120022

Copy link
Member

Choose a reason for hiding this comment

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

Does pylint have some kind of cache?

Copy link
Contributor

Choose a reason for hiding this comment

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

I was just wondering but it looks like one job takes a lot longer so we should use two jobs

name: Check pylint on tests
runs-on: ubuntu-22.04
timeout-minutes: 20
if: |
github.event.inputs.mypy-only != 'true'
|| github.event.inputs.pylint-only == 'true'
needs:
- info
- base
steps:
- name: Check out code from GitHub
uses: actions/checkout@v4.1.6
- name: Set up Python ${{ env.DEFAULT_PYTHON }}
id: python
uses: actions/setup-python@v5.1.0
with:
python-version: ${{ env.DEFAULT_PYTHON }}
check-latest: true
- name: Restore full Python ${{ env.DEFAULT_PYTHON }} virtual environment
id: cache-venv
uses: actions/cache/restore@v4.0.2
with:
path: venv
fail-on-cache-miss: true
key: >-
${{ runner.os }}-${{ steps.python.outputs.python-version }}-${{
needs.info.outputs.python_cache_key }}
- name: Register pylint problem matcher
run: |
echo "::add-matcher::.github/workflows/matchers/pylint.json"
- name: Run pylint (fully)
if: needs.info.outputs.test_full_suite == 'true'
run: |
. venv/bin/activate
python --version
pylint --ignore-missing-annotations=y tests
- name: Run pylint (partially)
if: needs.info.outputs.test_full_suite == 'false'
shell: bash
run: |
. venv/bin/activate
python --version
pylint --ignore-missing-annotations=y tests/components/${{ needs.info.outputs.integrations_glob }}

mypy:
name: Check mypy
runs-on: ubuntu-22.04
Expand Down
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ repos:
entry: script/run-in-env.sh pylint -j 0 --ignore-missing-annotations=y
language: script
types_or: [python, pyi]
files: ^homeassistant/.+\.(py|pyi)$
files: ^(homeassistant|tests)/.+\.(py|pyi)$
- id: gen_requirements_all
name: gen_requirements_all
entry: script/run-in-env.sh python3 -m script.gen_requirements_all
Expand Down
3 changes: 0 additions & 3 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,6 @@ include = ["homeassistant*"]

[tool.pylint.MAIN]
py-version = "3.12"
ignore = [
"tests",
]
# Use a conservative default here; 2 should speed up most setups and not hurt
# any too bad. Override on command line as appropriate.
jobs = 2
Expand Down