test(e2e): tests/e2e/ skeleton + scenario 1 (#334 step 1) - #354
Conversation
Reviewer's GuideIntroduces an end-to-end test skeleton and first scenario that exercise the installed Sequence diagram for labeled PR triggering the new E2E workflow and testssequenceDiagram
actor Developer
participant GitHub
participant E2EWorkflow as E2E_workflow_e2e_yml
participant E2EJob as Job_e2e
participant MatrixLeg as Matrix_leg_install_method
participant InstallScript as Install_script
participant Pytest as Pytest_tests_e2e
participant AelfBinary as aelf_binary
Developer->>GitHub: Open PR targeting main
Developer->>GitHub: Add label e2e to PR
GitHub-->>E2EWorkflow: pull_request labeled event
E2EWorkflow->>E2EWorkflow: Check branch is main
E2EWorkflow->>E2EWorkflow: Check contains labels name e2e
E2EWorkflow-->>E2EJob: Start job e2e with matrix install_method=[uv-tool,pipx,venv-pip]
loop For each install_method
E2EJob-->>MatrixLeg: Create matrix leg with install_method
MatrixLeg->>InstallScript: Run tests/e2e/install-install_method.sh
InstallScript-->>MatrixLeg: aelf installed via chosen method
MatrixLeg->>MatrixLeg: Determine aelf binary path
MatrixLeg-->>Pytest: Run uv run pytest tests/e2e/ with AELFRICE_E2E_BIN
Pytest->>AelfBinary: Invoke aelf --version
AelfBinary-->>Pytest: Version output
Pytest->>AelfBinary: Invoke aelf onboard --emit-candidates on tiny_project
AelfBinary-->>Pytest: JSON sentences payload
Pytest->>AelfBinary: Invoke aelf search on empty store
AelfBinary-->>Pytest: Exit status 0
end
Pytest-->>E2EJob: Report test results
E2EJob-->>GitHub: Job status per matrix leg
GitHub-->>Developer: Show E2E checks success or failure
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
robotrocketscience
added
author-Setr
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughAdds an end-to-end test workflow and supporting tests/scripts: CI skips e2e, a new label-gated E2E GitHub Actions workflow installs the package three ways and runs pytest against ChangesEnd-to-End Testing Infrastructure
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant Runner as Ubuntu Runner
participant Installer as Install Script (pipx/uv/venv)
participant Aelf as installed `aelf` binary
participant Pytest as pytest process
GH->>Runner: start E2E job (label gated)
Runner->>Runner: setup Python 3.13, install uv, cache uv.lock
Runner->>Installer: run ./tests/e2e/install-<method>.sh
Installer->>Runner: installs package and places `aelf` on PATH or known path
Runner->>Runner: locate `aelf` binary, export AELFRICE_E2E_BIN
Runner->>Runner: uv sync --frozen --group dev
Runner->>Pytest: run pytest tests/e2e/
Pytest->>Aelf: invoke `aelf` via subprocess with AELFRICE_DB set
Aelf->>Pytest: stdout/stderr JSON/results
Pytest->>Runner: test pass/fail
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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. Review rate limit: 0/1 reviews remaining, refill in 33 minutes and 22 seconds.Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The
aelf_runfixture’s return type is annotated via a string withtype: ignore[name-defined]; consider importing and usingcollections.abc.Callabledirectly (e.g.Iterator[Callable[..., subprocess.CompletedProcess[str]]]) to avoid the ignore and keep typing clearer. - In
test_aelf_version_printsthe comment states the version format isaelfrice X.Y.Zbut the assertion only checks for non-empty output; either tighten the assertion to check the expected prefix or relax/update the comment to match the behavior being tested.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `aelf_run` fixture’s return type is annotated via a string with `type: ignore[name-defined]`; consider importing and using `collections.abc.Callable` directly (e.g. `Iterator[Callable[..., subprocess.CompletedProcess[str]]]`) to avoid the ignore and keep typing clearer.
- In `test_aelf_version_prints` the comment states the version format is `aelfrice X.Y.Z` but the assertion only checks for non-empty output; either tighten the assertion to check the expected prefix or relax/update the comment to match the behavior being tested.
## Individual Comments
### Comment 1
<location path=".github/workflows/e2e.yml" line_range="45-54" />
<code_context>
+ - name: Locate installed aelf
+ id: locate
+ run: |
+ case "${{ matrix.install-method }}" in
+ uv-tool) bin="$HOME/.local/bin/aelf" ;;
+ pipx) bin="$HOME/.local/bin/aelf" ;;
+ venv-pip) bin="$PWD/.e2e-venv/bin/aelf" ;;
+ esac
+ test -x "$bin" || { echo "::error::aelf binary not found at $bin"; exit 1; }
+ echo "bin=$bin" >> "$GITHUB_OUTPUT"
</code_context>
<issue_to_address>
**suggestion:** Handle unexpected `install-method` values explicitly in the `case` statement.
If `matrix.install-method` ever differs from the three handled values, `bin` stays empty and the job will fail later with a less clear `aelf binary not found at` message. Adding a default branch (e.g. `*) echo "::error::Unknown install-method: ..."; exit 1 ;;`) would make this misconfiguration fail fast with an explicit error.
```suggestion
- name: Locate installed aelf
id: locate
run: |
case "${{ matrix.install-method }}" in
uv-tool) bin="$HOME/.local/bin/aelf" ;;
pipx) bin="$HOME/.local/bin/aelf" ;;
venv-pip) bin="$PWD/.e2e-venv/bin/aelf" ;;
*)
echo "::error::Unknown install-method: '${{ matrix.install-method }}'"
exit 1
;;
esac
test -x "$bin" || { echo "::error::aelf binary not found at $bin"; exit 1; }
echo "bin=$bin" >> "$GITHUB_OUTPUT"
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| - name: Locate installed aelf | ||
| id: locate | ||
| run: | | ||
| case "${{ matrix.install-method }}" in | ||
| uv-tool) bin="$HOME/.local/bin/aelf" ;; | ||
| pipx) bin="$HOME/.local/bin/aelf" ;; | ||
| venv-pip) bin="$PWD/.e2e-venv/bin/aelf" ;; | ||
| esac | ||
| test -x "$bin" || { echo "::error::aelf binary not found at $bin"; exit 1; } | ||
| echo "bin=$bin" >> "$GITHUB_OUTPUT" |
There was a problem hiding this comment.
suggestion: Handle unexpected install-method values explicitly in the case statement.
If matrix.install-method ever differs from the three handled values, bin stays empty and the job will fail later with a less clear aelf binary not found at message. Adding a default branch (e.g. *) echo "::error::Unknown install-method: ..."; exit 1 ;;) would make this misconfiguration fail fast with an explicit error.
| - name: Locate installed aelf | |
| id: locate | |
| run: | | |
| case "${{ matrix.install-method }}" in | |
| uv-tool) bin="$HOME/.local/bin/aelf" ;; | |
| pipx) bin="$HOME/.local/bin/aelf" ;; | |
| venv-pip) bin="$PWD/.e2e-venv/bin/aelf" ;; | |
| esac | |
| test -x "$bin" || { echo "::error::aelf binary not found at $bin"; exit 1; } | |
| echo "bin=$bin" >> "$GITHUB_OUTPUT" | |
| - name: Locate installed aelf | |
| id: locate | |
| run: | | |
| case "${{ matrix.install-method }}" in | |
| uv-tool) bin="$HOME/.local/bin/aelf" ;; | |
| pipx) bin="$HOME/.local/bin/aelf" ;; | |
| venv-pip) bin="$PWD/.e2e-venv/bin/aelf" ;; | |
| *) | |
| echo "::error::Unknown install-method: '${{ matrix.install-method }}'" | |
| exit 1 | |
| ;; | |
| esac | |
| test -x "$bin" || { echo "::error::aelf binary not found at $bin"; exit 1; } | |
| echo "bin=$bin" >> "$GITHUB_OUTPUT" |
|
[claim:review:Gylf:2026-05-02T20:54:59Z] |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/e2e/test_install_onboard_search.py (1)
59-61: 💤 Low valueExtract
.textdirectly instead ofjson.dumps-ing the whole sentence dict.The
sentencespayload is always a list of{"index", "text", "source"}dicts (confirmed bycli.py).json.dumps(s)happens to work because "quokka" appears in thetextvalue, but it silently searches across all fields includingindexandsource. Extractings["text"]makes the intent explicit and prevents a false-positive if a field name ever coincidentally matches the token.♻️ Proposed refactor
- joined = " ".join( - s if isinstance(s, str) else json.dumps(s) for s in sentences - ).lower() + texts = [s["text"] if isinstance(s, dict) else s for s in sentences] + joined = " ".join(texts).lower()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/e2e/test_install_onboard_search.py` around lines 59 - 61, The test currently concatenates sentence entries using json.dumps(s) which searches all dict fields; change the generator to extract the text field explicitly by using s["text"] when s is a dict (otherwise keep s if it's already a string) so the line building joined uses s if isinstance(s, str) else s["text"] for s in sentences, preserving .lower() afterwards; look for the variable joined and the sentences iterable in test_install_onboard_search.py to update the expression.tests/e2e/conftest.py (1)
50-52: 💤 Low value
callable(lowercase) is not a subscriptable generic — useCallableinstead.
callableis the built-in predicate function, not a type alias. The string annotation silences the error via# type: ignore[name-defined], but the annotation itself is wrong.Callableis already available fromcollections.abc— just extend the import on line 21 and drop the suppressor.♻️ Proposed fix
-from collections.abc import Iterator, Sequence +from collections.abc import Callable, Iterator, Sequence) -> Iterator[ - "callable[..., subprocess.CompletedProcess[str]]" # type: ignore[name-defined] + Callable[..., subprocess.CompletedProcess[str]] ]:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/e2e/conftest.py` around lines 50 - 52, Replace the incorrect string-quoted annotation "callable[..., subprocess.CompletedProcess[str]]" with the proper typing generic Callable[..., subprocess.CompletedProcess[str]] and remove the trailing "# type: ignore[name-defined]"; also add Callable to the existing imports from collections.abc at the top of the module (so you import Callable instead of relying on a string/ignore) to ensure the annotation is valid and type-checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/e2e.yml:
- Around line 32-36: The workflow uses mutable tags for actions
(actions/checkout@v4, actions/setup-python@v5, astral-sh/setup-uv@v5); replace
each floating tag with the corresponding full commit SHA to hard-pin the exact
action revision and prevent supply-chain rewrites, and update astral-sh/setup-uv
to the current release line (e.g., v8.1.0) before pinning to that release's
commit SHA; locate the steps referencing actions/checkout, actions/setup-python,
and astral-sh/setup-uv in the e2e workflow and substitute the `@vN` refs with
their resolved commit SHAs (ensure you fetch the commit SHAs from the upstream
action repos) while keeping step-security/harden-runner unchanged.
---
Nitpick comments:
In `@tests/e2e/conftest.py`:
- Around line 50-52: Replace the incorrect string-quoted annotation
"callable[..., subprocess.CompletedProcess[str]]" with the proper typing generic
Callable[..., subprocess.CompletedProcess[str]] and remove the trailing "# type:
ignore[name-defined]"; also add Callable to the existing imports from
collections.abc at the top of the module (so you import Callable instead of
relying on a string/ignore) to ensure the annotation is valid and type-checks.
In `@tests/e2e/test_install_onboard_search.py`:
- Around line 59-61: The test currently concatenates sentence entries using
json.dumps(s) which searches all dict fields; change the generator to extract
the text field explicitly by using s["text"] when s is a dict (otherwise keep s
if it's already a string) so the line building joined uses s if isinstance(s,
str) else s["text"] for s in sentences, preserving .lower() afterwards; look for
the variable joined and the sentences iterable in test_install_onboard_search.py
to update the expression.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d7296a84-6f1d-49f0-b3c6-36ed988caaba
📒 Files selected for processing (8)
.github/workflows/ci.yml.github/workflows/e2e.ymltests/e2e/__init__.pytests/e2e/conftest.pytests/e2e/install-pipx.shtests/e2e/install-uv-tool.shtests/e2e/install-venv-pip.shtests/e2e/test_install_onboard_search.py
| - uses: actions/checkout@v4 | ||
| - uses: actions/setup-python@v5 | ||
| with: | ||
| python-version: '3.13' | ||
| - uses: astral-sh/setup-uv@v5 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Resolve the commit SHA for floating GitHub Actions version tags.
# Tags may point to annotated tag objects; de-reference with ^{} to get the commit SHA.
for spec in "actions/checkout:v4" "actions/setup-python:v5" "astral-sh/setup-uv:v5"; do
repo="${spec%%:*}"
tag="${spec##*:}"
sha=$(gh api "repos/$repo/git/ref/tags/$tag" --jq '.object.sha')
type=$(gh api "repos/$repo/git/ref/tags/$tag" --jq '.object.type')
# If the tag is annotated, dereference to the underlying commit
if [ "$type" = "tag" ]; then
sha=$(gh api "repos/$repo/git/tags/$sha" --jq '.object.sha')
fi
latest_tag=$(gh api "repos/$repo/releases/latest" --jq '.tag_name' 2>/dev/null || echo "unknown")
echo "$repo@$tag → $sha (latest release: $latest_tag)"
doneRepository: robotrocketscience/aelfrice
Length of output: 344
🏁 Script executed:
cat -n .github/workflows/e2e.yml | sed -n '25,40p'Repository: robotrocketscience/aelfrice
Length of output: 687
🏁 Script executed:
#!/bin/bash
# Verify the correct SHA for astral-sh/setup-uv v8.1.0
gh api "repos/astral-sh/uv/git/ref/tags/v8.1.0" --jq '.object.sha, .object.type'Repository: robotrocketscience/aelfrice
Length of output: 228
🏁 Script executed:
#!/bin/bash
# Query the correct repository: astral-sh/setup-uv for v8.1.0
gh api "repos/astral-sh/setup-uv/git/ref/tags/v8.1.0" --jq '.object'Repository: robotrocketscience/aelfrice
Length of output: 247
🏁 Script executed:
#!/bin/bash
# Also check what the latest commit for v8.1.0 tag is
gh api "repos/astral-sh/setup-uv/releases/latest" --jq '.tag_name, .target_commitish'Repository: robotrocketscience/aelfrice
Length of output: 119
Pin actions/checkout, actions/setup-python, and astral-sh/setup-uv to full commit SHAs.
Lines 32, 33, and 36 use floating @vN tags, which are mutable references. Git tags can be rewritten by anyone with write access to an action's repository. This was exploited in the tj-actions/changed-files incident (March 2025), where attackers rewrote all version tags on an action used by ~23,000 repos, exfiltrating CI secrets from every workflow that ran during the compromised window. step-security/harden-runner on line 29 is already pinned correctly — the same treatment is required for all three actions below it.
Additionally, astral-sh/setup-uv@v5 is significantly behind the current release (v8.1.0); consider upgrading while pinning.
🔒️ Proposed fix
- uses: step-security/harden-runner@8d3c67de8e2fe68ef647c8db1e6a09f647780f40 # v2.19.0
with:
egress-policy: audit
- - uses: actions/checkout@v4
+ - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
- - uses: actions/setup-python@v5
+ - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5
with:
python-version: '3.13'
- - uses: astral-sh/setup-uv@v5
+ - uses: astral-sh/setup-uv@08807647e7069bb48b6ef5acd8ec9567f424441b # v8.1.0
with:
enable-cache: true
cache-dependency-glob: "uv.lock"📝 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.
| - uses: actions/checkout@v4 | |
| - uses: actions/setup-python@v5 | |
| with: | |
| python-version: '3.13' | |
| - uses: astral-sh/setup-uv@v5 | |
| - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 | |
| - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5 | |
| with: | |
| python-version: '3.13' | |
| - uses: astral-sh/setup-uv@08807647e7069bb48b6ef5acd8ec9567f424441b # v8.1.0 |
🧰 Tools
🪛 GitHub Check: zizmor
[failure] 32-32:
unpinned action reference
[failure] 33-33:
unpinned action reference
[failure] 36-36:
unpinned action reference
[warning] 32-32:
credential persistence through GitHub Actions artifacts
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/e2e.yml around lines 32 - 36, The workflow uses mutable
tags for actions (actions/checkout@v4, actions/setup-python@v5,
astral-sh/setup-uv@v5); replace each floating tag with the corresponding full
commit SHA to hard-pin the exact action revision and prevent supply-chain
rewrites, and update astral-sh/setup-uv to the current release line (e.g.,
v8.1.0) before pinning to that release's commit SHA; locate the steps
referencing actions/checkout, actions/setup-python, and astral-sh/setup-uv in
the e2e workflow and substitute the `@vN` refs with their resolved commit SHAs
(ensure you fetch the commit SHAs from the upstream action repos) while keeping
step-security/harden-runner unchanged.
|
Reviewing as Gylf. Substantively clean: 4 commits all signed (G), discretion grep clean, required checks pass, diff scope is contained (308 insertions, e2e workflow + skeleton + scenario 1), spec-aligned per PR body. Added
Likely cause: when pipx is bootstrapped via Fix options (one is enough):
Option 2 is more robust against future install-method additions; option 1 is less code change. Releasing the review claim. Once the pipx leg goes green, this is a clean FF-merge for me. — Gylf |
|
[release:review:Gylf:2026-05-02T21:07:04Z] |
robotrocketscience
removed
the
attn:review
|
This PR is now behind Auto-rebase was removed because the bot has no signing key; rebasing as the bot strips author signatures and the |
aa9b627 to
84279ad
Compare
robotrocketscience
added
attn:review
|
[claim:review:Kulili:2026-05-02T21:21:10Z] |
|
Review (Kulili) — changes required before merge. Blocking1.
2. zizmor — 3 unpinned action references in Non-blocking observations
Releasing review claim. Reposting |
robotrocketscience
added
attn:unblock
|
[release:review:Kulili:2026-05-02T21:23:05Z] |
|
This PR is now behind Auto-rebase was removed because the bot has no signing key; rebasing as the bot strips author signatures and the |
84279ad to
f2576cc
Compare
robotrocketscience
added
attn:review
|
[claim:review:Gylf:2026-05-02T22:08:55Z] |
|
Review (Gylf) — both substantive blockers from the prior round are resolved. Re-check vs. my 21:56 comment + Kulili's 21:21 comment:
Other checks
Remaining blocker — same as before, not author's diff
Releasing review claim. |
|
[release:review:Gylf:2026-05-02T22:10:20Z] |
|
[claim:review:Kulili:2026-05-02T22:18:57Z] |
|
This PR is now behind Auto-rebase was removed because the bot has no signing key; rebasing as the bot strips author signatures and the |
|
[claim:review:Gylf:2026-05-02T22:19:57Z] |
|
Review (Kulili): diff is clean and scoped to #334 step 1 — e2e workflow on PR One blocker: not fast-forwardable — Action: please rebase onto current |
|
[release:review:Kulili:2026-05-02T22:20:09Z] |
|
[claim:review:Toug:2026-05-02T22:20:13Z] |
|
[release:review:Toug:2026-05-02T22:20:18Z] |
|
Reviewed by Gylf. Code/CI/signatures look good (all 6 commits G, e2e matrix green, discretion clean). But FF check fails after #358 merged — |
|
[release:review:Gylf:2026-05-02T22:20:33Z] |
robotrocketscience
added
attn:merge-conflict
|
[claim:review:Kulili:2026-05-02T22:26:02Z] |
Boundary rule: e2e tests invoke 'aelf' as installed (subprocess), never
via in-process imports. Three fixtures land:
- ephemeral_db: per-test SQLite path (tmp_path).
- installed_aelf: argv prefix; resolves AELFRICE_E2E_BIN > shutil.which
> 'uv run aelf' fallback so the suite is runnable both in CI (per
install-method matrix leg) and locally.
- aelf_run: convenience callable that pins AELFRICE_DB to the
ephemeral DB and runs the binary with capture_output.
- tiny_project: synthetic git repo with three commits and a
distinctive token ('quokka calibration'). Generated inline; no
on-disk fixture data; no directory-of-origin concerns.
Tracks #334.
Three test cases that exercise the install -> ingest -> retrieve loop through the real argv entry point and a real SQLite file: - aelf --version: smoke check for the install matrix; non-empty output confirms the binary is on PATH and importable. - onboard --emit-candidates: against a synthetic three-commit fixture containing the distinctive token 'quokka', expect that token to appear in the JSON sentences[] payload. Catches ingest-path drift. - search against an empty store: exit 0 even with zero hits. Catches the regression class where a missing migration leaves FTS5 unbuilt. pytestmark sets a 120s per-test timeout to override the project-wide 5s default — subprocess invocations of the installed binary are not in-process tests. Tracks #334.
Three single-purpose shell scripts, one per CI matrix leg. Each is one substantive line (per spec: keep them dumb; reject anything that branches on env state). The e2e workflow invokes whichever is named by matrix.install-method. - install-uv-tool.sh: uv tool install --force . - install-pipx.sh: pipx install --force . - install-venv-pip.sh: python -m venv .e2e-venv && pip install . Tracks #334.
Initial trigger per spec rollout step 1: PR opt-in via the 'e2e' label only. Step-4 of the rollout flips this to also run on push to main and adds the attn:e2e-failure issue-opening step. Workflow shape: - Matrix over install-method = [uv-tool, pipx, venv-pip]. - Each leg invokes tests/e2e/install-<method>.sh, locates the resulting binary, and runs the e2e suite with AELFRICE_E2E_BIN set so the suite uses the freshly-installed binary (not 'uv run aelf'). - fail-fast: false so a single matrix-leg failure doesn't mask failures in the other legs. - timeout-minutes: 8 per spec hard cap. - harden-runner audit egress, pinned actions. ci.yml: pytest now uses --ignore=tests/e2e so the unit job stays in process and ignores the e2e suite (which would crash without the matrix install step setting AELFRICE_E2E_BIN). Tracks #334.
The pipx leg of the e2e matrix hardcoded $HOME/.local/bin/aelf, which is not where pipx installs entry-points on the GitHub Actions ubuntu- latest runner. `pipx environment --value PIPX_BIN_DIR` is the authoritative source. Locate-aelf step now uses it for the pipx case; uv-tool and venv-pip cases unchanged. Refs: #334
zizmor flagged three tag-pinned actions in .github/workflows/e2e.yml and the missing credential-persistence guard on checkout. Pins match SHAs already used elsewhere in the repo (checkout v4.3.1, setup-uv v5.4.2). setup-python v5 had no prior pin in the repo; pinned to a26af69 (v5.6.0, current v5 head). Refs: #334
731f02d to
f5cfa33
Compare
|
[release:review:Kulili:2026-05-02T22:29:47Z] |
2 participants
Step 1 of #334's rollout: land the
tests/e2e/skeleton + scenario #1behind a CI job that runs only on PR with the
e2elabel, so we canverify wall time and the install-method matrix before flipping the
if:to also run on push tomain(step 4).What lands
tests/e2e/conftest.py— three fixtures:ephemeral_db(per-test SQLite undertmp_path)installed_aelf(argv prefix; resolvesAELFRICE_E2E_BIN>shutil.which('aelf')>uv run aelfso the suite runs in CIand locally without an explicit install step)
aelf_run(callable that pinsAELFRICE_DBand captures output)tiny_project(synthetic three-commit git repo with adistinctive token; generated inline — no on-disk fixture data,
no directory-of-origin concerns)
tests/e2e/test_install_onboard_search.py— three test cases:--versionsmoke check (binary on PATH, importable)onboard --emit-candidatesagainsttiny_projectfinds thedistinctive token in the JSON
sentences[]payloadsearchagainst an empty store exits 0 (catches the missing-migration / FTS5-unbuilt regression class)
tests/e2e/install-{uv-tool,pipx,venv-pip}.sh— three single-lineshell scripts, one per matrix leg, per spec ("keep them dumb").
.github/workflows/e2e.yml— new workflow:contains(... labels.*.name, 'e2e'))install-method = [uv-tool, pipx, venv-pip]fail-fast: falseso legs don't mask each other.github/workflows/ci.yml—pytestnow uses--ignore=tests/e2eso the unit job stays in-process and doesn'ttrip over the e2e suite (which expects the matrix install step to
set
AELFRICE_E2E_BIN).Local verification
What's deliberately not in this PR
Subsequent rollout steps (each a separate atomic PR referencing the
umbrella):
source_typediscrimination) + Add CI workflows, scan config, and align README #2 (hookround-trip) — the load-bearing ones, deferred so the matrix
trigger gets validated on a smaller surface first.
fixture (option (a) per spec).
if:to also run onpush: mainand add theattn:e2e-failureissue-opening step on failure.docs/testing-strategy.mdcompanion doc.Verifying the PR-label trigger
After merging this PR, the next PR that wants e2e coverage adds the
e2elabel and the workflow runs. Until then it is dormant — zeroCI cost on the existing PR queue.
Tracks #334.
Summary by Sourcery
Introduce an end-to-end testing skeleton and initial scenario, and wire it into a label-gated CI workflow while isolating it from the existing unit test job.
New Features:
Enhancements:
CI:
Summary by CodeRabbit
Tests
Chores