Skip to content

feat: Modernize precommit hooks and optimize test performance #5929

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

Merged
franciscojavierarceo merged 40 commits into from
Feb 10, 2026
Merged

feat: Modernize precommit hooks and optimize test performance #5929

Changes from 1 commit
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
e7c9113
feat: Modernize precommit hooks and optimize test performance
franciscojavierarceo Jan 31, 2026
031a978
fix: Run uv commands from root to use pyproject.toml
franciscojavierarceo Jan 31, 2026
a23152e
fix: Use --no-project for mypy to run from sdk/python
franciscojavierarceo Jan 31, 2026
ae52fcc
fix: Simplify precommit config to use make targets
franciscojavierarceo Jan 31, 2026
88cefb3
fix: Use uv run --extra ci for tests to include all deps
franciscojavierarceo Jan 31, 2026
759dd9e
fix: Fix import sorting in snowflake bootstrap.py
franciscojavierarceo Jan 31, 2026
ff4548e
feat: Modernize development workflow with uv integration and CI perfo…
franciscojavierarceo Feb 2, 2026
de69e92
fix: Resolve MyPy type error in MilvusOnlineStoreCreator
franciscojavierarceo Feb 2, 2026
c8bbf87
fix: Ensure feast module is accessible in CI smoke tests
franciscojavierarceo Feb 2, 2026
46ed9d9
Merge branch 'master' into feat/precommit-test-performance-optimization
franciscojavierarceo Feb 2, 2026
df45285
fix: Ensure feast module is accessible in CI smoke tests
franciscojavierarceo Feb 2, 2026
6bc12c2
Apply suggestion from @Copilot
franciscojavierarceo Feb 2, 2026
eb6b346
refactor: Simplify Makefile with consistent uv run usage
franciscojavierarceo Feb 2, 2026
5417ab2
fix: Use uv sync for CI to enable consistent uv run usage
franciscojavierarceo Feb 3, 2026
6f6c736
fix: Use uv run in smoke tests for virtualenv compatibility
franciscojavierarceo Feb 3, 2026
9dae77f
chore: Untrack perf-monitor.py development utility
franciscojavierarceo Feb 3, 2026
63c8f3b
fix: Address review feedback for pytest.ini and Makefile
franciscojavierarceo Feb 3, 2026
2068303
Merge branch 'master' into feat/precommit-test-performance-optimization
franciscojavierarceo Feb 3, 2026
0da7c1d
fix: Configure environment paths for Ray worker compatibility
franciscojavierarceo Feb 3, 2026
8848a45
fix: Install make and fix Python paths in CI
franciscojavierarceo Feb 3, 2026
4904104
fix: Use RUNNER_OS environment variable correctly
franciscojavierarceo Feb 3, 2026
60466b2
fix: Ensure PATH is properly exported in test step
franciscojavierarceo Feb 3, 2026
97cd848
fix: Use dynamic site-packages detection for cross-platform compatibi…
franciscojavierarceo Feb 4, 2026
386c7cf
debug: Add Python 3.11 macOS debugging and compatibility workarounds
franciscojavierarceo Feb 4, 2026
d8b156c
fix: Apply macOS Ray compatibility workarounds to all Python versions
franciscojavierarceo Feb 4, 2026
0b6d274
fix: Make PYTHONPATH additive to support both Ray workers and CLI tests
franciscojavierarceo Feb 4, 2026
c530cf6
fix: Skip ray_transformation doctests to avoid macOS Ray worker timeouts
franciscojavierarceo Feb 5, 2026
dec75eb
chore: Remove feast_profile_demo from git tracking
franciscojavierarceo Feb 5, 2026
f50366b
fix: Skip test_e2e_local on macOS CI due to Ray/uv subprocess issues
franciscojavierarceo Feb 5, 2026
282558a
fix: Skip CLI tests on macOS CI due to Ray/uv subprocess issues
franciscojavierarceo Feb 6, 2026
f8051e1
chore: Remove perf-monitor.py from git tracking
franciscojavierarceo Feb 6, 2026
0e111fc
fix: Use uv pip sync with virtualenv instead of uv sync
franciscojavierarceo Feb 6, 2026
12b4a72
updated
franciscojavierarceo Feb 6, 2026
2d54924
fix: Skip test_cli_chdir on macOS CI and use uv run pytest for REST A…
franciscojavierarceo Feb 9, 2026
c69a5c8
fix: Run uv commands from repo root to use correct virtualenv
franciscojavierarceo Feb 9, 2026
cf72f4e
fix: Handle missing arguments gracefully in mypy-daemon.sh
franciscojavierarceo Feb 9, 2026
ad90593
chore: Revert .gitignore changes
franciscojavierarceo Feb 9, 2026
3e827bc
fix: Restore ruff format --check in lint-python target
franciscojavierarceo Feb 9, 2026
9552822
Merge branch 'master' into feat/precommit-test-performance-optimization
franciscojavierarceo Feb 9, 2026
9c499ad
Apply suggestion from @ntkathole
franciscojavierarceo Feb 9, 2026
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
Prev Previous commit
Next Next commit
refactor: Simplify Makefile with consistent uv run usage 
Replace complex venv detection logic with unified uv run commands:
- format-python: Use uv run ruff from project root
- lint-python: Use uv run for ruff and mypy consistently
- test-python-*: Standardize all test targets with uv run

This eliminates environment-specific conditionals and ensures
consistent behavior across local development and CI environments.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
  • Loading branch information
@franciscojavierarceo @claude
franciscojavierarceo and claude committed Feb 2, 2026
commit eb6b346ba25330a06b4ccc4229910562b2cc282c
49 changes: 21 additions & 28 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,12 @@ protos: compile-protos-python compile-protos-docs ## Compile protobufs for Pytho
build: protos build-docker ## Build protobufs and Docker images

format-python: ## Format Python code
cd $(ROOT_DIR)/sdk/python && ([ -f .venv/bin/ruff ] && .venv/bin/ruff check --fix feast/ tests/ || ruff check --fix feast/ tests/)
cd $(ROOT_DIR)/sdk/python && ([ -f .venv/bin/ruff ] && .venv/bin/ruff format feast/ tests/ || ruff format feast/ tests/)
cd $(ROOT_DIR) && uv run ruff check --fix sdk/python/feast/ sdk/python/tests/
cd $(ROOT_DIR) && uv run ruff format sdk/python/feast/ sdk/python/tests/

lint-python: ## Lint Python code
cd $(ROOT_DIR)/sdk/python && ([ -f .venv/bin/ruff ] && .venv/bin/ruff check feast/ tests/ || ruff check feast/ tests/)
cd $(ROOT_DIR)/sdk/python && ([ -f .venv/bin/mypy ] && .venv/bin/mypy feast || mypy feast)
cd $(ROOT_DIR) && uv run ruff check sdk/python/feast/ sdk/python/tests/
cd $(ROOT_DIR) && uv run sh -c "cd sdk/python && mypy feast"

# New combined target
precommit-check: format-python lint-python ## Run all precommit checks
Expand All @@ -74,7 +74,7 @@ install-precommit: ## Install precommit hooks (runs on commit, not push)

# Manual full type check
mypy-full: ## Full MyPy type checking with all files
cd ${ROOT_DIR}/sdk/python && uv run --no-project mypy feast tests
cd ${ROOT_DIR} && uv run sh -c "cd sdk/python && mypy feast tests"

# Run precommit on all files
precommit-all: ## Run all precommit hooks on all files
Expand Down Expand Up @@ -170,40 +170,33 @@ benchmark-python-local: ## Run integration + benchmark tests for Python (local d
##@ Tests

