docs: add E2E BDD testing design for cross-language and cross-OS exec…#184
Open
rjeffman wants to merge 40 commits into
Open
docs: add E2E BDD testing design for cross-language and cross-OS exec…#184rjeffman wants to merge 40 commits into
rjeffman wants to merge 40 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a Behave-based E2E BDD test suite: design doc, feature specs, step definitions, support utilities (CLI wrapper, fixtures, HTTP server), container images/runner, Make/pre-commit/pyproject integration, and exported dev dependency pins. ChangesE2E BDD Testing Implementation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
rjeffman
force-pushed
the
e2e_bdd
branch
2 times, most recently
from
aae2052 to
e2c9195
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e2c9195f32
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (10)
e2e/features/steps/marketplace_steps.py (1)
11-11: ⚡ Quick winAdd
-> Nonefor the step function signature.As per coding guidelines:
**/*.py: "Use Python 3.13 with modern type hints (e.g.,list[str]instead ofList[str])."🤖 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 `@e2e/features/steps/marketplace_steps.py` at line 11, Add an explicit return type annotation "-> None" to the step function signature for step_marketplace_catalog_served so it follows the project typing guidelines (use modern Python 3.13 hints). Locate the function named step_marketplace_catalog_served in marketplace_steps.py and change its signature to include "-> None"; no additional imports are required for this simple None return annotation.Source: Coding guidelines
e2e/features/support/http_server.py (1)
12-12: ⚡ Quick winAdd explicit
-> Noneannotations for new methods.As per coding guidelines:
**/*.py: "Use Python 3.13 with modern type hints (e.g.,list[str]instead ofList[str])."Also applies to: 21-21
🤖 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 `@e2e/features/support/http_server.py` at line 12, Add explicit return type annotations "-> None" to newly added methods, e.g., update the constructor signature "def __init__(self, directory: Path)" to include "-> None" and do the same for other new methods referenced in this change (the method at or around line with symbol names shown in the diff such as __init__ and the method near line 21). Ensure all new functions follow the codebase's Python 3.13 typing style (use "-> None" for procedures) without changing other logic.Source: Coding guidelines
e2e/features/environment.py (2)
10-10: ⚡ Quick winAdd explicit return type hints to Behave hooks.
As per coding guidelines:
**/*.py: "Use Python 3.13 with modern type hints (e.g.,list[str]instead ofList[str])."Also applies to: 29-29
🤖 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 `@e2e/features/environment.py` at line 10, The Behave hook functions (e.g., before_scenario and after_scenario) lack explicit return type hints; update their signatures to include modern Python return annotations (use None return type, e.g., def before_scenario(context, scenario) -> None:) and apply the same change to the corresponding hook at line 29 (after_scenario) so both functions use explicit type hints following the codebase guidelines.Source: Coding guidelines
29-33: ⚡ Quick winGuarantee temp-dir cleanup even when one server teardown fails.
Wrap the server-stop loop so
shutil.rmtree(...)runs in afinally; otherwise a single stop failure can leak scenario state into later runs.🤖 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 `@e2e/features/environment.py` around lines 29 - 33, The after_scenario cleanup must guarantee shutil.rmtree(context.tmp_dir) runs even if a server.stop() raises; change after_scenario so the server-stop loop is inside a try block and the rmtree call is in a finally block. Inside the try, iterate over getattr(context, "http_servers", []) and call server.stop(), but catch and log exceptions from each server.stop() (do not let one failure escape) so teardown continues; ensure references to after_scenario, context.http_servers, server.stop(), and context.tmp_dir are used to locate the code.e2e/features/market/rm.feature (1)
10-13: ⚡ Quick winAdd a post-remove state assertion in the success scenario.
The scenario currently validates message/exit code only. Please also assert the marketplace reference/cache is absent so
market rmis behaviorally verified, not just text-verified.🤖 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 `@e2e/features/market/rm.feature` around lines 10 - 13, After removing the marketplace you must assert its absence, not just success text; update the scenario that runs 'When I run lola "market rm community"' to also run a check like 'When I run lola "market list"' (or an equivalent step that inspects the marketplace cache) and add an assertion 'Then the output should not contain "community"' (or assert the cache file/entry for community is absent). Modify the feature to include the extra step and expectation so the removal is behaviorally verified rather than only text-verified.e2e/features/market/set.feature (1)
11-23: ⚡ Quick winThese scenarios should assert persisted enable/disable state, not only output text.
A follow-up assertion on marketplace state (e.g., config/list status after each command) would prevent false positives where output is correct but state is unchanged.
🤖 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 `@e2e/features/market/set.feature` around lines 11 - 23, The scenarios currently only assert CLI output for the commands 'lola "market set community --disable"' and 'lola "market set community --enable"' but not the persisted state; after each set command, run the CLI command that exposes persisted marketplace state (for example 'lola "market list"' or 'lola "market show community"' / 'lola "market config get community"') and assert the marketplace is actually marked disabled/enabled in that output or config; update both scenarios to add these follow-up assertions immediately after the set commands so the test verifies state persistence, not just transient text.e2e/features/steps/cli_steps.py (1)
11-62: ⚡ Quick winAdd Python 3.13 style type hints on step functions.
These step definitions are untyped; adding concrete parameter/return annotations will align this file with repo rules and improve editor/static-checker feedback.
As per coding guidelines:
**/*.py: Use Python 3.13 with modern type hints (e.g., list[str] instead of List[str]).🤖 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 `@e2e/features/steps/cli_steps.py` around lines 11 - 62, Annotate all step functions with Python 3.13 style type hints: add "from typing import Any" at the top, change signatures to include context: Any and concrete types for other params (e.g., command: str, code: int, text: str, pattern: str) and annotate each function return type as -> None (e.g., def step_run_lola(context: Any, command: str) -> None:). Update the step functions step_run_lola, step_exit_code, step_output_contains, step_output_matches, step_run_lola_no_args, and step_output_not_contains accordingly; you may also annotate locals like args: list[str] if desired, but ensure the function signatures and return types are added.Source: Coding guidelines
e2e/features/mod/add.feature (1)
15-16: ⚡ Quick winAdd an explicit exit-code assertion for the duplicate module scenario.
Right now this case can pass on message text alone; please assert the expected return code too (success or failure per product contract) to lock behavior.
🤖 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 `@e2e/features/mod/add.feature` around lines 15 - 16, Update the scenario that runs the step "When I run lola \"mod add {module_path}\"" to include an explicit assertion of the command's exit code (e.g., add a step like 'Then the exit code should be 1' or whatever return code the product contract defines for duplicate modules) in addition to the existing "Then the output should contain \"already exists\"" check so the test fails if the command returns an unexpected success/failure code.e2e/features/install/scope.feature (1)
1-11: ⚡ Quick winAdd a user-wide scope scenario to match the feature contract.
Line 2-Line 3 states support for both project-local and user-wide installs, but Line 5-Line 10 only exercises the default project scope. Add a second scenario that installs with explicit user scope and asserts files land in the user target location.
🤖 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 `@e2e/features/install/scope.feature` around lines 1 - 11, Add a new scenario to e2e/features/install/scope.feature titled "Install with user scope" that mirrors the existing "Install with project scope by default" steps (Given a module "my-module" with skills, commands, and agents; And the module "my-module" is registered) but runs the CLI with the user/global flag (e.g. run lola "install my-module --global -a claude-code" or the equivalent -g) and asserts exit code 0 and that the user-target directory (e.g. "{home}/.claude/skills/skill1") exists instead of the project path; keep step wording consistent with existing scenario so step definitions (Given/When/Then) reuse the same step implementations.e2e/features/install/uninstall.feature (1)
5-12: ⚡ Quick winCover legacy prefixed file cleanup in uninstall E2E.
Current assertions only check skill directory removal. Please add a scenario (or extend this one) that seeds legacy prefixed command/agent files and verifies they are removed by uninstall for backward compatibility.
Based on learnings: “Uninstall operations must check for and clean up old prefixed filenames (
<module>.<cmd>.md,<module>.<agent>.md) for backwards compatibility”.🤖 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 `@e2e/features/install/uninstall.feature` around lines 5 - 12, Add seeding and assertions for legacy prefixed files to the "Uninstall a previously installed module" scenario: before the uninstall step create legacy prefixed files named using the module prefix pattern (e.g. "{project}/.claude/commands/my-module.testcmd.md" and "{project}/.claude/agents/my-module.testagent.md" or whatever command/agent basenames your test suite uses) and after the When step assert those files no longer exist (similar to the existing "{project}/.claude/skills/skill1" check). Ensure you use the same scenario name "Uninstall a previously installed module" and the existing uninstall step When I run lola "uninstall my-module -a claude-code" so the test verifies removal of "<module>.<cmd>.md" and "<module>.<agent>.md" legacy filenames for backward compatibility.Source: Learnings
🤖 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 @.pre-commit-config.yaml:
- Around line 67-71: Pre-commit currently runs the behave hook (id: behave) with
the CI-default tag expression --tags="~`@wip`"; update the behave hook's entry so
it uses the documented pre-commit smoke subset by replacing --tags="~`@wip`" with
--tags="`@smoke`" in the entry command for the behave hook to align with
docs/dev-guide/design/e2e-bdd.md.
In `@docs/dev-guide/design/e2e-bdd.md`:
- Around line 19-67: Update the directory diagram in the e2e-bdd.md doc so it
accurately shows that Gherkin files and Python implementation live together
under features/ (include behave.ini, environment.py, steps/, support/ and the
feature subdirs under features/) and keep go/ as the future godog area;
specifically replace the current top-level features/ vs. future split with the
corrected tree that nests behave.ini, environment.py, steps/, support/, mod/,
install/, and market/ under features/ (this aligns with behave.ini,
environment.py, and the mk/e2e.mk behavior referenced elsewhere).
- Around line 771-785: Replace the incorrect local-run command "cd e2e/python &&
behave ../features" with the actual implementation command used by the project:
change the line to use "cd e2e/features && uv run behave" so the docs reflect
the real local workflow; update the Local Developer Workflow section where the
bash examples contain the old command string to the new command string to keep
examples accurate.
- Around line 450-462: Update the Makefile examples for targets e2e, e2e-wip,
and e2e-smoke so they match the implemented command invocation: replace the
shown `cd e2e/python && behave` commands with `cd e2e/features && uv run behave`
(preserve the existing flags like --format, --no-capture, and --tags), ensuring
each target (e2e, e2e-wip, e2e-smoke) uses the corrected path and the `uv run`
prefix.
- Around line 584-608: The example container entrypoint script in the Test
Entrypoint section uses the wrong directory; update the python branch in the
run-tests.sh example so the python case changes directory to the real tests
directory (use cd e2e/features instead of cd e2e/python) and keep the rest of
the script (exec behave --tags="$TAGS" --format progress --no-capture
../features) unchanged so the python case in the case "$LANG" block matches the
actual implementation.
- Around line 787-811: The Makefile snippets for the e2e targets are using the
wrong working directory and invocation: update the e2e, e2e-wip and e2e-smoke
targets (symbols: e2e, e2e-wip, e2e-smoke) to use cd e2e/features and invoke
behave via uv run behave (so the command becomes cd e2e/features && uv run
behave) and remove the explicit --format progress, leaving only --no-capture
(preserve the tag arguments like --tags="$(E2E_TAGS)" or
--tags="`@wip`"/"`@smoke`"); make the same substitution for every occurrence of cd
e2e/python && behave in the diff.
In `@e2e/containers/scripts/run-tests.sh`:
- Around line 4-7: The script currently reassigns the environment variable LANG
(used for locale), which can break subprocess locale/encoding; replace usage of
LANG for test-language selection with a distinct variable name (e.g., TEST_LANG
or RUNNER_LANG) wherever it’s referenced (initial assignment at the top and the
case switch, and the other occurrences around lines 17-18), update the case
statement and any downstream references to use that new variable name, and leave
the original LANG environment variable untouched so subprocesses inherit correct
locale settings.
In `@e2e/features/market/add.feature`:
- Around line 7-10: The marketplace catalog fixture used by the shared
"catalog-serving" helper is missing required module fields (notably tags), which
can make the "market add" scenario fail; update the fixture generation so each
module object includes name, description, version, repository, and tags (e.g.,
ensure the helper that builds the catalog entries adds a tags array) so the
served "/catalog.yml" conforms to the marketplace schema and the When step
running lola "market add community {server_url}/catalog.yml" operates on valid
data.
In `@e2e/features/steps/cli_steps.py`:
- Around line 13-15: The current code calls shlex.split after resolve_path which
lets backslashes/spaces in interpolated filesystem paths (from resolve_path) be
misinterpreted; change the order to tokenize the original template first using
shlex.split(command) on the un-resolved template, then map resolve_path(context,
arg) over the resulting args so each resolved path remains a single argv item,
and finally call context.cli.run(*args) and assign to context.last_result;
update the block around the variables command, args, resolve_path, shlex.split,
context.cli.run and context.last_result accordingly.
In `@e2e/features/steps/marketplace_steps.py`:
- Around line 22-30: module_entry currently omits the required "tags" field;
update the code that builds module_entry (the dict assigned to module_entry and
appended to modules) to include a "tags" key populated from the input row (e.g.
use row.get("tags") and normalize it to a list: if it's a string split on commas
and strip whitespace, if it's already a list use it, otherwise default to an
empty list) so every generated module contains name, description, version,
repository and tags to match the marketplace schema.
In `@e2e/features/support/cli.py`:
- Around line 80-82: The read_normalized function uses Path.read_text() without
an explicit encoding which can vary across systems; update read_normalized to
call Path.read_text(encoding="utf-8") to force UTF-8 decoding and then normalize
line endings (replace "\r\n" and any remaining "\r" with "\n") before returning;
reference the read_normalized function and the Path.read_text call when making
this change.
In `@e2e/features/support/fixtures.py`:
- Around line 16-18: The fixture markdown files are being written with
Path.write_text(...) without specifying encoding, which can lead to
cross-platform encoding differences; update each write_text call (e.g., the call
that writes SKILL.md in e2e/features/support/fixtures.py and the other
occurrences mentioned) to pass encoding="utf-8" (or switch to an explicit
open(..., encoding="utf-8") context) so the fixture files are written with
explicit UTF-8 encoding.
In `@e2e/features/support/http_server.py`:
- Around line 25-27: LocalHTTPServer.stop() currently only calls
self.server.shutdown(), leaving the listening socket open and the daemon thread
unjoined; update stop() to call self.server.server_close() after shutdown and
join the server thread (the thread created in start()) to ensure the socket/file
descriptor is released, and add explicit -> None type hints to both start() and
stop() signatures; locate the methods by name LocalHTTPServer.start and
LocalHTTPServer.stop to make the changes.
In `@mk/e2e.mk`:
- Around line 25-26: The e2e-container target does not forward the E2E_TAGS
environment variable into the container, so make e2e-container E2E_TAGS='`@smoke`'
has no effect; update the podman run invocation in the e2e-container target to
pass E2E_TAGS into the container (e.g., using -e E2E_TAGS="$(E2E_TAGS)" or
equivalent) so run-tests.sh inside the container can read it; locate the podman
run line in the e2e-container target to change the command.
---
Nitpick comments:
In `@e2e/features/environment.py`:
- Line 10: The Behave hook functions (e.g., before_scenario and after_scenario)
lack explicit return type hints; update their signatures to include modern
Python return annotations (use None return type, e.g., def
before_scenario(context, scenario) -> None:) and apply the same change to the
corresponding hook at line 29 (after_scenario) so both functions use explicit
type hints following the codebase guidelines.
- Around line 29-33: The after_scenario cleanup must guarantee
shutil.rmtree(context.tmp_dir) runs even if a server.stop() raises; change
after_scenario so the server-stop loop is inside a try block and the rmtree call
is in a finally block. Inside the try, iterate over getattr(context,
"http_servers", []) and call server.stop(), but catch and log exceptions from
each server.stop() (do not let one failure escape) so teardown continues; ensure
references to after_scenario, context.http_servers, server.stop(), and
context.tmp_dir are used to locate the code.
In `@e2e/features/install/scope.feature`:
- Around line 1-11: Add a new scenario to e2e/features/install/scope.feature
titled "Install with user scope" that mirrors the existing "Install with project
scope by default" steps (Given a module "my-module" with skills, commands, and
agents; And the module "my-module" is registered) but runs the CLI with the
user/global flag (e.g. run lola "install my-module --global -a claude-code" or
the equivalent -g) and asserts exit code 0 and that the user-target directory
(e.g. "{home}/.claude/skills/skill1") exists instead of the project path; keep
step wording consistent with existing scenario so step definitions
(Given/When/Then) reuse the same step implementations.
In `@e2e/features/install/uninstall.feature`:
- Around line 5-12: Add seeding and assertions for legacy prefixed files to the
"Uninstall a previously installed module" scenario: before the uninstall step
create legacy prefixed files named using the module prefix pattern (e.g.
"{project}/.claude/commands/my-module.testcmd.md" and
"{project}/.claude/agents/my-module.testagent.md" or whatever command/agent
basenames your test suite uses) and after the When step assert those files no
longer exist (similar to the existing "{project}/.claude/skills/skill1" check).
Ensure you use the same scenario name "Uninstall a previously installed module"
and the existing uninstall step When I run lola "uninstall my-module -a
claude-code" so the test verifies removal of "<module>.<cmd>.md" and
"<module>.<agent>.md" legacy filenames for backward compatibility.
In `@e2e/features/market/rm.feature`:
- Around line 10-13: After removing the marketplace you must assert its absence,
not just success text; update the scenario that runs 'When I run lola "market rm
community"' to also run a check like 'When I run lola "market list"' (or an
equivalent step that inspects the marketplace cache) and add an assertion 'Then
the output should not contain "community"' (or assert the cache file/entry for
community is absent). Modify the feature to include the extra step and
expectation so the removal is behaviorally verified rather than only
text-verified.
In `@e2e/features/market/set.feature`:
- Around line 11-23: The scenarios currently only assert CLI output for the
commands 'lola "market set community --disable"' and 'lola "market set community
--enable"' but not the persisted state; after each set command, run the CLI
command that exposes persisted marketplace state (for example 'lola "market
list"' or 'lola "market show community"' / 'lola "market config get community"')
and assert the marketplace is actually marked disabled/enabled in that output or
config; update both scenarios to add these follow-up assertions immediately
after the set commands so the test verifies state persistence, not just
transient text.
In `@e2e/features/mod/add.feature`:
- Around line 15-16: Update the scenario that runs the step "When I run lola
\"mod add {module_path}\"" to include an explicit assertion of the command's
exit code (e.g., add a step like 'Then the exit code should be 1' or whatever
return code the product contract defines for duplicate modules) in addition to
the existing "Then the output should contain \"already exists\"" check so the
test fails if the command returns an unexpected success/failure code.
In `@e2e/features/steps/cli_steps.py`:
- Around line 11-62: Annotate all step functions with Python 3.13 style type
hints: add "from typing import Any" at the top, change signatures to include
context: Any and concrete types for other params (e.g., command: str, code: int,
text: str, pattern: str) and annotate each function return type as -> None
(e.g., def step_run_lola(context: Any, command: str) -> None:). Update the step
functions step_run_lola, step_exit_code, step_output_contains,
step_output_matches, step_run_lola_no_args, and step_output_not_contains
accordingly; you may also annotate locals like args: list[str] if desired, but
ensure the function signatures and return types are added.
In `@e2e/features/steps/marketplace_steps.py`:
- Line 11: Add an explicit return type annotation "-> None" to the step function
signature for step_marketplace_catalog_served so it follows the project typing
guidelines (use modern Python 3.13 hints). Locate the function named
step_marketplace_catalog_served in marketplace_steps.py and change its signature
to include "-> None"; no additional imports are required for this simple None
return annotation.
In `@e2e/features/support/http_server.py`:
- Line 12: Add explicit return type annotations "-> None" to newly added
methods, e.g., update the constructor signature "def __init__(self, directory:
Path)" to include "-> None" and do the same for other new methods referenced in
this change (the method at or around line with symbol names shown in the diff
such as __init__ and the method near line 21). Ensure all new functions follow
the codebase's Python 3.13 typing style (use "-> None" for procedures) without
changing other logic.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 386bfbf7-bdd0-469a-a07f-09cd160bfb82
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (38)
.containerignore.pre-commit-config.yamlMakefiledocs/dev-guide/design/e2e-bdd.mde2e/containers/base/fedora.Containerfilee2e/containers/python.Containerfilee2e/containers/scripts/run-tests.she2e/features/behave.inie2e/features/cli.featuree2e/features/completions.featuree2e/features/environment.pye2e/features/install/install.featuree2e/features/install/list.featuree2e/features/install/scope.featuree2e/features/install/uninstall.featuree2e/features/install/update.featuree2e/features/market/add.featuree2e/features/market/ls.featuree2e/features/market/rm.featuree2e/features/market/set.featuree2e/features/market/update.featuree2e/features/mod/add.featuree2e/features/mod/info.featuree2e/features/mod/init.featuree2e/features/mod/ls.featuree2e/features/mod/rm.featuree2e/features/mod/search.featuree2e/features/mod/update.featuree2e/features/steps/cli_steps.pye2e/features/steps/filesystem_steps.pye2e/features/steps/marketplace_steps.pye2e/features/support/cli.pye2e/features/support/fixtures.pye2e/features/support/http_server.pye2e/features/sync.featuremk/e2e.mkpyproject.tomlsbom.json
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@e2e/containers/python.Containerfile`:
- Around line 9-11: The Dockerfile currently runs an unpinned pip install (the
RUN pip install ... line) which diverges from the pinned versions in
requirements-dev.txt; update the Containerfile to install pinned dependencies
instead by replacing the manual package list with a requirements-based install
(e.g., use pip install -r requirements-dev.txt) or, if runtime deps are declared
in pyproject.toml, copy pyproject.toml/poetry.lock into the image and install
from those files; ensure the RUN pip install line references the pinned
requirements file so container and local dev use identical package versions.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5ea3a6e0-b13d-4bc6-a493-4367a199e756
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (39)
.containerignore.pre-commit-config.yamlMakefiledocs/dev-guide/design/e2e-bdd.mde2e/containers/base/fedora.Containerfilee2e/containers/python.Containerfilee2e/containers/scripts/run-tests.she2e/features/behave.inie2e/features/cli.featuree2e/features/completions.featuree2e/features/environment.pye2e/features/install/install.featuree2e/features/install/list.featuree2e/features/install/scope.featuree2e/features/install/uninstall.featuree2e/features/install/update.featuree2e/features/market/add.featuree2e/features/market/ls.featuree2e/features/market/rm.featuree2e/features/market/set.featuree2e/features/market/update.featuree2e/features/mod/add.featuree2e/features/mod/info.featuree2e/features/mod/init.featuree2e/features/mod/ls.featuree2e/features/mod/rm.featuree2e/features/mod/search.featuree2e/features/mod/update.featuree2e/features/steps/cli_steps.pye2e/features/steps/filesystem_steps.pye2e/features/steps/marketplace_steps.pye2e/features/support/cli.pye2e/features/support/fixtures.pye2e/features/support/http_server.pye2e/features/sync.featuremk/e2e.mkpyproject.tomlrequirements-dev.txtsbom.json
✅ Files skipped from review due to trivial changes (9)
- e2e/features/install/scope.feature
- e2e/features/install/install.feature
- e2e/features/mod/init.feature
- e2e/features/mod/ls.feature
- e2e/features/install/uninstall.feature
- e2e/features/cli.feature
- e2e/features/behave.ini
- sbom.json
- requirements-dev.txt
🚧 Files skipped from review as they are similar to previous changes (27)
- e2e/features/market/add.feature
- e2e/features/market/update.feature
- .pre-commit-config.yaml
- e2e/features/mod/update.feature
- e2e/features/market/ls.feature
- e2e/features/install/update.feature
- e2e/containers/base/fedora.Containerfile
- e2e/features/environment.py
- e2e/features/install/list.feature
- e2e/features/mod/search.feature
- e2e/features/mod/rm.feature
- e2e/features/mod/add.feature
- e2e/features/sync.feature
- e2e/features/market/rm.feature
- e2e/features/mod/info.feature
- e2e/features/steps/marketplace_steps.py
- .containerignore
- e2e/containers/scripts/run-tests.sh
- e2e/features/market/set.feature
- e2e/features/support/http_server.py
- e2e/features/steps/filesystem_steps.py
- pyproject.toml
- Makefile
- e2e/features/support/cli.py
- e2e/features/steps/cli_steps.py
- docs/dev-guide/design/e2e-bdd.md
- e2e/features/support/fixtures.py
Collaborator
|
Wow! This is massive. I like the overall idea and support it. I would just break it into smaller incremental chunks instead of doing it all at once. But I understand this is entirely new and does not touch any breaking change, I'm fine to merge as it is. I will leave my approval here. |
mrbrandao
previously approved these changes
Jun 10, 2026
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
docs/dev-guide/design/e2e-bdd.md (1)
450-462:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMakefile examples include flags not present in actual implementation.
Lines 455, 458, and 461 show
--format progressin the behave commands, but the actual implementation inmk/e2e.mkdoes not include this flag becausebehave.inialready setsformat = progressas the default. Remove--format progressfrom these examples to match the implementation:📝 Proposed fix
e2e: ## Run E2E BDD tests - cd e2e/features && uv run behave --format progress --no-capture + cd e2e/features && uv run behave --no-capture e2e-wip: ## Run only `@wip` tagged E2E scenarios - cd e2e/features && uv run behave --tags="`@wip`" --format progress + cd e2e/features && uv run behave --tags="`@wip`" --no-capture e2e-smoke: ## Run smoke E2E tests (pre-commit) - cd e2e/features && uv run behave --tags="`@smoke`" --format progress + cd e2e/features && uv run behave --tags="`@smoke`" --no-capture🤖 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 `@docs/dev-guide/design/e2e-bdd.md` around lines 450 - 462, The Makefile examples for the e2e targets (e2e, e2e-wip, e2e-smoke) include the redundant flag `--format progress`; update the example lines in the mk/dev.mk snippet by removing `--format progress` from the `uv run behave` invocations so the e2e, e2e-wip, and e2e-smoke targets match the actual implementation (which relies on behave.ini's default `format = progress`).
🤖 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 `@docs/dev-guide/design/e2e-bdd.md`:
- Around line 755-756: The CI example uses a non-existent path "e2e/python" and
should point to the actual BDD directory; update the commands so they cd into
the real directory (e2e/features) or invoke behave against e2e/features (e.g.,
run behave from the repo root targeting e2e/features or cd e2e/features and run
behave .), ensuring the behave flags (--tags, --format, --no-capture) remain
unchanged and that references in the snippet (the behave invocation currently
using ../features) are replaced to reference the actual e2e/features location.
- Around line 596-597: Update the docs snippet in
docs/dev-guide/design/e2e-bdd.md so the container entrypoint example matches the
actual implementation in e2e/containers/scripts/run-tests.sh: change the working
directory to the repository e2e directory (as the script does) and remove the
redundant "../features" path argument when invoking behave, using the same flags
order (--tags="$TAGS" --no-capture --format progress) as in run-tests.sh to keep
the example consistent with the script.
---
Duplicate comments:
In `@docs/dev-guide/design/e2e-bdd.md`:
- Around line 450-462: The Makefile examples for the e2e targets (e2e, e2e-wip,
e2e-smoke) include the redundant flag `--format progress`; update the example
lines in the mk/dev.mk snippet by removing `--format progress` from the `uv run
behave` invocations so the e2e, e2e-wip, and e2e-smoke targets match the actual
implementation (which relies on behave.ini's default `format = progress`).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 53fbb9b0-6829-4a61-9dde-cf6f2866335b
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (39)
.containerignore.pre-commit-config.yamlMakefiledocs/dev-guide/design/e2e-bdd.mde2e/containers/base/fedora.Containerfilee2e/containers/python.Containerfilee2e/containers/scripts/run-tests.she2e/features/behave.inie2e/features/cli.featuree2e/features/completions.featuree2e/features/environment.pye2e/features/install/install.featuree2e/features/install/list.featuree2e/features/install/scope.featuree2e/features/install/uninstall.featuree2e/features/install/update.featuree2e/features/market/add.featuree2e/features/market/ls.featuree2e/features/market/rm.featuree2e/features/market/set.featuree2e/features/market/update.featuree2e/features/mod/add.featuree2e/features/mod/info.featuree2e/features/mod/init.featuree2e/features/mod/ls.featuree2e/features/mod/rm.featuree2e/features/mod/search.featuree2e/features/mod/update.featuree2e/features/steps/cli_steps.pye2e/features/steps/filesystem_steps.pye2e/features/steps/marketplace_steps.pye2e/features/support/cli.pye2e/features/support/fixtures.pye2e/features/support/http_server.pye2e/features/sync.featuremk/e2e.mkpyproject.tomlrequirements-dev.txtsbom.json
✅ Files skipped from review due to trivial changes (12)
- e2e/features/market/update.feature
- e2e/features/mod/rm.feature
- e2e/features/install/install.feature
- e2e/features/behave.ini
- .containerignore
- e2e/features/mod/info.feature
- e2e/features/market/rm.feature
- e2e/containers/base/fedora.Containerfile
- e2e/features/mod/add.feature
- e2e/features/mod/update.feature
- sbom.json
- e2e/features/install/uninstall.feature
🚧 Files skipped from review as they are similar to previous changes (25)
- e2e/features/install/update.feature
- e2e/features/market/add.feature
- e2e/features/mod/search.feature
- e2e/features/install/list.feature
- e2e/features/market/ls.feature
- .pre-commit-config.yaml
- e2e/features/completions.feature
- e2e/features/install/scope.feature
- e2e/features/cli.feature
- e2e/features/mod/ls.feature
- e2e/features/sync.feature
- e2e/features/market/set.feature
- e2e/containers/python.Containerfile
- e2e/features/mod/init.feature
- Makefile
- e2e/containers/scripts/run-tests.sh
- requirements-dev.txt
- e2e/features/environment.py
- e2e/features/steps/marketplace_steps.py
- mk/e2e.mk
- e2e/features/steps/filesystem_steps.py
- e2e/features/support/cli.py
- e2e/features/support/fixtures.py
- e2e/features/steps/cli_steps.py
- e2e/features/support/http_server.py
…ution Defines the BDD testing architecture using Gherkin feature files shared between Python (behave) and Go (godog), with rootless Podman containers for multi-distro Linux testing and native CI runners for macOS/Windows. Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
Scaffolding for black-box CLI testing via behave (Python) with godog (Go) portability. Includes subprocess CLI wrapper with LOLA_HOME isolation, module/marketplace fixture builders, local HTTP server, behave hooks, Makefile targets, and rootless Podman container support. Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
First BDD scenario exercising `lola mod ls` on an empty registry. Introduces cli_steps.py with When/Then steps for CLI invocation, exit code, and output assertions. Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
Introduces filesystem_steps.py with Given steps for module creation and registration. Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
Adds `Then output should match` step for regex assertions. Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
Adds `When I run lola with no arguments` step. Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
Adds `Then directory should exist` step to filesystem_steps.py. Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
Adds `Then directory should not exist` step to filesystem_steps.py. Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
Adds `Given the module is installed to` step to filesystem_steps.py. Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
Introduces marketplace_steps.py with a Given step that starts a local HTTP server serving a YAML catalog. Adds `Then file should exist` step to filesystem_steps.py. Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
Adds `Then output should not contain` step to cli_steps.py. Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
e2e/features/install/install.feature (3)
13-16: ⚡ Quick winConsider adding a negative filesystem assertion.
The error scenario verifies the exit code and error message but doesn't check that the failed installation left no artifacts. Adding an assertion like
And the directory "{project}/.claude/skills" should not existwould catch regressions where the CLI creates partial state even when installation fails.🤖 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 `@e2e/features/install/install.feature` around lines 13 - 16, Add a negative filesystem assertion to the "Install a module that is not registered" scenario: after the existing checks for exit code and output, assert that the installation artifacts directory (e.g., "{project}/.claude/skills" or the CLI's skill directory used by the lola install command) does not exist to ensure no partial state was created; update the scenario (Scenario: Install a module that is not registered) to include a step like 'And the directory "{project}/.claude/skills" should not exist' so the test fails if the CLI leaves behind any files or directories after a failed install.
11-11: ⚡ Quick winConsider parameterizing the hardcoded skill name.
The assertion hardcodes
skill1in the expected directory path. This creates tight coupling with the fixture implementation—if the "Given a module..." step changes the skill name, this assertion breaks.While explicit assertions align with the design goals, consider whether the skill name could be made configurable or documented as a fixture contract to improve maintainability.
🤖 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 `@e2e/features/install/install.feature` at line 11, The step assertion hardcodes the skill directory "skill1"; update the step to use a parameter (e.g., "{skill}" or "{skill_name}") instead of "skill1" so the expected path becomes "{project}/.claude/skills/{skill}" and wire that parameter to the module-creation step (the "Given a module..." step) or a Scenario Outline/Background variable so tests use the same skill name consistently; modify the feature step text and any corresponding step definition that checks the filesystem to accept and use the parameter (look for the step string 'And the directory "{project}/.claude/skills/skill1" should exist' and the matching step implementation) so the skill name is configurable rather than hardcoded.
6-7: Confirm prerequisite Given steps + keep coupling note
Both prerequisite Given steps are defined ine2e/features/steps/filesystem_steps.py(@given('a module "{name}" with skills, commands, and agents')and@given('the module "{name}" is registered')), so the scenario setup steps are present.
- Line 11 hardcodes
skill1in{project}/.claude/skills/skill1, creating fixture coupling; deriving the expected skill name from the module setup would make the test less brittle.🤖 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 `@e2e/features/install/install.feature` around lines 6 - 7, The test step uses a hardcoded skill name "skill1" in the Given steps, creating brittle coupling to the module fixture; update the step implementations in filesystem_steps.py (the `@given`('a module "{name}" with skills, commands, and agents') and `@given`('the module "{name}" is registered') handlers) to derive the expected skill directory/name from the module setup instead of using the literal "skill1" (e.g., read the skills list returned/created by the module setup or construct the skill name from the provided {name} parameter), and update any assertions or path construction that reference "skill1" to use that derived value so the scenario is not tied to a hardcoded fixture.
🤖 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 `@docs/dev-guide/design/e2e-bdd.md`:
- Around line 494-510: Remove the incorrect "python/" listing from the e2e
directory tree and update the comment on the "features/" entry so it explicitly
states that it contains both Gherkin files and the Python behave implementation
(steps/, support/, environment.py, behave.ini); locate the directory tree block
(the e2e/ tree that includes the "features/" and "python/" lines) and edit the
"features/" comment accordingly and delete the "python/" line to match the
actual implementation.
---
Nitpick comments:
In `@e2e/features/install/install.feature`:
- Around line 13-16: Add a negative filesystem assertion to the "Install a
module that is not registered" scenario: after the existing checks for exit code
and output, assert that the installation artifacts directory (e.g.,
"{project}/.claude/skills" or the CLI's skill directory used by the lola install
command) does not exist to ensure no partial state was created; update the
scenario (Scenario: Install a module that is not registered) to include a step
like 'And the directory "{project}/.claude/skills" should not exist' so the test
fails if the CLI leaves behind any files or directories after a failed install.
- Line 11: The step assertion hardcodes the skill directory "skill1"; update the
step to use a parameter (e.g., "{skill}" or "{skill_name}") instead of "skill1"
so the expected path becomes "{project}/.claude/skills/{skill}" and wire that
parameter to the module-creation step (the "Given a module..." step) or a
Scenario Outline/Background variable so tests use the same skill name
consistently; modify the feature step text and any corresponding step definition
that checks the filesystem to accept and use the parameter (look for the step
string 'And the directory "{project}/.claude/skills/skill1" should exist' and
the matching step implementation) so the skill name is configurable rather than
hardcoded.
- Around line 6-7: The test step uses a hardcoded skill name "skill1" in the
Given steps, creating brittle coupling to the module fixture; update the step
implementations in filesystem_steps.py (the `@given`('a module "{name}" with
skills, commands, and agents') and `@given`('the module "{name}" is registered')
handlers) to derive the expected skill directory/name from the module setup
instead of using the literal "skill1" (e.g., read the skills list
returned/created by the module setup or construct the skill name from the
provided {name} parameter), and update any assertions or path construction that
reference "skill1" to use that derived value so the scenario is not tied to a
hardcoded fixture.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 982f6881-0568-4b8c-81d7-3c8676c7fa17
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (39)
.containerignore.pre-commit-config.yamlMakefiledocs/dev-guide/design/e2e-bdd.mde2e/containers/base/fedora.Containerfilee2e/containers/python.Containerfilee2e/containers/scripts/run-tests.she2e/features/behave.inie2e/features/cli.featuree2e/features/completions.featuree2e/features/environment.pye2e/features/install/install.featuree2e/features/install/list.featuree2e/features/install/scope.featuree2e/features/install/uninstall.featuree2e/features/install/update.featuree2e/features/market/add.featuree2e/features/market/ls.featuree2e/features/market/rm.featuree2e/features/market/set.featuree2e/features/market/update.featuree2e/features/mod/add.featuree2e/features/mod/info.featuree2e/features/mod/init.featuree2e/features/mod/ls.featuree2e/features/mod/rm.featuree2e/features/mod/search.featuree2e/features/mod/update.featuree2e/features/steps/cli_steps.pye2e/features/steps/filesystem_steps.pye2e/features/steps/marketplace_steps.pye2e/features/support/cli.pye2e/features/support/fixtures.pye2e/features/support/http_server.pye2e/features/sync.featuremk/e2e.mkpyproject.tomlrequirements-dev.txtsbom.json
✅ Files skipped from review due to trivial changes (13)
- e2e/features/install/scope.feature
- e2e/features/market/update.feature
- e2e/features/install/list.feature
- e2e/features/mod/rm.feature
- e2e/features/mod/ls.feature
- e2e/features/completions.feature
- e2e/features/behave.ini
- e2e/containers/base/fedora.Containerfile
- e2e/features/market/add.feature
- .containerignore
- Makefile
- e2e/features/mod/add.feature
- sbom.json
🚧 Files skipped from review as they are similar to previous changes (23)
- .pre-commit-config.yaml
- pyproject.toml
- e2e/features/mod/update.feature
- e2e/features/market/set.feature
- e2e/containers/python.Containerfile
- e2e/features/cli.feature
- e2e/features/sync.feature
- e2e/features/mod/info.feature
- e2e/features/support/http_server.py
- e2e/features/support/fixtures.py
- e2e/containers/scripts/run-tests.sh
- e2e/features/steps/filesystem_steps.py
- e2e/features/install/uninstall.feature
- requirements-dev.txt
- e2e/features/mod/search.feature
- e2e/features/environment.py
- e2e/features/install/update.feature
- e2e/features/mod/init.feature
- mk/e2e.mk
- e2e/features/steps/marketplace_steps.py
- e2e/features/steps/cli_steps.py
- e2e/features/support/cli.py
- e2e/features/market/rm.feature
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a draft proposal for having e2e testing using BDD/Gherkin.
The goal is to implement e2e testing with BDD for the Python CLI, and allow porting to Go to use the same testing framework.
The PR is a draft until we settle on how it should be implemented.
I'll add examples as de design is polished.
Summary by CodeRabbit
New Features
Tests
Documentation
Chores