Refactor Makefile and run s3/adls/gcs integration tests in CI

.github/workflows/python-ci.yml

@@ -62,10 +62,12 @@ jobs:
       run: sudo apt-get update && sudo apt-get install -y libkrb5-dev # for kerberos
     - name: Install
       run: make install-dependencies
-    - name: Linters
+    - name: Run linters
       run: make lint
-    - name: Tests
-      run: make test-coverage-unit
+    - name: Run unit tests with coverage
+      run: COVERAGE=1 make test
+    - name: Generate coverage report (85%) # Coverage threshold should only increase over time — never decrease it!
+      run: COVERAGE_FAIL_UNDER=85 make coverage-report
 
   integration-test:
     runs-on: ubuntu-22.04
@@ -79,8 +81,30 @@ jobs:
       run: sudo apt-get update && sudo apt-get install -y libkrb5-dev # for kerberos
     - name: Install
       run: make install
-    - name: Run integration tests
-      run: make test-coverage-integration
+
+    - name: Run integration tests with coverage
+      run: COVERAGE=1 make test-integration
+    - name: Show debug logs
+      if: ${{ failure() }}
+      run: docker compose -f dev/docker-compose.yml logs
+
+    - name: Run s3 integration tests with coverage
+      run: COVERAGE=1 make test-s3
     - name: Show debug logs
       if: ${{ failure() }}
       run: docker compose -f dev/docker-compose.yml logs
+
+    - name: Run adls integration tests with coverage
+      run: COVERAGE=1 make test-adls
+    - name: Show debug logs
+      if: ${{ failure() }}
+      run: docker compose -f dev/docker-compose-azurite.yml logs
+
+    - name: Run gcs integration tests with coverage
+      run: COVERAGE=1 make test-gcs
+    - name: Show debug logs
+      if: ${{ failure() }}
+      run: docker compose -f dev/docker-compose-gcs-server.yml logs
+
+    - name: Generate coverage report (75%) # Coverage threshold should only increase over time — never decrease it!
+      run: COVERAGE_FAIL_UNDER=75 make coverage-report

Makefile

@@ -14,106 +14,143 @@
 # KIND, either express or implied.  See the License for the
 # specific language governing permissions and limitations
 # under the License.
+# ========================
+# Configuration Variables
+# ========================
 
+PYTEST_ARGS ?= -v  # Override with e.g. PYTEST_ARGS="-vv --tb=short"
+COVERAGE ?= 0      # Set COVERAGE=1 to enable coverage: make test COVERAGE=1
+COVERAGE_FAIL_UNDER ?= 85  # Minimum coverage % to pass: make coverage-report COVERAGE_FAIL_UNDER=70
 
-help:  ## Display this help
-	@awk 'BEGIN {FS = ":.*##"; printf "\nUsage:\n  make \033[36m\033[0m\n"} /^[a-zA-Z_-]+:.*?##/ { printf "  \033[36m%-20s\033[0m %s\n", $$1, $$2 } /^##@/ { printf "\n\033[1m%s\033[0m\n", substr($$0, 5) } ' $(MAKEFILE_LIST)
+ifeq ($(COVERAGE),1)
+  TEST_RUNNER = poetry run coverage run --parallel-mode --source=pyiceberg -m
+else
+  TEST_RUNNER = poetry run
+endif
 
 POETRY_VERSION = 2.1.1
-install-poetry:  ## Ensure Poetry is installed and the correct version is being used.
+
+# ============
+# Help Section
+# ============
+
+##@ General
+
+help: ## Display this help message
+	@awk 'BEGIN {FS = ":.*##"; printf "\nUsage:\n  make \033[36m<target>\033[0m\n"} /^[a-zA-Z_-]+:.*?##/ { printf "  \033[36m%-25s\033[0m %s\n", $$1, $$2 } /^##@/ { printf "\n\033[1m%s\033[0m\n", substr($$0, 5) } ' $(MAKEFILE_LIST)
+
+# ==================
+# Installation Tasks
+# ==================
+
+##@ Setup
+
+install-poetry: ## Ensure Poetry is installed at the specified version
 	@if ! command -v poetry &> /dev/null; then \
