chore: clean e2e tests #695
Conversation
|
WalkthroughThis update introduces a centralized constant for all use cases, replacing hardcoded arrays in multiple test suites. It removes environment-dependent logic for framework selection, standardizes test coverage, and adds a new end-to-end test for the "eject" command. Unused imports and obsolete tests are cleaned up for consistency. Changes
Sequence Diagram(s)sequenceDiagram
participant TestRunner
participant CLI
participant AppProcess
participant FileSystem
TestRunner->>CLI: Run create-llama CLI with parameters
CLI->>FileSystem: Generate Next.js project
TestRunner->>AppProcess: Start app process
AppProcess-->>TestRunner: App ready (UI element detected)
TestRunner->>CLI: Run "pnpm run eject"
CLI->>FileSystem: Create "next" directory
TestRunner->>CLI: Install dependencies in "next"
TestRunner->>CLI: Build project in "next"
CLI-->>TestRunner: Build completes
TestRunner->>AppProcess: Terminate app process
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (15)
✨ Finishing Touches
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: 1
🧹 Nitpick comments (3)
packages/create-llama/helpers/types.ts (1)
52-59: Good centralization of use cases, but consider consistent ordering.The centralized
ALL_USE_CASESconstant improves maintainability by providing a single source of truth. However, the ordering differs from theTemplateUseCaseunion type definition above.Consider reordering to match the union type for consistency:
export const ALL_USE_CASES: TemplateUseCase[] = [ - "agentic_rag", - "deep_research", "financial_report", + "deep_research", + "agentic_rag", "code_generator", "document_generator", "hitl", ];packages/create-llama/e2e/typescript/eject.spec.ts (2)
38-38: Inconsistent useLlamaParse configuration.The
useLlamaParseis hardcoded tofalse, but other test files set it based onvectorDb === "llamacloud". This inconsistency might cause test behavior to differ from actual usage patterns.Consider making it consistent with other tests:
- useLlamaParse: false, + useLlamaParse: vectorDb === "llamacloud",
9-9: Consider testing eject with multiple use cases.Currently only testing with
code_generatoruse case. Since eject behavior might vary across different use cases, consider testing with a broader set or at least document why this specific use case was chosen.If eject behavior is use-case dependent, consider parameterizing the test similar to other test files in this PR.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/create-llama/e2e/python/resolve_dependencies.spec.ts(1 hunks)packages/create-llama/e2e/shared/llamaindexserver_template.spec.ts(2 hunks)packages/create-llama/e2e/typescript/eject.spec.ts(1 hunks)packages/create-llama/e2e/typescript/resolve_dependencies.spec.ts(2 hunks)packages/create-llama/helpers/types.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (15)
- GitHub Check: typescript (22, macos-latest, nextjs, llamacloud, llamaindexserver)
- GitHub Check: typescript (22, windows-latest, nextjs, none, llamaindexserver)
- GitHub Check: typescript (22, ubuntu-22.04, nextjs, llamacloud, llamaindexserver)
- GitHub Check: typescript (22, ubuntu-22.04, nextjs, none, llamaindexserver)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, llamacloud, llamaindexserver)
- GitHub Check: typescript (22, macos-latest, nextjs, none, llamaindexserver)
- GitHub Check: typescript (22, windows-latest, nextjs, llamacloud, llamaindexserver)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, none, llamaindexserver)
- GitHub Check: lint
- GitHub Check: python (20, 3.11, macos-latest, fastapi, none, llamaindexserver)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, llamacloud, llamaindexserver)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, none, llamaindexserver)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, llamacloud, llamaindexserver)
- GitHub Check: Unit Tests (ubuntu-latest, 3.9)
- GitHub Check: Unit Tests (windows-latest, 3.9)
🔇 Additional comments (9)
packages/create-llama/e2e/typescript/resolve_dependencies.spec.ts (3)
7-7: Good addition of centralized constant import.Importing
ALL_USE_CASESaligns with the PR objective of centralizing use case definitions.
17-17: Improved test predictability by removing environment dependency.Hardcoding the framework to "nextjs" makes the test behavior more consistent and predictable, eliminating environment-dependent variability.
28-28: Enhanced test coverage with centralized use cases.Using
ALL_USE_CASESensures comprehensive testing across all defined use cases rather than a limited subset.packages/create-llama/e2e/python/resolve_dependencies.spec.ts (3)
7-7: Consistent with centralized constant strategy.Adding the
ALL_USE_CASESimport aligns with the repository-wide standardization effort.
16-16: Appropriate framework choice for Python tests.Hardcoding to "fastapi" makes sense for Python-based testing and removes environment dependency.
28-28: Comprehensive use case coverage.Iterating over
ALL_USE_CASESensures thorough mypy checking across all defined use cases.packages/create-llama/e2e/shared/llamaindexserver_template.spec.ts (2)
2-2: Clean import reorganization.Removing unused
execSyncimport and addingALL_USE_CASESwith proper type imports improves code cleanliness and supports the centralized constant usage.Also applies to: 5-9
23-23: Expanded test coverage with all use cases.Using
ALL_USE_CASESensures comprehensive testing across all defined use cases rather than a limited subset, improving test coverage consistency.packages/create-llama/e2e/typescript/eject.spec.ts (1)
1-18: Well-structured dedicated eject test setup.Good approach to isolate eject functionality testing in its own file with clear constants and configuration.
Summary by CodeRabbit
New Features
Tests
Chores