-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Release Candidate v1.4.5-RC #75
Conversation
…sed (- FIX #57 & #61 -) (- WIP #56 & #62 -) Changes in file README.md: - Added answers to what are the default values. Changes in file multicast/__init__.py: - Resolved #62 for now by changeing the default port to 59595 to comply with RFC-6335 Changes in file setup.cfg: - trivial post-release version update
### ChangeLog: Changes in file .github/labeler.yml: Unknown Changes
[TESTS] prototype config for dependabot Signed-off-by: Mr. Walls <reactive-firewall@users.noreply.github.com> ### ChangeLog: Changes in file .github/dependabot.yml: Unknown Changes
Bumps [github/codeql-action](https://github.com/github/codeql-action) from 2 to 3. - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](github/codeql-action@v2...v3) --- updated-dependencies: - dependency-name: github/codeql-action dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
…ns/dot-github/workflows/master/github/codeql-action-3
Changes in file multicast/__main__.py: def doStep(self, *args): Fixed issue #67
Testing out use of bandit for python scanning Signed-off-by: Mr. Walls <reactive-firewall@users.noreply.github.com>
Re-test after syncing with whitelisted settings
Changes in file setup.py: def readFile(filename): - fixed PTC-W6004 by improved input checking
Hopefully this goes well 🙊
This change will test if the fork was updated correctly.
…ciple (- WIP #71 -) Changes in file setup.py: def readFile(filename): - also used to read the requirements.txt file now
### ChangeLog: Changes in file setup.py: def readFile(filename):
Changes in file setup.py: def readFile(filename): - corrected regression by balancing the closing peren )
…e-67-fix' * PR #71 (feature-70-fix): [REGRESSION] fix for typo in setup.py 🙉 [PATCH] Applied changes as disscussed in review (- WIP #71 -) [PATCH] Improves on fix by using function to practice the D.R.Y. principle (- WIP #71 -) [STYLE] Improved input checking durring bootstrap (- WIP #70 -) * PR #68 (feature-bandit): Update .github/workflows/bandit.yml to test auto-fixes Update .github/workflows/bandit.yml to point at own fork Update bandit.yml Create bandit.yml * PR #69 (feature-67-fix): [FIX] Stability fix for error handling (- WIP #67 -) Changes in file .github/workflows/bandit.yml: - New workflow to bandit scan the repo. Changes in file multicast/__main__.py: def doStep(self, *args): - fix for error handling by simplifying use of `Exception.args` Changes in file setup.py: def readFile(filename): - stability and security improvements to bootstrapping.
…xt' manifest (- WIP #60 -) Changes in file .github/dependabot.yml: - also updates tests/requirements.txt via pip now Changes in file .github/labeler.yml: - Updated to label changes to the new tests/requirements.txt manifest correctly Changes in file .github/workflows/Tests.yml: jobs: - refactored to be more consistant regarding dependencies while testing in CI - pins requirements now for testing in CI (already did for production previously) - attempts to install additional python versions for testing by `tox` in CI (experemental) Changes in file Makefile: - minor changes to be consistant with maintained deps manifests Changes in file pyproject.toml: - minor changes to be consistant with maintained deps manifests Changes in file requirements.txt: - minor changes to be more consistant Changes in file setup.cfg: - minor changes to be consistant with maintained deps manifests Changes in file test-requirements.txt & in file tests/requirements.txt: - introduces a record-of-truth for test-code's dependencies as a new manifest for pip Changes in file tox.ini: - major changes to be consistant with new and maintained deps manifests
### ChangeLog: Changes in file tests/MulticastUDPClient.py: def say(self, address, port, conn, msg):
### ChangeLog: Changes in file Makefile: endif
### ChangeLog: Changes in file Makefile: PHONY: cleanup init must_be_root must_have_flake must_have_pytest uninstall clean: clean-docs cleanup endif test-style: cleanup must_have_flake test-tox: cleanup
### ChangeLog: Changes in file tests/requirements.txt: Unknown Changes
### ChangeLog: Changes in file tests/requirements.txt: pytest-flake8>=1.0.7 virtualenv>=15.0.1
* Correct CI warning when CodeClimate is skipped. * Corrected a test-code dependancy (`pytest-flake8` `1.0` --> `1.0.7` )
* From PR #74 (feature-60-test-reqs): [PATCH] Apply suggestions from code review [HOTFIX] fix for some test dependency conflicts (- WIP #60 -) [STYLE] fix for bad whitespace style-testing violation [TESTS] Reconciled testing dependencies via new 'tests/requirements.txt' manifest (- WIP #60 -) * From PR #73 (feature-72-bad-emoji): [DOCUMENTATION] Minor update to project README (- Fixes #72 -)
WalkthroughThis pull request introduces a new test suite in Changes
Sequence Diagram(s)sequenceDiagram
participant Tester
participant RequirementsFile
participant VirtualEnv
Tester->>RequirementsFile: Check existence
RequirementsFile-->>Tester: File exists
Tester->>RequirementsFile: Validate format
RequirementsFile-->>Tester: Format valid
Tester->>VirtualEnv: Create virtual environment
VirtualEnv-->>Tester: Environment created
Tester->>VirtualEnv: Install dependencies
VirtualEnv-->>Tester: Installation successful
Assessment against linked issues
Possibly related PRs
Suggested labels
Poem
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Additional context usedPath-based instructions (1)
Learnings (1)
Ruff
Additional comments not posted (4)
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 (
|
#103 -) ### ChangeLog: Changes in file tests/requirements.txt: Unknown Changes
### ChangeLog: Changes in file requirements.txt: Unknown Changes
### ChangeLog: Changes in file tests/check_legacy_setup_coverage: Unknown Changes
### ChangeLog: Changes in file Makefile: cleanup: Changes in file tests/__init__.py: Unknown Changes Changes in file tests/test_deps.py: Unknown Changes
### ChangeLog: Changes in file tests/context.py: def checkCovCommand(args=[None]):
### ChangeLog: Changes in file tests/context.py: def checkCovCommand(args=[None]):
* Enhanced documentation for the MCastClient class, improving clarity on initialization and message-sending capabilities. * Added examples for various initialization scenarios and usage of the say method for better understanding and testing. * Enhanced cleanup process to include the test_env directory during cleanup operations. * Introduced a new test suite to validate the presence and format of the requirements.txt file. * Expanded docstrings for the constructor and say method, detailing parameters and expected behaviors. * Expanded the test coverage by including the test_deps module in the test execution list. * Updated the license information for various dependencies in the requirements files for clarity and consistency. * Commented out inactive dependencies, including virtualenv, six, pgpy, and tox. * Updated commands for installing Python dependencies for improved clarity and structure. * Removed the installation of flake8 and streamlined coverage installation using a requirements file. * Renamed several job steps for better understanding and organization. * Enhanced command handling in tests for clearer expected behavior when invoking commands. --- * From PR #103 (HOTFIX-deps-style-update): [DOCUMENTATION] minor update for module license details (- WIP #96 & #103 -) * From PR #107 (100-add-tests-for-verifying-testsrequirementstxt-file): [TESTING] Updated script header with info (- WIP #100 -) * From PR #103 (patch--150-98-imp): [DOCUMENTATION] Updated script header with info (- WIP #98 -) * From PR #106 (patch-150-60-ci-fixup): [TESTING] fixed another unstable doc-tests (- WIP #53 -) [TESTING] fixed some unstable doc-tests (- WIP #60 & #53 -) [TESTING] fixed some bad whitespace (- WIP #60 -) [CI] minor update for CircleCI to use new 'tests/requirements.txt'(- WIP #99 & #100 & #60 -) * From PR #105 (patch-150-79-documentation): [PATCH] debugged failing tests (- WIP #53 & #96 -) [DOCUMENTATION] added additional doc-strings to MulticastUDPClient.McastClient.init (- WIP #53 & #96 -) [DOCUMENTATION] added additional doc-strings to MulticastUDPClient.McastClient.say (- WIP #53 & #96 -) Changes in file .circleci/config.yml: * Updated commands for installing Python dependencies, replaced Changes in file Makefile: * Added cleanup commands for ./test_env/**/*, ./test_env/*, and ./test_env/ in the cleanup target. Changes in file requirements.txt: * Standardized "license" spelling and specified licenses for sphinx, setuptools, pip, build, multicast, and wheel. Commented out inactive dependencies. Changes in file tests/MulticastUDPClient.py: * Added detailed docstrings to the __init__ and say methods in the MCastClient class. Changes in file tests/__init__.py: * Added test_deps to the depends list and imported it. Changes in file tests/check_legacy_setup_coverage: * Updated script header. Changes in file tests/context.py: * Adjusted the checkCovCommand function to clarify command handling when the 'run' argument is missing. Changes in file tests/requirements.txt: * Updated license comments for multiple dependencies, changing "licence" to "license" and clarifying license types for pep8, pytest, and others. Commented out inactive dependencies. Changes in file tests/test_deps.py: * Created a new test suite to check for the existence, format, and installation of dependencies in tests/requirements.txt.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (9)
- .circleci/config.yml (4 hunks)
- Makefile (7 hunks)
- requirements.txt (1 hunks)
- tests/MulticastUDPClient.py (4 hunks)
- tests/init.py (2 hunks)
- tests/check_legacy_setup_coverage (3 hunks)
- tests/context.py (7 hunks)
- tests/requirements.txt (1 hunks)
- tests/test_deps.py (1 hunks)
Files skipped from review as they are similar to previous changes (6)
- Makefile
- requirements.txt
- tests/init.py
- tests/check_legacy_setup_coverage
- tests/context.py
- tests/requirements.txt
Additional context used
Path-based instructions (2)
tests/test_deps.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/MulticastUDPClient.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.
Ruff
tests/test_deps.py
25-25: Local variable
ImportErr
is assigned to but never usedRemove assignment to unused variable
ImportErr
(F841)
36-36: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
44-44: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
52-52: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
GitHub Check: CodeQL
tests/test_deps.py
[failure] 72-72: Inefficient regular expression
This part of the regular expression may cause exponential backtracking on strings starting with '-' and containing many repetitions of '<=!'.
[failure] 72-72: Inefficient regular expression
This part of the regular expression may cause exponential backtracking on strings starting with '->' and containing many repetitions of '!<'.
Additional comments not posted (13)
tests/test_deps.py (2)
60-65
: LGTM!The test method is correctly implemented and follows the project's testing conventions. It checks for the existence of
tests/requirements.txt
and provides a clear assertion message.
88-98
: LGTM!The test method is correctly implemented and follows the project's testing conventions. It creates a virtual environment, installs the dependencies from
tests/requirements.txt
usingpip
, and asserts that the installation was successful.Skipping the test on unsupported platforms is a good practice to avoid failures. Using the
context
module to execute thepip
command is also a good practice for portability..circleci/config.yml (8)
24-24
: LGTM!The change from
|| true
to|| : ;
is a valid alternative to suppress the error code while allowing the script to continue execution. It aligns with the overall objective of streamlining the CI/CD pipeline.
29-29
: LGTM!The modification to the command for installing Python dependencies from
requirements.txt
aligns with the overall objective of streamlining the CI/CD pipeline.
33-35
: LGTM!The renaming of the job to "install test depends attempt" and the replacement of the coverage installation command with a command that installs dependencies from
tests/requirements.txt
reflect a broader scope of dependency installation. These changes ensure that all necessary testing dependencies are installed in one step and align with the overall objective of streamlining the CI/CD pipeline.
66-66
: LGTM!The modification to install dependencies from
tests/requirements.txt
aligns with the previous change and ensures consistency in the CI/CD pipeline. It contributes to the overall objective of streamlining the CI/CD pipeline.
112-114
: LGTM!The renaming of the job to "setup depends" and the simplification of the command for installing dependencies from
requirements.txt
contribute to the clarity and maintainability of the CI/CD pipeline configuration. These changes align with the overall objective of streamlining the CI/CD pipeline.
120-120
: LGTM!The addition of the command to install testing dependencies from
tests/requirements.txt
before running the pytest tests aligns with the overall objective of streamlining the CI/CD pipeline. It ensures that the necessary dependencies are in place for the pytest tests to run successfully.
168-170
: LGTM!The renaming of the job to "install reqs attempt" and the modification of the command for installing dependencies from
requirements.txt
contribute to the clarity and consistency of the CI/CD pipeline configuration. These changes align with the overall objective of streamlining the CI/CD pipeline.
172-174
: LGTM!The renaming of the job to "install test-reqs attempt" and the addition of the command for installing testing dependencies from
tests/requirements.txt
contribute to the clarity and consistency of the CI/CD pipeline configuration. These changes align with the overall objective of streamlining the CI/CD pipeline.tests/MulticastUDPClient.py (3)
Line range hint
87-129
: Excellent work on enhancing the class docstring!The updated docstring for the
MCastClient
class is comprehensive and well-structured. It includes clear testing examples that demonstrate how to import and use the class. This improvement greatly enhances the documentation and makes it easier for developers to understand and utilize the class effectively.
135-196
: Great job on the comprehensive docstring for__init__
!The updated docstring for the
__init__
function is excellent. It provides a clear explanation of the initialization process, including details about the optional parametersgrp_addr
andsrc_port
. The "Meta Testing" section with labeled test cases is particularly useful, as it demonstrates how to initialize theMCastClient
object under different scenarios.The docstring follows the established style in the codebase, ensuring consistency and readability. The actual implementation of the function remains unchanged, so there are no functional modifications to review.
Overall, this is a significant improvement to the documentation of the
__init__
function.
Line range hint
210-264
: Excellent work on the comprehensive docstring forsay
!The updated docstring for the
say
function is very well-written and informative. It provides a clear explanation of the function's purpose, including details about its parameters, return value, and printed output. The "Meta Testing" section with test cases is particularly helpful, as it demonstrates how to use the function and what to expect.The docstring follows the established style in the codebase, ensuring consistency and readability. The inclusion of the note about the assumption that the connection is properly configured is important for developers to be aware of.
The actual implementation of the function remains unchanged, so there are no functional modifications to review.
Overall, this is a significant improvement to the documentation of the
say
function.
PR/MR Overlap
Relevant Issues
Summary by CodeRabbit
New Features
tests/requirements.txt
file to manage testing dependencies.44244
to59259
.Bug Fixes
Documentation
HEAR
andSAY
commands from5353
to59595
in the README.Chores
1.5-rc
and updated dependency specifications across multiple files.