[feature] Support Python projects using pyproject.toml #706#707
[feature] Support Python projects using pyproject.toml #706#707BHARATH0153 wants to merge 4 commits into
Conversation
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
🔥 Files not summarized due to errors (5)
📝 WalkthroughCaution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Overview
Files Reviewed (5 files)
Notes
Previous Review Summaries (2 snapshots, latest commit e2b39ad)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit e2b39ad)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Notes
Files Reviewed (4 files)
Fix these issues in Kilo Cloud Previous reviewStatus: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Notes
Files Reviewed (4 files)
Reviewed by glm-5.2-20260616 · Input: 199.1K · Output: 3.2K · Cached: 49.5K |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@openwisp_utils/releaser/config.py`:
- Around line 97-110: The version parsing in the config builder leaves a partial
state in the try block when parsing fails after setting pyproject fields. In the
config routine that handles version_str, move the package_type and version_path
assignments until after all int(parts[..]) conversions succeed, so a
ValueError/TypeError leaves the config fully unset instead of mixing
CURRENT_VERSION=None with pyproject metadata. Keep the early return behavior for
non-X.Y.Z inputs consistent with the rest of the function.
In `@openwisp_utils/releaser/tests/test_config.py`:
- Around line 429-439: The pyproject.toml package detection test in
test_pyproject_toml_package_detection depends on load_config resolving TOML
parsing through openwisp_utils.releaser.config, which falls back to tomli on
Python 3.10. Fix this by either adding tomli to the test/runtime dependencies
used by the releaser tests or guarding this assertion so it is skipped when
tomli is unavailable on Python 3.10, keeping the test aligned with the
package_type and version_path checks in load_config.
In `@openwisp_utils/releaser/version.py`:
- Around line 67-84: The version bump in version.py can falsely succeed when
tomllib parses a version but the literal text does not match version = "x", so
update the logic in the tomllib branch to verify the replacement actually
changed content before returning. In the path that uses tomllib.loads and
current, make the replacement conditional on a real match; otherwise fall
through to _bump_with_regex so formats like single quotes or different spacing
are handled correctly. Also avoid blindly using str.replace on the whole file,
since it can rewrite unrelated identical version strings elsewhere.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1cf961f5-4dfb-47f0-a82a-87ecf2441e37
📒 Files selected for processing (4)
openwisp_utils/releaser/config.pyopenwisp_utils/releaser/tests/test_config.pyopenwisp_utils/releaser/tests/test_version_bumping.pyopenwisp_utils/releaser/version.py
📜 Review details
⏰ Context from checks skipped due to timeout. (15)
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Kilo Code Review
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{py,html,txt}
📄 CodeRabbit inference engine (Custom checks)
For Django pull requests, ensure all user-facing strings are marked as translatable using the Django i18n framework
Files:
openwisp_utils/releaser/tests/test_config.pyopenwisp_utils/releaser/tests/test_version_bumping.pyopenwisp_utils/releaser/version.pyopenwisp_utils/releaser/config.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Place imports at the top of the file; only defer imports when necessary (e.g., Django model imports inside functions or methods where the app registry is not yet ready)
Avoid unnecessary blank lines inside function and method bodies
Add or update tests for every behavior change
Runopenwisp-qa-formatafter editing
Prefer in-process tests so coverage tools can measure changed code
When checking coverage for a changed module, usepython -m pytest <test_path> --cov=<dotted.module.path> --cov-report=term-missing
Watch for unsafe file paths, unsafe subprocess usage, token or secret exposure, and changes that could weaken QA or release safeguards
Write comments and docstrings only when they explain why code is shaped a certain way; place comments before the relevant code block instead of scattering them inside it
Files:
openwisp_utils/releaser/tests/test_config.pyopenwisp_utils/releaser/tests/test_version_bumping.pyopenwisp_utils/releaser/version.pyopenwisp_utils/releaser/config.py
**/tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
For bug fixes, write the regression test first, run it against the unfixed code, confirm it fails for the expected reason, then implement the fix
Files:
openwisp_utils/releaser/tests/test_config.pyopenwisp_utils/releaser/tests/test_version_bumping.py
🧠 Learnings (1)
📚 Learning: 2026-06-15T22:45:01.529Z
Learnt from: nemesifier
Repo: openwisp/openwisp-utils PR: 701
File: setup.py:72-72
Timestamp: 2026-06-15T22:45:01.529Z
Learning: When reviewing changes that require a minimum Selenium Python version for specific APIs (e.g., BiDi features like `Options.enable_bidi`, console/message handlers like `Script.add_console_message_handler`, or event handlers like `BrowsingContext.add_event_handler`), do not use the Selenium API docs site (`https://www.selenium.dev/selenium/docs/api/py/`) as the source of introduction/version history, since it reflects the latest API shape. Instead, confirm feature availability by checking the official Python changelog (`py/CHANGES` in the Selenium repo) and/or inspecting the Selenium source (tags/commits corresponding to candidate versions). Only accept the stated minimum version after verifying that the referenced APIs exist in that Selenium version (e.g., Selenium 4.32.0 includes the BiDi APIs used by openwisp-utils’ BiDi-related PR `#701` as of that version).
Applied to files:
openwisp_utils/releaser/tests/test_config.pyopenwisp_utils/releaser/tests/test_version_bumping.pyopenwisp_utils/releaser/version.pyopenwisp_utils/releaser/config.py
🪛 ast-grep (0.44.0)
openwisp_utils/releaser/config.py
[warning] 67-67: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(init_py_path, "r")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
🔇 Additional comments (4)
openwisp_utils/releaser/config.py (1)
7-14: LGTM!Also applies to: 33-33, 237-237
openwisp_utils/releaser/version.py (1)
152-152: LGTM!openwisp_utils/releaser/tests/test_version_bumping.py (1)
374-440: LGTM!openwisp_utils/releaser/tests/test_config.py (1)
442-482: LGTM!
|
coverage: 97.746% (+0.1%) from 97.641% — BHARATH0153:master into openwisp:master |
Closes openwisp#706 - Detect pyproject.toml as package type python - Read version from [project] version field - Bump version using exact string replacement + regex fallback - Add tomli dependency for Python < 3.11 - Add tests for malformed TOML, non-numeric version, and priority - Add pragma: no cover to version-specific import branches
|
@pandafy @nemesifier had a coverage drop from the pyproject.toml changes. The cause is the conditional import tomllib block — on Python 3.11+ the except ImportError branch is never executed (tomllib is built-in), on 3.10 the import tomllib line is never reached (it fails and falls through to tomli). can't cover both paths on any single Python version. |
|
@coderabbitai review please |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
cat -n openwisp_utils/releaser/config.py
echo "---"
cat -n openwisp_utils/releaser/version.py
echo "---"
cat -n openwisp_utils/releaser/tests/test_config.py
echo "---"
cat -n openwisp_utils/releaser/tests/test_version_bumping.pyLength of output: 61798
ReviewBug:
|
| Severity | Issue |
|---|---|
| 🔴 Bug | PACKAGE_VERSION_HANDLERS["pyproject"] in config.py is dead/unreachable code |
| 🟠 Issue | Silent fallthrough from setup.py → pyproject.toml when VERSION tuple is absent |
| 🟠 Issue | PEP 440 pre-release versions silently produce None |
| 🟡 Minor | except Exception too broad in _handle_pyproject_toml_version |
| 🟡 Minor | Misleading test name test_bump_version_pyproject_toml_not_found |
| 🟡 Minor | Misleading function name get_package_type_from_setup |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Read the main changed files
echo "=== config.py ==="
cat -n openwisp_utils/releaser/config.py
echo ""
echo "=== version.py ==="
cat -n openwisp_utils/releaser/version.py
echo ""
echo "=== test_config.py ==="
cat -n openwisp_utils/releaser/tests/test_config.py
echo ""
echo "=== test_version_bumping.py ==="
cat -n openwisp_utils/releaser/tests/test_version_bumping.pyLength of output: 62025 🏁 Script executed: #!/bin/bash
echo "=== setup.py or pyproject.toml dependencies ==="
fd -e toml -e cfg -e txt -e py --max-depth 2 -x grep -l "tomli" {}
echo ""
echo "=== setup.cfg or requirements ==="
fd "requirements" --max-depth 2 -x cat {}
echo ""
echo "=== pyproject.toml at root ==="
cat pyproject.toml 2>/dev/null || echo "not found"
echo ""
echo "=== setup.cfg ==="
cat setup.cfg 2>/dev/null || echo "not found"
echo ""
echo "=== conftest.py for fixtures ==="
fd "conftest.py" openwisp_utils/releaser --exec cat {}Length of output: 8544 🏁 Script executed: #!/bin/bash
echo "=== setup.py extras / install_requires ==="
cat setup.py
echo ""
echo "=== grep for tomli in setup.py ==="
grep -n "tomli" setup.py || echo "tomli not found in setup.py"Length of output: 3828
🔴 Correctness: Section-ambiguous version replacement in
|
|
@pandafy please need your perspective check when you have time thanks! |
<h3>Flake8 and Test Failures Detected</h3>Hello @BHARATH0153,
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@openwisp_utils/releaser/config.py`:
- Around line 110-131: Remove the candidate_files scan from
_handle_pyproject_toml_version and keep this method focused only on
pyproject.toml handling. The use of project_name here is invalid in this scope,
so delete the fallback logic that builds package_directory, iterates over
__init__.py/version.py, and parses VERSION from those files. Let
_handle_python_version own any filesystem fallback/version-file scanning
instead, and preserve the pyproject.toml-derived version_path and
CURRENT_VERSION values in this method.
In `@openwisp_utils/releaser/tests/test_version_bumping.py`:
- Around line 466-470: The test in test_version_bumping.py has a stray assertion
after the pytest.raises(RuntimeError) block that references written_content, but
that variable is never defined in this test. Remove the trailing assert from the
test case around bump_version and keep the malformed_content scenario focused on
asserting the RuntimeError from bump_version(config, "1.2.4"), using the
existing mock_open/patch setup to locate the affected test.
In `@openwisp_utils/releaser/version.py`:
- Around line 79-85: The version bump logic can rewrite the wrong version field
because both the regex fallback in _bump_with_regex and the tomllib-based
replace in bump_version only target the first matching occurrence. Update
bump_version/version.py to locate the [project] section explicitly, then replace
only the version entry inside that section, using the existing _bump_with_regex
and bump_version symbols as the place to fix the section-scoped matching.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c8475dc5-765a-40d8-8011-d1eaf70895f2
📒 Files selected for processing (5)
openwisp_utils/releaser/config.pyopenwisp_utils/releaser/tests/test_config.pyopenwisp_utils/releaser/tests/test_version_bumping.pyopenwisp_utils/releaser/version.pysetup.py
📜 Review details
⏰ Context from checks skipped due to timeout. (14)
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.1.0
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{py,html,txt}
📄 CodeRabbit inference engine (Custom checks)
For Django pull requests, ensure all user-facing strings are marked as translatable using the Django i18n framework
Files:
openwisp_utils/releaser/version.pyopenwisp_utils/releaser/tests/test_config.pysetup.pyopenwisp_utils/releaser/tests/test_version_bumping.pyopenwisp_utils/releaser/config.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Place imports at the top of the file; only defer imports when necessary (e.g., Django model imports inside functions or methods where the app registry is not yet ready)
Avoid unnecessary blank lines inside function and method bodies
Add or update tests for every behavior change
Runopenwisp-qa-formatafter editing
Prefer in-process tests so coverage tools can measure changed code
When checking coverage for a changed module, usepython -m pytest <test_path> --cov=<dotted.module.path> --cov-report=term-missing
Watch for unsafe file paths, unsafe subprocess usage, token or secret exposure, and changes that could weaken QA or release safeguards
Write comments and docstrings only when they explain why code is shaped a certain way; place comments before the relevant code block instead of scattering them inside it
Files:
openwisp_utils/releaser/version.pyopenwisp_utils/releaser/tests/test_config.pysetup.pyopenwisp_utils/releaser/tests/test_version_bumping.pyopenwisp_utils/releaser/config.py
**/tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
For bug fixes, write the regression test first, run it against the unfixed code, confirm it fails for the expected reason, then implement the fix
Files:
openwisp_utils/releaser/tests/test_config.pyopenwisp_utils/releaser/tests/test_version_bumping.py
🧠 Learnings (1)
📚 Learning: 2026-06-15T22:45:01.529Z
Learnt from: nemesifier
Repo: openwisp/openwisp-utils PR: 701
File: setup.py:72-72
Timestamp: 2026-06-15T22:45:01.529Z
Learning: When reviewing changes that require a minimum Selenium Python version for specific APIs (e.g., BiDi features like `Options.enable_bidi`, console/message handlers like `Script.add_console_message_handler`, or event handlers like `BrowsingContext.add_event_handler`), do not use the Selenium API docs site (`https://www.selenium.dev/selenium/docs/api/py/`) as the source of introduction/version history, since it reflects the latest API shape. Instead, confirm feature availability by checking the official Python changelog (`py/CHANGES` in the Selenium repo) and/or inspecting the Selenium source (tags/commits corresponding to candidate versions). Only accept the stated minimum version after verifying that the referenced APIs exist in that Selenium version (e.g., Selenium 4.32.0 includes the BiDi APIs used by openwisp-utils’ BiDi-related PR `#701` as of that version).
Applied to files:
openwisp_utils/releaser/version.pyopenwisp_utils/releaser/tests/test_config.pysetup.pyopenwisp_utils/releaser/tests/test_version_bumping.pyopenwisp_utils/releaser/config.py
🪛 ast-grep (0.44.0)
openwisp_utils/releaser/config.py
[warning] 73-73: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(init_py_path, "r")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
[warning] 120-120: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(version_path, "r")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
🔇 Additional comments (2)
openwisp_utils/releaser/tests/test_config.py (1)
592-602: 🩺 Stability & Availability | ⚡ Quick winVerify
tomliis available in the test environment on Python 3.10.
config.pyfalls back totomliwhen stdlibtomllibis unavailable (< 3.11). Withpyproject.tomlmapped to"python", this test will fail on 3.10 unlesstomliis installed in the test deps. Thesetup.pychange adds it underreleaserextras—confirm the CI test job installs that extra (ortomlidirectly).#!/bin/bash rg -nP 'tomli|releaser|3\.10|python-version' tox.ini .github/workflows/ 2>/dev/nullsetup.py (1)
78-78: LGTM!
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@openwisp_utils/releaser/config.py`:
- Around line 110-131: Remove the candidate_files scan from
_handle_pyproject_toml_version and keep this method focused only on
pyproject.toml handling. The use of project_name here is invalid in this scope,
so delete the fallback logic that builds package_directory, iterates over
__init__.py/version.py, and parses VERSION from those files. Let
_handle_python_version own any filesystem fallback/version-file scanning
instead, and preserve the pyproject.toml-derived version_path and
CURRENT_VERSION values in this method.
In `@openwisp_utils/releaser/tests/test_version_bumping.py`:
- Around line 466-470: The test in test_version_bumping.py has a stray assertion
after the pytest.raises(RuntimeError) block that references written_content, but
that variable is never defined in this test. Remove the trailing assert from the
test case around bump_version and keep the malformed_content scenario focused on
asserting the RuntimeError from bump_version(config, "1.2.4"), using the
existing mock_open/patch setup to locate the affected test.
In `@openwisp_utils/releaser/version.py`:
- Around line 79-85: The version bump logic can rewrite the wrong version field
because both the regex fallback in _bump_with_regex and the tomllib-based
replace in bump_version only target the first matching occurrence. Update
bump_version/version.py to locate the [project] section explicitly, then replace
only the version entry inside that section, using the existing _bump_with_regex
and bump_version symbols as the place to fix the section-scoped matching.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c8475dc5-765a-40d8-8011-d1eaf70895f2
📒 Files selected for processing (5)
openwisp_utils/releaser/config.pyopenwisp_utils/releaser/tests/test_config.pyopenwisp_utils/releaser/tests/test_version_bumping.pyopenwisp_utils/releaser/version.pysetup.py
📜 Review details
🔇 Additional comments (2)
openwisp_utils/releaser/tests/test_config.py (1)
592-602: 🩺 Stability & Availability | ⚡ Quick winVerify
tomliis available in the test environment on Python 3.10.
config.pyfalls back totomliwhen stdlibtomllibis unavailable (< 3.11). Withpyproject.tomlmapped to"python", this test will fail on 3.10 unlesstomliis installed in the test deps. Thesetup.pychange adds it underreleaserextras—confirm the CI test job installs that extra (ortomlidirectly).#!/bin/bash rg -nP 'tomli|releaser|3\.10|python-version' tox.ini .github/workflows/ 2>/dev/nullsetup.py (1)
78-78: LGTM!
🛑 Comments failed to post (3)
openwisp_utils/releaser/config.py (1)
110-131: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Confirm project_name is not defined in _handle_pyproject_toml_version scope # and check where version.py fallback is handled. ast-grep outline openwisp_utils/releaser/config.py rg -nP '\bproject_name\b' openwisp_utils/releaser/config.py rg -nP 'version\.py' openwisp_utils/releaser/config.pyRepository: openwisp/openwisp-utils
Length of output: 1058
🏁 Script executed:
#!/bin/bash set -euo pipefail sed -n '63,140p' openwisp_utils/releaser/config.py | cat -n printf '\n---\n' rg -n "pyproject|version.py|_handle_python_version|_handle_pyproject_toml_version" -S tests openwisp_utils | head -n 200Repository: openwisp/openwisp-utils
Length of output: 13346
Remove the
candidate_filesscan from_handle_pyproject_toml_version.project_nameis out of scope here, so any validpyproject.tomlversion will raiseNameError. Even if that were fixed, rescanning__init__.py/version.pyhere would overwrite thepyproject.tomlresult; that fallback belongs in_handle_python_version.🧰 Tools
🪛 ast-grep (0.44.0)
[warning] 120-120: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(version_path, "r")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').(open-filename-from-request)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@openwisp_utils/releaser/config.py` around lines 110 - 131, Remove the candidate_files scan from _handle_pyproject_toml_version and keep this method focused only on pyproject.toml handling. The use of project_name here is invalid in this scope, so delete the fallback logic that builds package_directory, iterates over __init__.py/version.py, and parses VERSION from those files. Let _handle_python_version own any filesystem fallback/version-file scanning instead, and preserve the pyproject.toml-derived version_path and CURRENT_VERSION values in this method.openwisp_utils/releaser/tests/test_version_bumping.py (1)
466-470: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
written_contentis undefined — this assertion raisesNameError.Line 470 runs after the
with pytest.raises(...)block and referenceswritten_content, which is never assigned in this test. It also asserts the generic"1.2.4\n"format, which is irrelevant to a malformed-pyproject case. The test's intent (expectRuntimeError) is already satisfied inside thewithblock, so this trailing line is stray and should be removed.🐛 Proposed fix
m_open = mock_open(read_data=malformed_content) with patch("os.path.exists", return_value=True), patch("builtins.open", m_open): with pytest.raises(RuntimeError, match="Failed to find"): bump_version(config, "1.2.4") - assert written_content == "1.2.4\n"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.m_open = mock_open(read_data=malformed_content) with patch("os.path.exists", return_value=True), patch("builtins.open", m_open): with pytest.raises(RuntimeError, match="Failed to find"): bump_version(config, "1.2.4")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@openwisp_utils/releaser/tests/test_version_bumping.py` around lines 466 - 470, The test in test_version_bumping.py has a stray assertion after the pytest.raises(RuntimeError) block that references written_content, but that variable is never defined in this test. Remove the trailing assert from the test case around bump_version and keep the malformed_content scenario focused on asserting the RuntimeError from bump_version(config, "1.2.4"), using the existing mock_open/patch setup to locate the affected test.openwisp_utils/releaser/version.py (1)
79-85: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Regex fallback may bump the wrong
versionfield.
_bump_with_regexreplaces the first^version\s*=\s*"..."(count=1). If any table preceding[project](e.g. a tool/build-system table) declares aversion = "...", that line is rewritten instead of[project].version. Thetomllibpath avoids this only when the exactversion = "{current}"string is unique; with two identical strings,content.replace(..., 1)(Line 77) also hits the first one.Consider anchoring the bump to the
[project]section (parse, locate the section span, and replace within it) rather than relying on first-occurrence matching.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@openwisp_utils/releaser/version.py` around lines 79 - 85, The version bump logic can rewrite the wrong version field because both the regex fallback in _bump_with_regex and the tomllib-based replace in bump_version only target the first matching occurrence. Update bump_version/version.py to locate the [project] section explicitly, then replace only the version entry inside that section, using the existing _bump_with_regex and bump_version symbols as the place to fix the section-scoped matching.
Moved version.py candidate-files check from _handle_pyproject_toml_version to _handle_python_version so it runs regardless of pyproject.toml existence. Added TypeError to except clause in _handle_python_version list() call.
|
@nemesifier once look into it please thanks! |
CodingWithSaksham
left a comment
CodingWithSaksham
left a comment
There was a problem hiding this comment.
Hi @BHARATH0153, I have reviewed your PR and the overall design is solid and this is a clean first step.
That said, before a revised PR lands, I would prefer to step back and have a broader discussion on issue #706 first. The reason is that pyproject.toml support is not just a releaser feature, it is the opening move of a larger migration that we want to get right. There are multiple cases that can happen like a mixture of setup.py and pyproject.toml (your PR), fully dropping setup.py and migrating to pyproject.toml and its PEP styles, or just remaining on setup.py.
|
@CodingWithSaksham let's wait for the @nemesifier @pandafy review |
|
@coderabbitai summary |
✅ Action performedSummary regeneration triggered. |
4 participants
closes #706
Changes:
Checklist