Skip to content

KNOX-3335: Add pylint pre-push linting for .github/workflows/tests Python integration tests#1260

Merged
smolnar82 merged 2 commits into
apache:masterfrom
Raghav-Mah3shwari:KNOX-3335_2
Jun 12, 2026
Merged

KNOX-3335: Add pylint pre-push linting for .github/workflows/tests Python integration tests#1260
smolnar82 merged 2 commits into
apache:masterfrom
Raghav-Mah3shwari:KNOX-3335_2

Conversation

@Raghav-Mah3shwari

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?
This PR adds pylint to the Python integration test pipeline so lint checks run before pytest in the Docker Compose tests container. If pylint fails, integration tests do not run.

Changes include:

Added pylint==4.0.5 to .github/workflows/tests/requirements.txt
Updated .github/workflows/compose/docker-compose.yml to:
Run pylint *.py after installing dependencies and before waiting for Knox / running pytest
Upgrade the tests container from python:3.9-slim to python:3.10-slim (required by pylint 4.x)
Fixed existing pylint violations across the integration test suite so the default pylint rules pass (10.00/10), including:
Module, class, and function docstrings
Line length, trailing whitespace, and trailing newline issues
Replaced broad except Exception with requests.exceptions.RequestException in test_health.py
Removed unnecessary f-strings and wrapped long lines in common_utils.py and test files
How was this patch tested?
Automated linting (local venv):

cd .github/workflows/tests
python3 -m venv .venv && source .venv/bin/activate
pip install -r requirements.txt
pylint *.py
Result: 10.00/10

Docker Compose integration test workflow (local):

Build Knox runtime image (requires Maven artifacts in target/)

docker compose -f ./.github/workflows/compose/docker-compose.yml build knox-dev

Start Knox stack and run tests (pylint runs first inside the tests container)

docker compose -f ./.github/workflows/compose/docker-compose.yml up -d ldap knox
docker compose -f ./.github/workflows/compose/docker-compose.yml up --exit-code-from tests tests
Verified in the tests container output:

Dependencies install successfully
pylint *.py completes with score 10.00/10
Knox wait completes
pytest runs afterward
CI: The existing Apache Knox Docker Compose Tests GitHub Actions workflow (.github/workflows/tests.yml) will pick up the docker-compose change automatically on PRs.

Integration Tests
No new integration test cases were added. This change improves quality gates for the existing suite under .github/workflows/tests/.

Existing integration tests were updated only to satisfy pylint (docstrings, formatting, minor exception handling). Test behavior and assertions are unchanged.

UI changes
N/A — no UI changes in this patch.

@github-actions

Copy link
Copy Markdown

Test Results

22 tests   22 ✅  2s ⏱️
 1 suites   0 💤
 1 files     0 ❌

Results for commit 08dc31d.

@smolnar82

smolnar82 commented Jun 12, 2026

Copy link
Copy Markdown
Contributor
image

At least we know pylint was triggered :)

Let's wait for @moresandeep 's feedback and merge this change soon.

Thanks, @Raghav-Mah3shwari , for your contribution! This is a cool addition to our Docker-based integration tests.

@Raghav-Mah3shwari

Copy link
Copy Markdown
Contributor Author

sure @smolnar82 , thanks for the feedback

@smolnar82 smolnar82 requested a review from moresandeep June 12, 2026 14:19
@smolnar82 smolnar82 added integrationTest python Pull requests that update python code labels Jun 12, 2026

@moresandeep moresandeep left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good!

@smolnar82 smolnar82 merged commit 10eedcc into apache:master Jun 12, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

integrationTest python Pull requests that update python code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants