Skip to content

Conversation

@quinntaylormitchell
Copy link
Collaborator

@quinntaylormitchell quinntaylormitchell commented Nov 10, 2025

Description

As described in title.

Checklist

  • The PR satisfies the contribution guidelines.
  • This is a breaking change and that has been indicated in the PR title, OR this isn't a
    breaking change.
  • Necessary docs have been updated, OR no docs need to be updated.

Validation performed

Ran uv run pytest; tests run successfully.

Summary by CodeRabbit

  • Documentation

    • Updated integration testing guide to show simplified pytest command usage in examples, making instructions clearer and easier to follow.
  • Chores

    • Adjusted integration test task configurations to invoke pytest directly, streamlining task commands and test execution for CI and local runs.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 10, 2025

Walkthrough

Removed the python -m prefix from pytest invocations in documentation and CI/task configuration examples, changing python -m pytest ... to pytest ... without altering any other arguments or behavior.

Changes

Cohort / File(s) Change Summary
Documentation
docs/src/dev-docs/testing/integration-tests.md
Updated examples to remove python -m from pytest commands (e.g., uv run python -m pytest --markersuv run pytest --markers; uv run python -m pytest -m clp_suv run pytest -m clp_s).
Taskfiles / CI tasks
taskfiles/tests/integration.yaml
Replaced invocations of uv run python -m pytest ... with uv run pytest ... for integration tasks (including -m core and specific test paths); preserved arguments and redirections.

Sequence Diagram(s)

(omitted — changes are simple command text substitutions and do not alter control flow)

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

  • Simple, homogeneous text edits limited to command invocations.
  • Quick grep recommended to confirm no remaining python -m pytest occurrences.

Possibly related issues

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: simplifying pytest command invocation by removing 'python -m' prefix across integration tests.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e41b184 and caae063.

📒 Files selected for processing (1)
  • taskfiles/tests/integration.yaml (1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: Bill-hbrhbr
Repo: y-scope/clp PR: 1100
File: integration-tests/tests/fixtures/integration_test_logs.py:54-56
Timestamp: 2025-08-17T16:10:38.722Z
Learning: For PR #1100 (feat(integration-tests): Add CLP package integration tests boilerplate), do not raise cache weakness problems related to the pytest cache implementation in the integration test logs fixtures.
📚 Learning: 2025-08-16T10:24:29.316Z
Learnt from: Bill-hbrhbr
Repo: y-scope/clp PR: 1100
File: integration-tests/pyproject.toml:11-13
Timestamp: 2025-08-16T10:24:29.316Z
Learning: In the CLP project's integration-tests directory, the integration tests package is not intended to be shipped or distributed - only the tests are leveraged for local testing purposes, so console script entry points should not be included in pyproject.toml.

Applied to files:

  • taskfiles/tests/integration.yaml
📚 Learning: 2025-08-17T16:10:38.722Z
Learnt from: Bill-hbrhbr
Repo: y-scope/clp PR: 1100
File: integration-tests/tests/fixtures/integration_test_logs.py:54-56
Timestamp: 2025-08-17T16:10:38.722Z
Learning: For PR #1100 (feat(integration-tests): Add CLP package integration tests boilerplate), do not raise cache weakness problems related to the pytest cache implementation in the integration test logs fixtures.

Applied to files:

  • taskfiles/tests/integration.yaml
📚 Learning: 2025-07-28T08:33:57.487Z
Learnt from: Bill-hbrhbr
Repo: y-scope/clp PR: 1100
File: integration-tests/tests/test_identity_transformation.py:133-137
Timestamp: 2025-07-28T08:33:57.487Z
Learning: In CLP integration tests, the `run_and_assert` utility function should only be used for commands that test CLP package functionality (like clp/clp-s compression/decompression operations). For processing or helper commands (like jq, sort, etc.), direct `subprocess.run` calls should be used instead, as they serve different purposes and don't need the same error handling patterns.

Applied to files:

  • taskfiles/tests/integration.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (18)
  • GitHub Check: ubuntu-jammy-lint
  • GitHub Check: musllinux_1_2-x86_64-dynamic-linked-bins
  • GitHub Check: ubuntu-jammy-dynamic-linked-bins
  • GitHub Check: package-image
  • GitHub Check: musllinux_1_2-x86_64-static-linked-bins
  • GitHub Check: ubuntu-jammy-static-linked-bins
  • GitHub Check: centos-stream-9-dynamic-linked-bins
  • GitHub Check: centos-stream-9-static-linked-bins
  • GitHub Check: manylinux_2_28-x86_64-dynamic-linked-bins
  • GitHub Check: manylinux_2_28-x86_64-static-linked-bins
  • GitHub Check: lint-check (ubuntu-24.04)
  • GitHub Check: rust-checks (macos-15)
  • GitHub Check: rust-checks (ubuntu-22.04)
  • GitHub Check: rust-checks (ubuntu-24.04)
  • GitHub Check: build-macos (macos-14, false)
  • GitHub Check: build-macos (macos-14, true)
  • GitHub Check: build-macos (macos-15, true)
  • GitHub Check: build-macos (macos-15, false)
🔇 Additional comments (1)
taskfiles/tests/integration.yaml (1)

25-25: Excellent simplification of pytest invocations.

The removal of python -m prefix is a clean refactor that aligns with modern uv run conventions. Both commands are correct and match the PR objective.

Also applies to: 29-29


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Bill-hbrhbr
Bill-hbrhbr previously approved these changes Nov 10, 2025
Copy link
Contributor

@Bill-hbrhbr Bill-hbrhbr left a comment

Choose a reason for hiding this comment

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

Let's wait until #1574 is merged to merge this PR.

Copy link
Member

@kirkrodrigues kirkrodrigues left a comment

Choose a reason for hiding this comment

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

For the PR title, how about:

refactor(integration-tests): Simplify `uv run python -m pytest` to `uv run pytest` (fixes #1571).

Shouldn't be chore because:
https://github.com/commitizen/conventional-commit-types/blob/c3a9be4c73e47f2e8197de775f41d981701407fb/index.json#L41

@quinntaylormitchell quinntaylormitchell changed the title chore(integration-tests): Simplify python -m pytest command to pytest (fixes #1571). refactor(integration-tests): Simplify uv run python -m pytest to uv run pytest (fixes #1571). Nov 11, 2025
@quinntaylormitchell quinntaylormitchell merged commit f50b5dc into y-scope:main Nov 11, 2025
31 checks passed
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.

3 participants