test: e2e #687
test: e2e #687
Conversation
|
WalkthroughThe GitHub Actions workflow file was updated to remove the entire Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant GitHub Actions
participant macOS Runner
Developer->>GitHub Actions: Push code / PR
GitHub Actions->>macOS Runner: Trigger e2e-typescript job (macos-latest, --llamacloud, llamaindexserver)
macOS Runner-->>GitHub Actions: Report test results
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (5)
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/e2e.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: typescript (22, 3.11, macos-latest, nextjs, --llamacloud, llamaindexserver)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --llamacloud, llamaindexserver)
- GitHub Check: lint
- GitHub Check: Unit Tests (windows-latest, 3.9)
- GitHub Check: Unit Tests (ubuntu-latest, 3.9)
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
.github/workflows/e2e.yml (1)
88-90:⚠️ Potential issueExpression syntax will break for the
template-typeskey
${{ matrix.template-types }}is invalid because the key contains a hyphen.
GitHub expressions require bracket notation for such keys.- TEMPLATE_TYPE: ${{ matrix.template-types }} + TEMPLATE_TYPE: ${{ matrix['template-types'] }}
♻️ Duplicate comments (3)
.github/workflows/e2e.yml (3)
23-23: OS matrix cut to a single platform – same concern as before
This comment duplicates feedback already given in a previous review about losing Windows + Ubuntu coverage.
25-25: Datasources matrix narrowed – same concern as before
Identical to the earlier comment flagging loss of--no-filesand--example-filescenarios.
26-26: Template types trimmed – same concern as before
Repeats the earlier warning about skipping “streaming” templates.
🧹 Nitpick comments (3)
.github/workflows/e2e.yml (1)
21-21: Loss of Node.js v22 coverageShrinking the matrix to only Node 20 drops validation on the current LTS (v22). If any of the tooling bumps its minimum Node version, we will miss it until production.
Consider keeping both major versions:
- node-version: [20] + node-version: [20, 22]packages/create-llama/e2e/shared/llamaindexserver_template.spec.ts (2)
27-30: Commenting-out three use-cases slashes test coverageRemoving
financial_report,deep_research, andcode_generatormeans we’ll no longer detect regressions specific to those flows. If the runtime cost is the issue, consider selectively running them on a cron schedule rather than dropping them entirely.
90-94: Dead skip conditionsThe
test.skipclause still referencesfinancial_reportanddeep_research, which are now commented out above. This condition will never match and can be removed to reduce noise.- useCase === "financial_report" || - useCase === "deep_research" ||
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/e2e.yml(1 hunks)packages/create-llama/e2e/shared/llamaindexserver_template.spec.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: lint
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --llamacloud, llamaindexserver)
- GitHub Check: Unit Tests (ubuntu-latest, 3.9)
- GitHub Check: Unit Tests (windows-latest, 3.9)
|
Fixed in PR #686 |
Summary by CodeRabbit