Skip to content

Fix test target and integration opt-in #618

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

Draft
marcpaterno wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Fix test target and integration opt-in #618

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 15 additions & 1 deletion CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ To maintain high code quality and consistency, we use several automated tools. W
| :--- | :--- | :--- |
| `make format` | Automatically format all code using `black` | Frequently during development |
| `make lint` | Run all linters (`black`, `flake8`, `mypy`, `pylint`) in parallel | Before every commit |
| `make test` | Run fast unit tests in parallel | Regularly during development |
| `make test` | Run unit tests in parallel with coverage reporting | Regularly during development |
| `make unit-tests` | Run all unit tests with 100% per-component coverage check | Before pushing |
| `make test-ci` | Run the full test suite exactly as the CI system does | Final check before pushing |
| `make docs` | Build and verify all documentation (tutorials + API) | When changing tutorials or docstrings |
Expand All @@ -50,6 +50,7 @@ graph TD

pre-commit["make pre-commit"]:::main
test-ci["make test-ci"]:::main
test-all["make test-all"]:::main
docs["make docs"]:::main
unit-tests["make unit-tests"]:::main

Expand All @@ -66,6 +67,12 @@ graph TD
test-ci --> test-integration["make test-integration"]
test-ci --> test-example["make test-example"]

%% Test-all dependencies
test-all --> test["make test\n(+coverage)"]
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

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

Mermaid labels generally don’t interpret escaped \n sequences; they tend to render literally. Since the rest of this file uses <br/> for line breaks in Mermaid node labels, consider replacing \n in this label with <br/> to ensure consistent rendering in the docs.

Suggested change
test-all --> test["make test\n(+coverage)"]
test-all --> test["make test<br/>(+coverage)"]

Copilot uses AI. Check for mistakes.
test-all --> test-slow
test-all --> test-integration
test-all --> test-example

%% Test coverage dependencies
test-all-coverage --> unit-tests-core["unit-tests-core"]
test-all-coverage --> unit-tests-post["unit-tests-post"]
Expand Down Expand Up @@ -148,6 +155,13 @@ graph TD
barrier -.->|then parallel| test-int["test-integration"]:::sequential
barrier -.->|then parallel| test-ex["test-example"]:::sequential

%% Test-all parallelism
test-all["make test-all<br/>(parallel)"]:::parallel
test-all --> test-run["test<br/>(pytest -n auto<br/>+coverage)"]:::pytest
test-all --> test-slow
test-all --> test-int
test-all --> test-ex

%% Unit tests post parallelism
test-all-cov --> unit-post["unit-tests-post<br/>(parallel)"]:::parallel

Expand Down
13 changes: 5 additions & 8 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

SHELL := /bin/bash

.PHONY: help format lint typecheck test test-coverage test-example test-integration test-slow \
.PHONY: help format lint typecheck test test-example test-integration test-slow \
test-all clean clean-docs clean-coverage docs tutorials api-docs docs-build \
lint-black lint-flake8 lint-pylint lint-pylint-firecrown lint-pylint-plugins \
lint-pylint-tests lint-pylint-examples lint-mypy pre-commit install all-checks \
Expand Down Expand Up @@ -71,7 +71,7 @@ help: ## Show common developer targets
@echo "During development:"
@echo " make format - Auto-format code (run frequently)"
@echo " make lint - Check code quality (before commit)"
@echo " make test - Run fast tests (during development)"
@echo " make test - Run tests with coverage (during development)"
@echo ""
@echo "Before committing:"
@echo " make unit-tests - Verify 100% coverage on changed modules"
Expand All @@ -95,7 +95,7 @@ help-all: ## Show this help message
@echo "Common workflows:"
@echo " make format - Format all code with black"
@echo " make lint - Run all linting tools (parallel by default)"
@echo " make test - Run fast tests (parallel by default)"
@echo " make test - Run tests with coverage (parallel by default)"
@echo " make unit-tests - Run all unit tests with 100% coverage check"
@echo " make test-ci - Run the full CI suite (all tests, slow, examples)"
@echo " make docs - Build and verify all documentation (tutorials + API)"
Expand Down Expand Up @@ -162,10 +162,7 @@ typecheck: lint-mypy ## Alias for mypy type checking

##@ Testing

test: ## Run tests in parallel (fast, no --runslow)
$(PYTEST_PARALLEL) $(PYTEST_DURATIONS)

test-coverage: ## Run tests with coverage reporting
test: ## Run tests in parallel with coverage reporting
$(RM) $(COVERAGE_JSON)
$(RM) -r $(HTMLCOV_DIR)
$(PYTEST_PARALLEL) $(PYTEST_DURATIONS) $(PYTEST_COV_FLAGS)
Comment on lines +165 to 168
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

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

In the test target, the coverage JSON filename later echoed to the user is hard-coded as coverage.json, but the report is generated at $(COVERAGE_JSON) (which can include COVERAGE_ID). Please update the echoed filename to use the variable so it matches the actual artifact name.

Copilot uses AI. Check for mistakes.
Expand All @@ -190,7 +187,7 @@ test-example: ## Run example tests only
fi

test-integration: ## Run integration tests only
$(PYTEST) -v -s -m integration tests/integration
$(PYTEST) -v -s --runintegration -m integration tests/integration

test-all: test-slow test-example test-integration test ## Run all tests (slow + example + integration)

Expand Down
14 changes: 14 additions & 0 deletions tests/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ def pytest_addoption(parser):

--runslow: used to run tests marked as slow, which are otherwise not run.
--example: used to run only example tests, which are otherwise not run.
--runintegration: used to run tests marked as integration, which are otherwise not run.
"""
parser.addoption(
"--runslow", action="store_true", default=False, help="run slow tests"
Expand All @@ -100,6 +101,12 @@ def pytest_addoption(parser):
default=False,
help="run example tests",
)
parser.addoption(
"--runintegration",
action="store_true",
default=False,
help="run integration tests",
)


def pytest_configure(config):
Expand All @@ -108,6 +115,10 @@ def pytest_configure(config):
Use the marker `slow` for any test that takes more than a second to run.
Tests marked as `slow` are not run unless the user requests them by specifying
the --runslow flag to the pytest program.

Use the marker `integration` for tests that verify independent units work together.
Tests marked as `integration` are not run unless the user requests them by
specifying the --runintegration flag to the pytest program.
"""
config.addinivalue_line("markers", "slow: mark test as slow to run")

Expand All @@ -121,6 +132,9 @@ def pytest_collection_modifyitems(config, items):
if not config.getoption("--runslow"):
_skip_tests(items, "slow", "need --runslow option to run")

if not config.getoption("--runintegration"):
_skip_tests(items, "integration", "need --runintegration option to run")


def _skip_tests(items, keyword, reason):
"""Helper method to skip tests based on a marker name."""
Expand Down
Loading