Skip to content

Fix test target and integration opt-in#618

Draft
marcpaterno wants to merge 1 commit intomasterfrom
upgrade-makefile
Draft

Fix test target and integration opt-in#618
marcpaterno wants to merge 1 commit intomasterfrom
upgrade-makefile

Conversation

@marcpaterno
Copy link
Collaborator

@marcpaterno marcpaterno commented Feb 24, 2026

Description

This PR does some improvement of the Makefile to streamline the development workflow.

Add --runintegration pytest option so that tests marked with the integration marker are skipped by default, consistent with the existing --runslow and --example behavior. Update the test-integration Makefile target to pass this flag.

Remove the test-coverage target and merge its coverage- reporting behavior directly into the test target, making coverage collection the default for all test runs.

Update all documentation (CONTRIBUTING.md, Makefile help text, copilot-instructions.md, testing_protocol, and auto_testing instructions) to reflect these changes.

Type of change

  • Refactoring, only of Makefile and related documentation and testing configuration,

Checklist:

The following checklist will make sure that you are following the code style and
guidelines of the project as described in the
contributing page.

  • I have run bash pre-commit-check and fixed any issues
  • I have added tests that prove my fix is effective or that my feature works
  • I have made corresponding changes to the documentation
  • I have 100% test coverage for my changes (please check this after the CI system has verified the coverage)

Add --runintegration pytest option so that tests marked with
the integration marker are skipped by default, consistent
with the existing --runslow and --example behavior. Update
the test-integration Makefile target to pass this flag.

Remove the test-coverage target and merge its coverage-
reporting behavior directly into the test target, making
coverage collection the default for all test runs.

Update all documentation (CONTRIBUTING.md, Makefile help
text, copilot-instructions.md, testing_protocol, and
auto_testing instructions) to reflect these changes.
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refines the developer testing workflow by making integration tests opt-in via a new pytest flag and by making coverage collection the default behavior of make test.

Changes:

  • Add a --runintegration pytest option and skip @pytest.mark.integration tests unless explicitly enabled.
  • Remove the test-coverage Makefile target and fold coverage reporting into the default test target.
  • Update CONTRIBUTING documentation to reflect the updated targets/workflows.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
tests/conftest.py Adds --runintegration and default-skipping behavior for integration-marked tests.
Makefile Makes make test generate coverage by default and updates test-integration to opt in to integration tests.
CONTRIBUTING.md Updates workflow docs/diagrams to include test-all and reflect coverage-default test runs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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.
Comment on lines +165 to 168
test: ## Run tests in parallel with coverage reporting
$(RM) $(COVERAGE_JSON)
$(RM) -r $(HTMLCOV_DIR)
$(PYTEST_PARALLEL) $(PYTEST_DURATIONS) $(PYTEST_COV_FLAGS)
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.
@marcpaterno marcpaterno marked this pull request as draft February 25, 2026 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants