Skip to content

Conversation

@quinntaylormitchell
Copy link
Collaborator

@quinntaylormitchell quinntaylormitchell commented Nov 7, 2025

Description

When local source packages aren’t installed as editable, uv treats them like normal immutable distributions. As a result, even if the source code in the repository changes, uv will keep reinstalling the old cached wheel unless the package version is bumped or the cache is purged. Making the package editable (editable = true) ensures the virtual environment directly references the working directory, so changes take effect immediately without rebuilding or relying on cached wheels. See issue #1573 for more details.

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 task clean; cleans sucessfully.

Summary by CodeRabbit

  • Chores
    • Streamlined test cache management for faster test execution.
    • Enhanced local development environment configuration for improved setup efficiency.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 7, 2025

Walkthrough

These changes modify pytest cache clearing in integration tests and update dependency installation configuration. The taskfile now removes the .pytest_cache directory directly instead of invoking pytest with the --cache-clear flag, and editable mode is enabled for four uv source dependencies.

Changes

Cohort / File(s) Summary
Cache clearing refactor
taskfiles/tests/integration.yaml
Replaced pytest --cache-clear command invocation with direct filesystem deletion of .pytest_cache directory
Editable dependency installs
integration-tests/pyproject.toml
Added editable = true flag to four uv source paths in [tool.uv.sources] section for locally checked-out components

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Straightforward script command replacement with filesystem operation
  • Configuration-only changes with repetitive pattern (adding editable = true to multiple entries)
  • No complex logic or structural modifications required

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 accurately describes both main changes: marking local packages as editable and simplifying the cache-clear task to directly remove the uv project cache.
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 0f72d93 and 8032db4.

⛔ Files ignored due to path filters (1)
  • integration-tests/uv.lock is excluded by !**/*.lock
📒 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.
Learnt from: junhaoliao
Repo: y-scope/clp PR: 0
File: :0-0
Timestamp: 2025-10-22T21:02:31.113Z
Learning: Repository y-scope/clp: Maintain deterministic CI/builds for Rust; add a check to verify Cargo.lock is in sync with Cargo.toml without updating dependencies (non-mutating verification in clp-rust-checks workflow).
Learnt from: Bill-hbrhbr
Repo: y-scope/clp PR: 1156
File: components/core/CMakeLists.txt:772-772
Timestamp: 2025-08-09T04:07:27.083Z
Learning: In the CLP project's CMakeLists.txt, when reviewing changes related to the ${zstd_TARGET} variable usage in test linking, the team is planning a refactoring PR to improve this mechanism. Guards for undefined target variables should be deferred to that separate PR rather than being added in focused dependency migration PRs.
📚 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
📚 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
🔇 Additional comments (1)
taskfiles/tests/integration.yaml (1)

12-15: Cache-clear task simplification is correct and well-scoped.

The change from invoking pytest during cache clearing to directly removing the .pytest_cache directory is a clean simplification that aligns with the PR objective. By avoiding the pytest invocation, this eliminates the test-collection phase that could import out-of-date project libraries, directly addressing the collection-time failures mentioned in the PR.

The command is executed in the correct working directory context, and rm -rf .pytest_cache safely targets the specific cache directory.

Please confirm that the related editable = true changes in integration-tests/pyproject.toml are included in this PR, as the AI summary references these changes but the file is not provided for review. These editable package configurations are complementary to this cache-clear simplification and together should prevent the stale install issues described in issue #1573.


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.

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.

As mentioned let's add the editable flags for an actual fix.

@quinntaylormitchell quinntaylormitchell changed the title fix(task): Change cache-clear integration tests task to avoid code import conflict (fixes #1573). chore(integration-tests): Change cache-clear task to avoid code import conflict (fixes #1573); simplify python -m pytest command to pytest (fixes #1571). . Nov 10, 2025
@quinntaylormitchell quinntaylormitchell changed the title chore(integration-tests): Change cache-clear task to avoid code import conflict (fixes #1573); simplify python -m pytest command to pytest (fixes #1571). . chore(integration-tests): Change cache-clear task to avoid code import conflict (fixes #1573); simplify python -m pytest command to pytest (fixes #1571). 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.

As discusses offline let's address issue #1571 in a separate PR.

@quinntaylormitchell quinntaylormitchell changed the title chore(integration-tests): Change cache-clear task to avoid code import conflict (fixes #1573); simplify python -m pytest command to pytest (fixes #1571). fix(task): Change cache-clear task to avoid code import conflict (fixes #1573). Nov 10, 2025
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.

For title, how about:

fix(integration-tests): Prevent stale uv installs by marking local source packages as editable; simplify `cache-clear` to direct local project cache removal.

@quinntaylormitchell quinntaylormitchell changed the title fix(task): Change cache-clear task to avoid code import conflict (fixes #1573). fix(integration-tests): Prevent stale uv installs by marking local source packages as editable; simplify cache-clear to direct local project cache removal (fixes #1573). Nov 10, 2025
kirkrodrigues
kirkrodrigues previously approved these changes Nov 10, 2025
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:

fix(integration-tests): Prevent stale uv installs by marking local source packages as editable; simplify `cache-clear` task to directly remove uv project cache (fixes #1573).

@quinntaylormitchell quinntaylormitchell changed the title fix(integration-tests): Prevent stale uv installs by marking local source packages as editable; simplify cache-clear to direct local project cache removal (fixes #1573). fix(integration-tests): Prevent stale uv installs by marking local source packages as editable; simplify cache-clear task to directly remove uv project cache (fixes #1573). Nov 10, 2025
@quinntaylormitchell quinntaylormitchell merged commit 027b366 into y-scope:main Nov 10, 2025
30 of 33 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