test-python-unit: ## Run Python unit tests (use pattern=<pattern> to filter tests, e.g., pattern=milvus, pattern=test_online_retrieval.py, pattern=test_online_retrieval.py::test_get_online_features_milvus)
cd $(ROOT_DIR)/sdk/python && ([ -f .venv/bin/python ] && .venv/bin/python -m pytest -n 8 --color=yes $(if $(pattern),-k "$(pattern)") tests || python -m pytest -n 8 --color=yes $(if $(pattern),-k "$(pattern)") tests)
cd $(ROOT_DIR) && uv run python -m pytest -n 8 --color=yes $(if $(pattern),-k "$(pattern)") sdk/python/tests

# Fast unit tests only
test-python-unit-fast: ## Run fast unit tests only (no external dependencies)
cd $(ROOT_DIR)/sdk/python && ([ -f .venv/bin/python ] && .venv/bin/python -m pytest tests/unit -n auto -x --tb=short || python -m pytest tests/unit -n auto -x --tb=short)
cd $(ROOT_DIR) && uv run python -m pytest sdk/python/tests/unit -n auto -x --tb=short

# Changed files only (requires pytest-testmon)
test-python-changed: ## Run tests for changed files only
cd $(ROOT_DIR)/sdk/python && ([ -f .venv/bin/python ] && .venv/bin/python -m pytest --testmon -n 8 --tb=short tests || python -m pytest --testmon -n 8 --tb=short tests)
cd $(ROOT_DIR) && uv run python -m pytest --testmon -n 8 --tb=short sdk/python/tests

# Quick smoke test for PRs
test-python-smoke: ## Quick smoke test for development
cd $(ROOT_DIR)/sdk/python && ([ -f .venv/bin/python ] && .venv/bin/python -m pytest \
tests/unit/test_unit_feature_store.py \
tests/unit/test_repo_operations_validate_feast_project_name.py \
-n 4 --tb=short || python -m pytest \
tests/unit/test_unit_feature_store.py \
tests/unit/test_repo_operations_validate_feast_project_name.py \
-n 4 --tb=short)
cd $(ROOT_DIR) && uv run python -m pytest \
sdk/python/tests/unit/test_unit_feature_store.py \
sdk/python/tests/unit/test_repo_operations_validate_feast_project_name.py \
-n 4 --tb=short
Comment thread
devin-ai-integration[bot] marked this conversation as resolved.

test-python-integration: ## Run Python integration tests (CI)
cd $(ROOT_DIR)/sdk/python && ([ -f .venv/bin/python ] && .venv/bin/python -m pytest --tb=short -v -n 8 --integration --color=yes --durations=10 --timeout=1200 --timeout_method=thread --dist loadgroup \
cd $(ROOT_DIR) && uv run python -m pytest --tb=short -v -n 8 --integration --color=yes --durations=10 --timeout=1200 --timeout_method=thread --dist loadgroup \
-k "(not snowflake or not test_historical_features_main)" \
-m "not rbac_remote_integration_test" \
--log-cli-level=INFO -s \
tests || python -m pytest --tb=short -v -n 8 --integration --color=yes --durations=10 --timeout=1200 --timeout_method=thread --dist loadgroup \
-k "(not snowflake or not test_historical_features_main)" \
-m "not rbac_remote_integration_test" \
--log-cli-level=INFO -s \
tests)
sdk/python/tests

# Integration tests with better parallelization
test-python-integration-parallel: ## Run integration tests with enhanced parallelization
cd $(ROOT_DIR)/sdk/python && .venv/bin/python -m pytest tests/integration \
cd $(ROOT_DIR) && uv run python -m pytest sdk/python/tests/integration \
-n auto --dist loadscope \
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot Feb 9, 2026

Choose a reason for hiding this comment

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

🔴 test-python-integration-parallel target uses 300s timeout instead of 1200s like all other integration targets

The new test-python-integration-parallel Makefile target specifies --timeout=300 (5 minutes), while every other integration test target in the Makefile uses --timeout=1200 (20 minutes). Integration tests are inherently slower and frequently need more than 5 minutes per test. This will cause legitimate integration tests to be killed with timeout errors when run via this target.

Root Cause and Impact

All existing integration test targets consistently use --timeout=1200:

  • test-python-integration at Makefile:157 uses --timeout=1200
  • test-python-integration-local at Makefile:169 uses --timeout=1200
  • test-python-integration-rbac-remote at Makefile:179 uses --timeout=1200