-		echo "Poetry could not be found. Installing..."; \
+		echo "Poetry not found. Installing..."; \
 		pip install --user poetry==$(POETRY_VERSION); \
 	else \
 		INSTALLED_VERSION=$$(pip show poetry | grep Version | awk '{print $$2}'); \
 		if [ "$$INSTALLED_VERSION" != "$(POETRY_VERSION)" ]; then \
-			echo "Poetry version $$INSTALLED_VERSION does not match required version $(POETRY_VERSION). Updating..."; \
+			echo "Updating Poetry to version $(POETRY_VERSION)..."; \
 			pip install --user --upgrade poetry==$(POETRY_VERSION); \
 		else \
-			echo "Poetry version $$INSTALLED_VERSION is already installed."; \
-		fi \
+			echo "Poetry version $(POETRY_VERSION) already installed."; \
+		fi; \
 	fi
 
-install-dependencies: ## Install dependencies including dev, docs, and all extras
+install-dependencies: ## Install all dependencies including extras
 	poetry install --all-extras
 
-install: | install-poetry install-dependencies
+install: install-poetry install-dependencies ## Install Poetry and dependencies
+
+# ===============
+# Code Validation
+# ===============
+
+##@ Quality
 
 check-license: ## Check license headers
 	./dev/check-license
 
-lint: ## lint
+lint: ## Run code linters via pre-commit
 	poetry run pre-commit run --all-files
 
-test: ## Run all unit tests, can add arguments with PYTEST_ARGS="-vv"
-	poetry run pytest tests/ -m "(unmarked or parametrize) and not integration" ${PYTEST_ARGS}
+# ===============
+# Testing Section
+# ===============
 
-test-s3: # Run tests marked with s3, can add arguments with PYTEST_ARGS="-vv"
-	sh ./dev/run-minio.sh
-	poetry run pytest tests/ -m s3 ${PYTEST_ARGS}
+##@ Testing
 
-test-integration: | test-integration-setup test-integration-exec ## Run all integration tests, can add arguments with PYTEST_ARGS="-vv"
+test: ## Run all unit tests (excluding integration)
+	$(TEST_RUNNER) pytest tests/ -m "(unmarked or parametrize) and not integration" $(PYTEST_ARGS)
 
-test-integration-setup: # Prepare the environment for integration
+test-integration: test-integration-setup test-integration-exec ## Run integration tests
+
+test-integration-setup: ## Start Docker services for integration tests
 	docker compose -f dev/docker-compose-integration.yml kill
 	docker compose -f dev/docker-compose-integration.yml rm -f
 	docker compose -f dev/docker-compose-integration.yml up -d
 	sleep 10
 	docker compose -f dev/docker-compose-integration.yml cp ./dev/provision.py spark-iceberg:/opt/spark/provision.py
 	docker compose -f dev/docker-compose-integration.yml exec -T spark-iceberg ipython ./provision.py
 
-test-integration-exec:  # Execute integration tests, can add arguments with PYTEST_ARGS="-vv"
-	poetry run pytest tests/ -v -m integration ${PYTEST_ARGS}
+test-integration-exec: ## Run integration tests (excluding provision)
+	$(TEST_RUNNER) pytest tests/ -m integration $(PYTEST_ARGS)
 
-test-integration-rebuild:
+test-integration-rebuild: ## Rebuild integration Docker services from scratch
 	docker compose -f dev/docker-compose-integration.yml kill
 	docker compose -f dev/docker-compose-integration.yml rm -f
 	docker compose -f dev/docker-compose-integration.yml build --no-cache
 
-test-adls: ## Run tests marked with adls, can add arguments with PYTEST_ARGS="-vv"
+test-s3: ## Run tests marked with @pytest.mark.s3
+	sh ./dev/run-minio.sh
+	$(TEST_RUNNER) pytest tests/ -m s3 $(PYTEST_ARGS)
+
+test-adls: ## Run tests marked with @pytest.mark.adls
 	sh ./dev/run-azurite.sh
