Dependencies update #51
Conversation
WalkthroughCI and workflow files updated for Python 3.10+ and version generation; linting, pre-commit, and formatting tools upgraded; packaging and metadata migrated from RST to Markdown with a version bump to 5.0.0; runtime and dev dependencies updated; large typing refactor applied to the Behave agent and its .pyi stub removed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Runner as Behave Runner
participant Agent as BehaveAgent
participant RP as ReportPortal Client
Runner->>Agent: start_launch(context)
Agent->>RP: start_launch(meta, attributes)
RP-->>Agent: launch_id
loop Items (feature/scenario/step)
Runner->>Agent: start_feature/start_scenario/start_step
Agent->>RP: start_test_item(...)
RP-->>Agent: item_id
alt logs/attachments
Agent->>RP: log(message, level, [attachment])
end
Runner->>Agent: finish_step/status
Agent->>RP: finish_test_item(status)
end
Runner->>Agent: finish_launch()
Agent->>RP: finish_launch(status)
RP-->>Agent: ack
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing touches
🧪 Generate unit tests
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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #51 +/- ##
===========================================
- Coverage 100.00% 99.43% -0.57%
===========================================
Files 3 3
Lines 328 353 +25
===========================================
+ Hits 328 351 +23
- Misses 0 2 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
CHANGELOG.md (1)
14-14: Fix typo: “Python 12” → “Python 3.12”.User-facing doc error in a released section.
-## [4.0.3] -### Added -- Python 12 support, by @HardNorth +## [4.0.3] +### Added +- Python 3.12 support, by @HardNorth
🧹 Nitpick comments (27)
.gitignore (1)
110-110: Consolidate the venv ignore pattern for simplicity.Use a single pattern to cover both
.venvand versioned variants; this also avoids redundancy with Line 107.-.venv3*/ +.venv*/Optionally, you can then remove Line 107 (
.venv) since.venv*/already matches it.MANIFEST.in (1)
1-2: Tighten sdist contents; drop self-include.Including MANIFEST.in itself is unnecessary; consider shipping LICENSE/CHANGELOG instead.
-include MANIFEST.in include README.md requirements.txt +include LICENSE +include CHANGELOG.mdCHANGELOG.md (1)
7-8: Polish wording: “updated to” (not “on”).Minor copy tweak for consistency.
-- Client version updated on [5.6.5](https://github.com/reportportal/client-Python/releases/tag/5.6.5), by @HardNorth -- Behave version updated on [1.3.3](https://github.com/behave/behave/releases/tag/v1.3.3), by @HardNorth +- Client version updated to [5.6.5](https://github.com/reportportal/client-Python/releases/tag/5.6.5), by @HardNorth +- Behave version updated to [1.3.3](https://github.com/behave/behave/releases/tag/v1.3.3), by @HardNorth.flake8 (1)
2-2: Align Flake8 ignores with Black.Black recommends ignoring W503 in addition to E203; also ensure Black’s line-length matches 119 in pyproject.
-ignore = E203 +ignore = E203, W503requirements.txt (1)
1-1: Guard against future major bumps.Consider upper-bounding behave to avoid untested majors.
-behave>=1.3.3 +behave>=1.3.3,<2.0requirements-dev.txt (1)
1-2: Pin dev tool ranges to stabilize formatting.Unpinned Black/Isort can churn diffs; suggest bounded ranges.
-black -isort +black>=24,<25 +isort>=5.13,<6tox.ini (1)
33-38: GH Actions mapping is sensible; run pep once on 3.10.Optional: add
skip_missing_interpreters = trueto ease local tox.behave_reportportal/utils.py (1)
25-26: Singleton not thread-safe; add a lock around instance creation.Concurrent first access can instantiate twice. Guard with a class-level lock.
Apply within this block:
- cls._instances[cls] = super(Singleton, cls).__call__(*args, **kwargs) + with cls._lock: + if cls not in cls._instances: + cls._instances[cls] = super(Singleton, cls).__call__(*args, **kwargs)And add outside this hunk:
import threading class Singleton(type): _instances = {} _lock = threading.Lock()pyproject.toml (1)
3-6: Avoid hard pinning wheel in build-system or align with CI.CI installs latest wheel, but pyproject pins 0.40.0; this mismatch can cause surprises. Prefer a lower-bound or align versions.
- "wheel==0.40.0", + "wheel>=0.40.0",.pre-commit-config.yaml (1)
21-24: Let pre-commit pass changed files; drop path args for Black.Passing directories makes Black scan whole trees each run and can ignore other paths managed by pre-commit.
- - id: black - args: [ '--check', 'behave_reportportal', 'tests' ] + - id: black + args: [ '--check' ]behave_reportportal/config.py (5)
116-121: Fix missing space in deprecation warning message.Two adjacent literals concatenate without a space, yielding “deprecatedin”.
- "'step_based' config setting has been deprecated" "in favor of the new log_layout configuration.", + "'step_based' config setting has been deprecated in favor of the new log_layout configuration.",
139-144: Tighten wording of api_key warning.Minor grammar/readability tweak.
- "that's not supposed to happen because ReportPortal " - "is usually requires an authorization key. " - "Please check your code.", + "this is unexpected because ReportPortal usually requires an authorization key. " + "Please check your configuration.",
103-104: Split launch_attributes on any whitespace.Handles multiple spaces/tabs more robustly.
- self.launch_attributes = launch_attributes and launch_attributes.split(" ") + self.launch_attributes = launch_attributes and launch_attributes.split()
85-89: Broaden retries type to accept int and str.Tests already pass ints; update annotation and parsing.
- retries: Optional[str] = None, + retries: Optional[Union[str, int]] = None,- self.retries = retries and int(retries) + self.retries = int(retries) if retries is not None else NoneAlso applies to: 106-106
147-151: Trim/normalize output and client_type inputs.Guard against accidental whitespace.
- self.launch_uuid_print_output = ( - OutputType[launch_uuid_print_output.upper()] if launch_uuid_print_output else None - ) - self.client_type = ClientType[client_type.upper()] if client_type else ClientType.SYNC + self.launch_uuid_print_output = ( + OutputType[launch_uuid_print_output.strip().upper()] if launch_uuid_print_output else None + ) + self.client_type = ClientType[client_type.strip().upper()] if client_type else ClientType.SYNCsetup.py (2)
23-31: Open files with explicit UTF-8 encodingAvoid locale-dependent reads for README.md; specify encoding.
-def read_file(fname): +def read_file(fname): """Read the given file. @@ - with open(os.path.join(os.path.dirname(__file__), fname)) as f: + with open(os.path.join(os.path.dirname(__file__), fname), encoding="utf-8") as f: return f.read()
43-44: Remove stale package_data for .pyi if no stubs remainIf .pyi files were removed, drop this to avoid confusion; otherwise keep as-is.
- package_data={"behave_reportportal": ["*.pyi"]}, + # package_data={"behave_reportportal": ["*.pyi"]},tests/units/test_rp_agent.py (3)
285-293: Silence false-positive secret scanner for deprecated token arg in testsThese are test-only dummy values exercising a deprecated path; add Ruff ignore.
- cfg = Config( + cfg = Config( endpoint="endpoint", - token="token", + token="token", # noqa: S106 - test dummy value for deprecated param project="project",
836-841: Same token false-positive hereAdd ignore.
- cfg = Config( + cfg = Config( endpoint="endpoint", - token="token", + token="token", # noqa: S106 - test dummy value for deprecated param project="project",
904-905: And here as wellAdd ignore.
- cfg = Config(endpoint="E", token="T", project="P", log_layout=LogLayout.STEP) + cfg = Config(endpoint="E", token="T", project="P", log_layout=LogLayout.STEP) # noqa: S106behave_reportportal/behave_agent.py (7)
40-52: Decorator should return the wrapped function’s result (future-proofing)Current usages return None, but returning the inner result avoids surprises if signatures evolve.
def check_rp_enabled(func: Callable) -> Callable: @@ - def wrap(*args, **kwargs): + def wrap(*args, **kwargs): if args and isinstance(args[0], BehaveAgent): # noinspection PyProtectedMember if not args[0]._rp: return - func(*args, **kwargs) + return func(*args, **kwargs)
192-193: Stray trailing comma creates a tuple expressionNo functional impact, but it’s noisy. Remove the comma.
- self._log_cleanups(context, "scenario"), + self._log_cleanups(context, "scenario")
298-301: Avoid list comprehension for side effectsUse a simple loop for readability.
- [pt.add_row(row.cells) for row in step.table.rows] + for row in step.table.rows: + pt.add_row(row.cells)
268-276: Gracefully handle attachment I/O errorsA missing/unreadable file will raise and abort logging. Consider swallowing OSError and posting a warning log instead.
- if file_to_attach: - with open(file_to_attach, "rb") as f: - attachment = { - "name": os.path.basename(file_to_attach), - "data": f.read(), - "mime": mimetypes.guess_type(file_to_attach)[0] or "application/octet-stream", - } + if file_to_attach: + try: + with open(file_to_attach, "rb") as f: + attachment = { + "name": os.path.basename(file_to_attach), + "data": f.read(), + "mime": mimetypes.guess_type(file_to_attach)[0] or "application/octet-stream", + } + except OSError: + self._rp.log(time=timestamp(), message=f"Attachment not found: {file_to_attach}", level="WARN", item_id=item_id)
382-390: Prefer keyword args for finish_test_itemAlign with the rest of the file and the client’s signature for clarity.
- self._rp.finish_test_item(self._step_id, timestamp(), "PASSED") + self._rp.finish_test_item(item_id=self._step_id, end_time=timestamp(), status="PASSED")
411-417: Remove duplicate assignment and use keyword argsMinor cleanup.
- self._step_id = self._step_id = self._rp.start_test_item( + self._step_id = self._rp.start_test_item( name=msg, start_time=timestamp(), item_type=item_type, parent_item_id=item_id, has_stats=False if self._cfg.log_layout is LogLayout.NESTED else True, ) - self._rp.finish_test_item(self._step_id, timestamp(), "PASSED") + self._rp.finish_test_item(item_id=self._step_id, end_time=timestamp(), status="PASSED")
400-404: Avoid building a list just to call iter()Use next(generator, None) for simplicity.
- layer = next( - iter([level for level in context._stack if level.get("@layer") == scope]), - None, - ) + layer = next((level for level in context._stack if level.get("@layer") == scope), None)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
.flake8(1 hunks).github/workflows/release.yml(3 hunks).github/workflows/tests.yml(2 hunks).gitignore(1 hunks).pre-commit-config.yaml(1 hunks)CHANGELOG.md(1 hunks)MANIFEST.in(1 hunks)README.md(1 hunks)README.rst(0 hunks)behave_reportportal/behave_agent.py(23 hunks)behave_reportportal/behave_agent.pyi(0 hunks)behave_reportportal/config.py(2 hunks)behave_reportportal/utils.py(1 hunks)pyproject.toml(1 hunks)requirements-dev.txt(1 hunks)requirements.txt(1 hunks)setup.cfg(1 hunks)setup.py(2 hunks)tests/features/steps/calculator.py(2 hunks)tests/units/test_config.py(4 hunks)tests/units/test_rp_agent.py(18 hunks)tox.ini(2 hunks)
💤 Files with no reviewable changes (2)
- behave_reportportal/behave_agent.pyi
- README.rst
🧰 Additional context used
🧬 Code graph analysis (3)
tests/units/test_config.py (2)
tests/units/test_rp_agent.py (1)
config(32-40)behave_reportportal/config.py (2)
LogLayout(31-45)read_config(163-174)
tests/units/test_rp_agent.py (2)
behave_reportportal/behave_agent.py (3)
_code_ref(448-451)create_rp_service(55-73)_log(261-282)behave_reportportal/config.py (2)
Config(48-160)LogLayout(31-45)
behave_reportportal/behave_agent.py (2)
behave_reportportal/config.py (2)
Config(48-160)LogLayout(31-45)behave_reportportal/utils.py (1)
Singleton(17-26)
🪛 Gitleaks (8.27.2)
README.md
[high] 55-55: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 Ruff (0.12.2)
tests/units/test_rp_agent.py
904-904: Possible hardcoded password assigned to argument: "token"
(S106)
🔇 Additional comments (12)
requirements.txt (1)
3-3: LGTM on client bump.reportportal-client~=5.6.5 aligns with the changelog.
tox.ini (1)
11-11: Python 3.13 env addition looks good.setup.cfg (1)
5-5: Switch to README.md acknowledged.Ensure the long description content type is set to
text/markdown(in setup.cfg or setup.py) so PyPI renders correctly..github/workflows/tests.yml (1)
10-10: Matrix update (add 3.13) looks good.pyproject.toml (1)
9-17: LGTM: isort/black config aligns with repo style.tests/features/steps/calculator.py (1)
58-58: LGTM: assertion message formatting is cleaner without behavior changes.Also applies to: 70-71
.github/workflows/release.yml (1)
28-34: LGTM: moving version generation earlier is sensible.tests/units/test_config.py (2)
192-201: LGTM: LogLayout parametrization covers case-insensitive and numeric inputs.
339-358: LGTM: timeout parsing tests exercise all branches.setup.py (2)
20-20: Major version bump checkv5.0.0 suggests breaking changes. Ensure CHANGELOG and release notes call out Python 3.7 drop and any public API shifts, and CI/publishing pipelines are aligned.
44-55: Python version metadata looks consistentpython_requires >=3.8 aligns with classifiers up to 3.13. LGTM.
tests/units/test_rp_agent.py (1)
23-24: Confirm client classes availability across versionsEnsure the pinned reportportal-client version used in CI exposes BatchedRPClient and ThreadedRPClient; some older versions varied class names/exports.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
tests/units/test_rp_agent.py (2)
907-911: Resolved: mocks correctly instantiatedNice fix; using
mock.Mock()instances unblocks attribute assignment and call assertions.
934-938: Resolved: mocks correctly instantiated (scenario-based cleanup test)Same fix applied here; looks good.
🧹 Nitpick comments (11)
.pre-commit-config.yaml (2)
4-11: pre-commit-hooks upgrade looks good; consider adding a few low-cost safety checks.Optional but useful to catch common issues across the repo.
- repo: https://github.com/pre-commit/pre-commit-hooks rev: v5.0.0 hooks: - id: trailing-whitespace - id: end-of-file-fixer - id: check-yaml - id: check-added-large-files + - id: check-merge-conflict + - id: check-case-conflict + - id: mixed-line-ending + - id: debug-statements + - id: check-toml + - id: check-json
20-29: Order isort before black; confirm intent to use --check (no autofix).If you prefer auto-formatting locally/CI, drop --check and let hooks rewrite. Also, running isort before black avoids minor churn.
- - repo: https://github.com/psf/black - rev: 24.10.0 - hooks: - - id: black - args: [ '--check' ] - - repo: https://github.com/pycqa/isort - rev: 6.0.0 - hooks: - - id: isort + - repo: https://github.com/pycqa/isort + rev: 6.0.0 + hooks: + - id: isort + - repo: https://github.com/psf/black + rev: 24.10.0 + hooks: + - id: blackIf you do want autofix:
- - id: black - args: [ '--check' ] + - id: blackbehave_reportportal/config.py (2)
144-146: Makeenabledrobust against accidental whitespace-only valuesWhitespace-only strings currently enable RP; strip before evaluation.
- self.enabled = all([self.endpoint, self.project, self.api_key]) + self.enabled = all(v and str(v).strip() for v in [self.endpoint, self.project, self.api_key])
146-149: Gracefully handle invalidlaunch_uuid_print_outputvaluesMapping unknown values raises KeyError; fallback to None with a warning.
- self.launch_uuid_print_output = ( - OutputType[launch_uuid_print_output_strip.upper()] if launch_uuid_print_output_strip else None - ) + if launch_uuid_print_output_strip: + try: + self.launch_uuid_print_output = OutputType[launch_uuid_print_output_strip.upper()] + except KeyError: + warn( + f"Unknown launch_uuid_print_output: {launch_uuid_print_output!r}; using None.", + RuntimeWarning, + stacklevel=2, + ) + self.launch_uuid_print_output = None + else: + self.launch_uuid_print_output = Nonetests/units/test_rp_agent.py (2)
904-906: Silence Ruff S106 false-positive for test tokenThis is a test fixture, not a secret. Add a local noqa to keep CI green.
- cfg = Config(endpoint="E", token="T", project="P", log_layout=LogLayout.STEP) + cfg = Config(endpoint="E", token="T", project="P", log_layout=LogLayout.STEP) # noqa: S106
835-841: Silence Ruff S106 false-positive for test tokenSame as above; add noqa on the token line.
- token="token", + token="token", # noqa: S106behave_reportportal/behave_agent.py (2)
333-367: Include exception message when traceback missingIf only
exceptionis present (noexc_traceback), we currently skip the exception detail. Appendstr(exception)in that case for better diagnostics.- if exc_holder.exception and exc_holder.exc_traceback: + if exc_holder.exception and exc_holder.exc_traceback: message.append( "".join( traceback.format_exception( type(exc_holder.exception), exc_holder.exception, exc_holder.exc_traceback, ) ) ) + elif exc_holder.exception: + message.append(str(exc_holder.exception))
214-222: Minor: DRYhas_statscomputation
has_stats=False if self._cfg.log_layout is LogLayout.NESTED else Truerepeats. Consider a small helper/variable to centralize it.Also applies to: 393-396, 419-422
MANIFEST.in (1)
1-1: Add pyproject.toml to sdist; add py.typed only if you ship a typing markerpyproject.toml is present in the repo; behave_reportportal/py.typed was not found. Update MANIFEST.in (root) to include pyproject.toml, and only add "include behave_reportportal/py.typed" if you add a PEP 561 py.typed marker to the package.
setup.py (2)
42-44: If the package is now typed (PEP 561), ship a py.typed marker.Without py.typed, type checkers will treat the package as untyped after removing .pyi.
setup( @@ - packages=["behave_reportportal"], + packages=["behave_reportportal"], + package_data={"behave_reportportal": ["py.typed"]},And add an empty file behave_reportportal/py.typed to the repo (plus include it in MANIFEST.in as noted).
45-45: Improve license metadata for PyPI.Add the OSI classifier and ensure the license file is included in wheels.
license="Apache 2.0", @@ classifiers=[ + "License :: OSI Approved :: Apache Software License", "Programming Language :: Python :: 3.8", "Programming Language :: Python :: 3.9", "Programming Language :: Python :: 3.10", "Programming Language :: Python :: 3.11", "Programming Language :: Python :: 3.12", "Programming Language :: Python :: 3.13", ], + license_files=["LICENSE"],Also applies to: 48-54
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
.flake8(1 hunks).pre-commit-config.yaml(1 hunks)CHANGELOG.md(1 hunks)MANIFEST.in(1 hunks)behave_reportportal/behave_agent.py(20 hunks)behave_reportportal/config.py(2 hunks)requirements.txt(1 hunks)setup.py(2 hunks)tests/units/test_rp_agent.py(19 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- requirements.txt
- .flake8
- CHANGELOG.md
🧰 Additional context used
🧬 Code graph analysis (2)
behave_reportportal/behave_agent.py (3)
tests/units/test_rp_agent.py (1)
config(32-40)behave_reportportal/config.py (2)
Config(48-161)LogLayout(31-45)behave_reportportal/utils.py (1)
Singleton(17-26)
tests/units/test_rp_agent.py (2)
behave_reportportal/behave_agent.py (3)
_code_ref(451-454)create_rp_service(55-73)_log(261-287)behave_reportportal/config.py (2)
Config(48-161)LogLayout(31-45)
🪛 Ruff (0.12.2)
tests/units/test_rp_agent.py
904-904: Possible hardcoded password assigned to argument: "token"
(S106)
🔇 Additional comments (8)
.pre-commit-config.yaml (2)
12-19: pydocstyle bump and scoped exclude look fine.Regex and rev are sane; no actionable concerns.
30-33: Approve — flake8 config matches Black/isort.flake8: ignore = E203, W503; max-line-length = 119. pyproject.toml: [tool.black] line-length = 119 and [tool.isort] line_length = 119. No changes required.
behave_reportportal/behave_agent.py (1)
55-74: create_rp_service passthroughs look correctPropagating
http_timeoutand other Config fields tocreate_clientis consistent and complete.setup.py (5)
29-31: UTF-8 file reading helper — LGTM.Explicit encoding avoids locale gotchas.
37-39: Switch to README.md with Markdown content-type — LGTM.This aligns with the manifest and modern PyPI rendering.
48-54: No action needed — CI already tests Python 3.13.
.github/workflows/tests.yml:10 includes python-version with '3.13'; tox.ini maps 3.13 -> py313; setup.py classifiers list 3.13.
42-42: No change required — no subpackages detected; packages=['behave_reportportal'] is fine.
Repository contains a single package directory behave_reportportal with init.py and modules (behave_agent.py, config.py, utils.py); no nested packages found.
44-44: install_requires usage OK — requirements files contain only plain package specssetup.py (line 44) uses install_requires=read_file("requirements.txt").splitlines(); ./requirements.txt and ./requirements-dev.txt contain only plain package specifiers (no -r/--requirement, -c/--constraint, -e/--editable, URLs, or comments), so no change required.
2 participants
Summary by CodeRabbit
New Features
Chores
Documentation
Tests
Refactor