[HOTFIX] Regression tox fix #121
Conversation
….py (- WIP #119 -) ### ChangeLog: Changes in file setup.cfg: where = Changes in file setup.py: def readFile(filename): Changes in file tox.ini: commands = passenv =
…& PR #118 -) Changes in file .codecov.yml: * minor tweak to leave setup.py un-flagged Changes in file Makefile: * Cleaned up a bit Changes in file docs/conf.py: * refactored to comply with PEP-8 style Changes in file setup.py: * refactored to comply with PEP-8 style Changes in file tox.ini: * total refactor and debugging to ensure better stability
WalkthroughThe pull request introduces several modifications across multiple configuration files, including Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
added
Multicast
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## master #121 +/- ##
=========================================
Coverage 97.158% 97.158%
=========================================
Files 6 6
Lines 387 387
Branches 35 35
=========================================
Hits 376 376
Misses 7 7
Partials 4 4
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Sentry.
|
reactive-firewall
added
Linter
|
Fix worked: https://github.com/reactive-firewall/multicast/actions/runs/10952835394/job/30412304622?pr=121 no-longer has the |
…uires Functio (- WIP #119 -) See also PR #121 Additions in file tests/test_install_requires.py: - [x] Comprehensive Test Cases: * Valid Inputs: - Simple Version Constraints: - Test lines like package>=1.0 to ensure minimum versions are correctly parsed. - Multiple Version Constraints: - Test lines like package>=1.0,!=1.5,<2.0 and verify that only the minimum constraint (>=1.0) is captured. - Comments and Empty Lines: - Include full-line comments (# Comment) and inline comments (package>=1.0 # Inline comment). - Test with empty lines and lines with varying whitespace. - Options and URLs: - Ensure lines starting with -, --, or URLs (http://..., https://...) are ignored. * Invalid Inputs: - Malformed Lines: - Test lines with invalid syntax or unsupported specifiers to confirm they are handled gracefully. - Unsupported Specifiers: - Include lines with incompatible version specifiers to ensure they are skipped or reported appropriately. - [x] Integrated with Existing Testing Infrastructure Fixes in file setup.py: + added fix for safely handling missing requirements.txt files + added constraint to safely require semi-pinned packages in requirements.txt files (must have version now) Changes in file tests/__init__.py: - [x] Integrated with Existing Testing Infrastructure
…(- WIP #119 & PR #121 -) New Test-Suite in file tests/test_manifest.py: - [x] Verify Source Distribution Contents * Intent: Confirm that all necessary files are included in the source distribution (sdist). * Actions: - Create Test Cases in tests/test_manifest.py: - Use setUp and tearDown methods to build the distribution and clean up after tests. - Utilize tarfile (for .tar.gz) or zipfile (for .zip) to inspect the contents. - Verify inclusion of crucial files (requirements.txt, README.md, LICENSE.md, CHANGELOG.md, etc.). - Ensure exclusion of unwanted files or directories as specified in MANIFEST.in.
There was a problem hiding this comment.
Actionable comments posted: 4
Outside diff range and nitpick comments (1)
setup.py (1)
88-126: LGTM! The new function enhances the parsing of requirements.The
parse_requirements_for_install_requiresfunction introduces a more robust parsing mechanism for therequirements.txtfile. It effectively filters out comments, empty lines, and unsupported specifications, and extracts the minimal version constraints suitable for theinstall_requiresparameter.The regular expression used for parsing the package name and version is comprehensive and should cover most common scenarios. The function handles various edge cases and ensures that the
install_requireslist contains only the necessary information.This change should resolve the tox issue encountered in the previous pull request, as mentioned in the PR objectives.
Here are a few suggestions for further improvement:
Consider adding a brief comment explaining the purpose of the
bad_prefixeslist and why certain prefixes are considered bad.The regular expression used for parsing the package name and version can be slightly optimized. For example, you can use
[^\s,;]+instead of[^\s,;]+?to avoid unnecessary backtracking.Consider adding a few test cases to ensure that the function handles different scenarios correctly, such as packages with version specifiers like
==,>,<, etc.These are minor suggestions and do not affect the overall functionality of the function.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- setup.py (2 hunks)
- tests/init.py (2 hunks)
- tests/test_build.py (1 hunks)
- tests/test_install_requires.py (1 hunks)
- tests/test_manifest.py (1 hunks)
Additional context used
Path-based instructions (4)
tests/__init__.py (1)
Pattern
tests/*: When reviewing test-code for this project,
- Prioritize portability over clarity, especially when dealing with cross-Python compatibility. However, with the priority in mind, do still consider improvements to clarity when relevant.
- As a general guideline, consider the code-style advocated in the PEP-8 standard (excluding the use of spaces for indentation) and evaluate suggested changes for code-style compliance.
- As a general guideline, try to provide any relevant, official, and supporting documentation links to any tool's suggestions in review comments. This guideline is important for posterity.
- As a project rule, python source files, with names prefixed by the string "test_" and located in the project's "tests" directory, are the project's unit-testing code.
- As a project rule, any files with names prefixed by the string "check_" and located in the project's "tests" directory are the project's non-unit testing code. "Non-unit testing" in this context refers to any type of testing other than unit-testing, such as (but NOT limited to) functional testing, style linting, regression testing, etc. . It can also be assumed that non-unit testing code is usually written as bash shell-scripts.
tests/test_build.py (1)
Pattern
tests/*: When reviewing test-code for this project,
- Prioritize portability over clarity, especially when dealing with cross-Python compatibility. However, with the priority in mind, do still consider improvements to clarity when relevant.
- As a general guideline, consider the code-style advocated in the PEP-8 standard (excluding the use of spaces for indentation) and evaluate suggested changes for code-style compliance.
- As a general guideline, try to provide any relevant, official, and supporting documentation links to any tool's suggestions in review comments. This guideline is important for posterity.
- As a project rule, python source files, with names prefixed by the string "test_" and located in the project's "tests" directory, are the project's unit-testing code.
- As a project rule, any files with names prefixed by the string "check_" and located in the project's "tests" directory are the project's non-unit testing code. "Non-unit testing" in this context refers to any type of testing other than unit-testing, such as (but NOT limited to) functional testing, style linting, regression testing, etc. . It can also be assumed that non-unit testing code is usually written as bash shell-scripts.
tests/test_install_requires.py (1)
Pattern
tests/*: When reviewing test-code for this project,
- Prioritize portability over clarity, especially when dealing with cross-Python compatibility. However, with the priority in mind, do still consider improvements to clarity when relevant.
- As a general guideline, consider the code-style advocated in the PEP-8 standard (excluding the use of spaces for indentation) and evaluate suggested changes for code-style compliance.
- As a general guideline, try to provide any relevant, official, and supporting documentation links to any tool's suggestions in review comments. This guideline is important for posterity.
- As a project rule, python source files, with names prefixed by the string "test_" and located in the project's "tests" directory, are the project's unit-testing code.
- As a project rule, any files with names prefixed by the string "check_" and located in the project's "tests" directory are the project's non-unit testing code. "Non-unit testing" in this context refers to any type of testing other than unit-testing, such as (but NOT limited to) functional testing, style linting, regression testing, etc. . It can also be assumed that non-unit testing code is usually written as bash shell-scripts.
tests/test_manifest.py (1)
Pattern
tests/*: When reviewing test-code for this project,
- Prioritize portability over clarity, especially when dealing with cross-Python compatibility. However, with the priority in mind, do still consider improvements to clarity when relevant.
- As a general guideline, consider the code-style advocated in the PEP-8 standard (excluding the use of spaces for indentation) and evaluate suggested changes for code-style compliance.
- As a general guideline, try to provide any relevant, official, and supporting documentation links to any tool's suggestions in review comments. This guideline is important for posterity.
- As a project rule, python source files, with names prefixed by the string "test_" and located in the project's "tests" directory, are the project's unit-testing code.
- As a project rule, any files with names prefixed by the string "check_" and located in the project's "tests" directory are the project's non-unit testing code. "Non-unit testing" in this context refers to any type of testing other than unit-testing, such as (but NOT limited to) functional testing, style linting, regression testing, etc. . It can also be assumed that non-unit testing code is usually written as bash shell-scripts.
Learnings (1)
Common learnings
Learnt from: reactive-firewall PR: reactive-firewall/multicast#110 File: docs/CI.md:32-32 Timestamp: 2024-09-17T06:40:42.918Z Learning: Reactive-firewall prefers to keep the word "own" in headings when emphasizing the user's personal test.Learnt from: reactive-firewall PR: reactive-firewall/multicast#110 File: docs/USAGE.md:66-66 Timestamp: 2024-09-17T03:38:28.677Z Learning: Reactive-firewall prefers to use "Caveat" as a heading instead of "Note".
Ruff
tests/test_build.py
25-25: Local variable
ImportErris assigned to but never usedRemove assignment to unused variable
ImportErr(F841)
38-38: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
tests/test_install_requires.py
26-26: Local variable
ImportErris assigned to but never usedRemove assignment to unused variable
ImportErr(F841)
39-39: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
tests/test_manifest.py
26-26: Local variable
ImportErris assigned to but never usedRemove assignment to unused variable
ImportErr(F841)
40-40: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
Additional comments not posted (19)
tests/test_build.py (1)
1-80: Great work on the PEP 517 build test!The unit test file follows the project's naming convention and covers the essential aspects of building the package using PEP 517 standards. The imports are properly handled, and the test method verifies the expected output and the presence of the generated distribution files.
Tools
Ruff
25-25: Local variable
ImportErris assigned to but never usedRemove assignment to unused variable
ImportErr(F841)
38-38: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
tests/test_install_requires.py (9)
49-53: LGTM!The
setUpmethod correctly sets up the temporary requirements file for testing.
54-60: LGTM!The
tearDownmethod correctly cleans up the temporary requirements file and ensures that the parent class'stearDownmethod is called.
62-64: LGTM!The
write_requirementsmethod correctly writes the provided content to the temporary requirements file.
66-70: LGTM!The
test_simple_version_constraintmethod correctly tests parsing a simple version constraint.
72-76: LGTM!The
test_multiple_version_constraintsmethod correctly tests parsing multiple version constraints.
78-89: LGTM!The
test_comments_and_empty_linesmethod correctly tests handling comments and empty lines in the requirements file.
91-102: LGTM!The
test_options_and_urls_ignoredmethod correctly tests that options and URLs are ignored during parsing.
104-108: LGTM!The
test_malformed_linesmethod correctly tests handling of malformed requirement lines.
110-114: LGTM!The
test_nonexistent_requirements_filemethod correctly tests behavior when requirements file does not exist.tests/test_manifest.py (3)
47-57: LGTM!The
setUpmethod is correctly setting up the test environment by cleaning previous builds and asserting the expected output.
58-90: LGTM!The
test_sdist_includes_required_filesmethod is correctly testing that the source distribution includes all required files.
92-119: LGTM!The
test_sdist_excludes_unwanted_filesmethod is correctly testing that the source distribution excludes unwanted files.tests/__init__.py (4)
99-101: LGTM!The new test module imports are syntactically correct, align with the PR objective of expanding the test suite, and follow the project's naming convention.
105-105: LGTM!The
dependslist is correctly updated to include the new test modules, aligning with the PR objective and ensuring their inclusion in the test execution flow.
143-143: LGTM!The
test_casestuple is correctly updated to include the new test classes, aligning with the PR objective and ensuring their inclusion in the test execution flow.
144-144: LGTM!The
test_casestuple is correctly updated to include the new test class, aligning with the PR objective and ensuring its inclusion in the test execution flow.setup.py (2)
169-187: Good practice to wrap thesetupfunction in a conditional block.Wrapping the
setupfunction in theif __name__ == '__main__':conditional block is a good practice. It ensures that the setup code is executed only when the script is run directly, not when it is imported as a module.This change follows the best practices for Python packaging and prevents unintended execution of the setup code when the module is imported.
129-129: Updating therequirementsvariable to use the new parsing function is a good fix.Replacing the original line that read the requirements from the file with a call to the new
parse_requirements_for_install_requiresfunction ensures that the requirements are processed according to the new parsing logic.This change should resolve the issue with the
install_requiresfield that was causing the tox failures, as mentioned in the PR objectives. Therequirementsvariable now contains the cleaned-up list of requirements that can be directly used in the package setup process.
Summary by CodeRabbit
New Features
helptarget in the build process for better user guidance.MANIFEST.intarget to streamline file inclusion in builds.Bug Fixes
setup.pyfrom the analysis.Documentation
Chores