-	poetry run pytest tests/ -m adls ${PYTEST_ARGS}
+	$(TEST_RUNNER) pytest tests/ -m adls $(PYTEST_ARGS)
 
-test-gcs: ## Run tests marked with gcs, can add arguments with PYTEST_ARGS="-vv"
+test-gcs: ## Run tests marked with @pytest.mark.gcs
 	sh ./dev/run-gcs-server.sh
-	poetry run  pytest tests/ -m gcs ${PYTEST_ARGS}
+	$(TEST_RUNNER) pytest tests/ -m gcs $(PYTEST_ARGS)
 
-test-coverage-unit: # Run test with coverage for unit tests, can add arguments with PYTEST_ARGS="-vv"
-	poetry run coverage run --source=pyiceberg/ --data-file=.coverage.unit -m pytest tests/ -v -m "(unmarked or parametrize) and not integration" ${PYTEST_ARGS}
-
-test-coverage-integration: # Run test with coverage for integration tests, can add arguments with PYTEST_ARGS="-vv"
-	docker compose -f dev/docker-compose-integration.yml kill
-	docker compose -f dev/docker-compose-integration.yml rm -f
-	docker compose -f dev/docker-compose-integration.yml up -d
-	sh ./dev/run-azurite.sh
-	sh ./dev/run-gcs-server.sh
-	sleep 10
-	docker compose -f dev/docker-compose-integration.yml cp ./dev/provision.py spark-iceberg:/opt/spark/provision.py
-	docker compose -f dev/docker-compose-integration.yml exec -T spark-iceberg ipython ./provision.py
-	poetry run coverage run --source=pyiceberg/ --data-file=.coverage.integration -m pytest tests/ -v -m integration ${PYTEST_ARGS}
+test-coverage: COVERAGE=1
+test-coverage: test test-integration test-s3 test-adls test-gcs coverage-report ## Run all tests with coverage and report
 
-test-coverage: | test-coverage-unit test-coverage-integration ## Run all tests with coverage including unit and integration tests
-	poetry run coverage combine .coverage.unit .coverage.integration
-	poetry run coverage report -m --fail-under=90
+coverage-report: ## Combine and report coverage
+	poetry run coverage combine
+	poetry run coverage report -m --fail-under=$(COVERAGE_FAIL_UNDER)
 	poetry run coverage html
 	poetry run coverage xml
 
+# ================
+# Documentation
+# ================
 
-clean: ## Clean up the project Python working environment
-	@echo "Cleaning up Cython and Python cached files"
-	@rm -rf build dist *.egg-info
-	@find . -name "*.so" -exec echo Deleting {} \; -delete
-	@find . -name "*.pyc" -exec echo Deleting {} \; -delete
-	@find . -name "__pycache__" -exec echo Deleting {} \; -exec rm -rf {} +
-	@find . -name "*.pyd" -exec echo Deleting {} \; -delete
-	@find . -name "*.pyo" -exec echo Deleting {} \; -delete
-	@echo "Cleanup complete"
+##@ Documentation
 
-docs-install:
+docs-install: ## Install docs dependencies
 	poetry install --with docs
 
-docs-serve:
+docs-serve: ## Serve local docs preview (hot reload)
 	poetry run mkdocs serve -f mkdocs/mkdocs.yml
 
-docs-build:
+docs-build: ## Build the static documentation site
 	poetry run mkdocs build -f mkdocs/mkdocs.yml --strict
+
+# ===================
+# Project Maintenance
+# ===================
+
+##@ Maintenance
+
+clean: ## Remove build artifacts and caches
+	@echo "Cleaning up Cython and Python cached files..."
+	@rm -rf build dist *.egg-info
+	@find . -name "*.so" -exec echo Deleting {} \; -delete
+	@find . -name "*.pyc" -exec echo Deleting {} \; -delete
+	@find . -name "__pycache__" -exec echo Deleting {} \; -exec rm -rf {} +
+	@find . -name "*.pyd" -exec echo Deleting {} \; -delete
+	@find . -name "*.pyo" -exec echo Deleting {} \; -delete
+	@echo "Cleanup complete."