But the new target at Makefile:199-203 uses --timeout=300:

test-python-integration-parallel:
	uv run python -m pytest sdk/python/tests/integration \
		-n auto --dist loadscope \
		--timeout=300 --tb=short -v \
		--integration --color=yes --durations=20

The 300s value appears to have been copied from the new timeout = 300 setting in sdk/python/pytest.ini:21, which is intended as a default for unit tests. Integration tests need the longer 1200s timeout.

Impact: Integration tests running via this target will fail with timeout errors for any test taking over 5 minutes, which is common for integration tests that interact with external services.

Suggested change
-n auto --dist loadscope \
--timeout=1200 --tb=short -v \
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

--timeout=300 --tb=short -v \
--integration --color=yes --durations=20
Comment thread
devin-ai-integration[bot] marked this conversation as resolved.
Comment on lines +200 to +204
Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

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

These targets hardcode .venv/bin/python without the fallback to python used by other test targets. That makes make test-python-integration-parallel fail for developers/CI environments that install deps into a different venv or use --system installs. Consider using the same conditional fallback pattern as test-python-unit/test-python-integration.

Copilot uses AI. Check for mistakes.
Expand All @@ -214,25 +207,25 @@ test-python-integration-local: ## Run Python integration tests (local dev mode)
HADOOP_HOME=$$HOME/hadoop \
CLASSPATH="$$( $$HADOOP_HOME/bin/hadoop classpath --glob ):$$CLASSPATH" \
HADOOP_USER_NAME=root \
cd $(ROOT_DIR)/sdk/python && .venv/bin/python -m pytest --tb=short -v -n 8 --color=yes --integration --durations=10 --timeout=1200 --timeout_method=thread --dist loadgroup \
cd $(ROOT_DIR) && uv run python -m pytest --tb=short -v -n 8 --color=yes --integration --durations=10 --timeout=1200 --timeout_method=thread --dist loadgroup \
-k "not test_lambda_materialization and not test_snowflake_materialization" \
-m "not rbac_remote_integration_test" \
--log-cli-level=INFO -s \
tests
sdk/python/tests

test-python-integration-rbac-remote: ## Run Python remote RBAC integration tests
FEAST_IS_LOCAL_TEST=True \
FEAST_LOCAL_ONLINE_CONTAINER=True \
cd $(ROOT_DIR)/sdk/python && .venv/bin/python -m pytest --tb=short -v -n 8 --color=yes --integration --durations=10 --timeout=1200 --timeout_method=thread --dist loadgroup \
cd $(ROOT_DIR) && uv run python -m pytest --tb=short -v -n 8 --color=yes --integration --durations=10 --timeout=1200 --timeout_method=thread --dist loadgroup \
-k "not test_lambda_materialization and not test_snowflake_materialization" \
-m "rbac_remote_integration_test" \
--log-cli-level=INFO -s \
tests
sdk/python/tests

test-python-integration-container: ## Run Python integration tests using Docker
@(docker info > /dev/null 2>&1 && \
FEAST_LOCAL_ONLINE_CONTAINER=True \
cd $(ROOT_DIR)/sdk/python && .venv/bin/python -m pytest -n 8 --integration tests \
cd $(ROOT_DIR) && uv run python -m pytest -n 8 --integration sdk/python/tests \
) || echo "This script uses Docker, and it isn't running - please start the Docker Daemon and try again!";

test-python-universal-spark: ## Run Python Spark integration tests
Expand Down Expand Up @@ -604,7 +597,7 @@ test-python-universal-couchbase-online: ## Run Python Couchbase online store int
sdk/python/tests

test-python-universal: ## Run all Python integration tests
cd $(ROOT_DIR)/sdk/python && .venv/bin/python -m pytest -n 8 --integration tests
cd $(ROOT_DIR) && uv run python -m pytest -n 8 --integration sdk/python/tests

##@ Java

Expand Down
Loading