-
Notifications
You must be signed in to change notification settings - Fork 60
LCORE-475: fix behave tests issue with new version #366
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
Conversation
WalkthroughThis update adjusts the capitalization of step definitions and corresponding feature file steps in Behave-based end-to-end tests, ensuring consistency. Additionally, the development dependency on the "behave" package is broadened from an exact version to a minimum version requirement in the Changes
Sequence Diagram(s)sequenceDiagram
participant Tester
participant Behave
participant StepDefinitions
Tester->>Behave: Run feature files
Behave->>StepDefinitions: Match "The system is in default state"
StepDefinitions-->>Behave: Execute corresponding function
Behave-->>Tester: Report test results
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Possibly related PRs
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
tisnik
left a comment
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.
Nice catch, LGTM
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: 1
🧹 Nitpick comments (1)
tests/e2e/features/steps/common.py (1)
7-11: Step does not actually verify the service is upThe decorator string change is fine, but the body still only asserts
context is not None.
Consider adding a real health-check (e.g., HTTP ping) so the “service is started” step fulfils its contract.import requests, os @given("The service is started locally") def service_is_started_locally(context: Context) -> None: host = os.getenv("SERVICE_HOST", "http://localhost:8080") resp = requests.get(f"{host}/liveness", timeout=2) assert resp.status_code == 200, f"Service not live on {host}"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
pyproject.toml(1 hunks)tests/e2e/features/llm_interface.feature(1 hunks)tests/e2e/features/rest_api.feature(3 hunks)tests/e2e/features/smoketests.feature(1 hunks)tests/e2e/features/steps/common.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: radofuchs
PR: lightspeed-core/lightspeed-stack#306
File: .github/workflows/e2e_tests.yaml:4-4
Timestamp: 2025-07-28T13:15:54.895Z
Learning: The user radofuchs prefers unrestricted pull_request_target workflows with secrets access for comprehensive PR testing, accepting the security trade-offs to ensure all problems are caught during the PR process.
🔇 Additional comments (4)
tests/e2e/features/rest_api.feature (1)
12-13: LGTM – step text now matches updated step definitionsThe capitalization fix removes the Behave mismatch introduced by 1.3.x’s strict matching.
Also applies to: 30-31, 46-47, 53-54
tests/e2e/features/llm_interface.feature (1)
12-13: LGTM – consistency restoredCapitalization aligns with the new decorator string in
common.py.tests/e2e/features/smoketests.feature (1)
12-13: LGTM – capitalization fixMatches the updated step definition; no further issues spotted.
tests/e2e/features/steps/common.py (1)
13-17: No stale lowercase steps found
All feature files use the updated “Given The system is in default state” step definition. No further changes are needed.
| "ruff>=0.11.13", | ||
| "aiosqlite", | ||
| "behave==1.2.6", | ||
| "behave>=1.3.0", |
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.
💡 Verification agent
🧩 Analysis chain
Constrain the upper bound of behave for build reproducibility
Switching from an exact pin to an open-ended >=1.3.0 means CI may suddenly pick up a future 2.x release with breaking changes.
Recommend capping to the next major to keep the dependency surface predictable:
- "behave>=1.3.0",
+ "behave>=1.3.0,<2",Run the script below to confirm no other unconstrained major upgrades slipped into pyproject.toml.
🏁 Script executed:
#!/usr/bin/env bash
# Lists all dev dependencies without an upper bound
grep -n "dev =" -A100 pyproject.toml | grep -E '">=[0-9]+\.[0-9]+' | grep -v '<'Length of output: 82
🏁 Script executed:
#!/usr/bin/env bash
# Search for any dependency specifiers in pyproject.toml using >= without an upper bound
grep -n -E '"[A-Za-z0-9_\-]+>=[0-9]+\.[0-9]+"' pyproject.toml | grep -v '<'Length of output: 77
🏁 Script executed:
#!/usr/bin/env bash
# Search for any version specifiers using >= without an upper bound in pyproject.toml
grep -n '>=.*"' pyproject.toml | grep -v '<'Length of output: 793
Constrain upper bounds on all open-ended dependencies
Beyond pinning behave>=1.3.0,<2, we should audit and cap every >= specifier to the next major release to avoid inadvertent breaking changes when new majors are published. In your pyproject.toml, the following dependencies currently lack an upper bound:
• Line 25: fastapi>=0.115.6
• Line 26: uvicorn>=0.34.3
• Line 27: kubernetes>=30.1.0
• Line 30: rich>=14.0.0
• Line 31: cachetools>=6.1.0
• Line 32: prometheus-client>=0.22.1
• Line 33: starlette>=0.47.1
• Line 34: aiohttp>=3.12.14
• Line 35: authlib>=1.6.0
• Line 60: black>=25.1.0
• Line 61: pytest>=8.3.2
• Line 62: pytest-cov>=5.0.0
• Line 63: pytest-mock>=3.14.0
• Line 64: pytest-asyncio>=1.0.0
• Line 65: pyright>=1.1.401
• Line 66: pylint>=3.3.7
• Line 67: pydocstyle>=6.3.0
• Line 68: mypy>=1.16.0
• Line 69: types-PyYAML>=6.0.2
• Line 70: types-requests>=2.28.0
• Line 71: ruff>=0.11.13
• Line 73: behave>=1.3.0
• Line 74: types-cachetools>=6.1.0.20250717
• Line 75 & 79: build>=1.2.2.post1
• Line 76 & 80: twine>=6.1.0 / >=5.1.1
For example, update the snippet on line 73 as:
- "behave>=1.3.0",
+ "behave>=1.3.0,<2",—and apply a similar <next_major cap to each listed dependency.
📝 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.
| "behave>=1.3.0", | |
| "behave>=1.3.0,<2", |
🤖 Prompt for AI Agents
In pyproject.toml at line 73 and all other lines with open-ended dependencies
(lines 25-35, 60-76, 79-80), add an upper bound to each version specifier by
capping them to less than the next major version. For example, change
"behave>=1.3.0" to "behave>=1.3.0,<2". Apply this pattern consistently to all
listed dependencies to prevent unintended breaking changes from future major
releases.
Description
updated behave to 1.3.0 and fixed typos mismatches between step definitions and steps in feature files
Type of change
Related Tickets & Documents
Checklist before requesting a review
Testing
Summary by CodeRabbit