mengw15 commented on PR #5262: URL: https://github.com/apache/texera/pull/5262#issuecomment-4812501757
> Test-isolation fix touching `migration-llm.*` (from the already-merged #5260), added in commit [5797a27](https://github.com/apache/texera/pull/5262/commits/5797a2700c66423b7ef7a940792d2b74cb081a92). > > `migration-llm.spec.ts` stubs the LLM transport by mocking the `ai` module (`vi.mock("ai", …)`). That mock is unreliable under the Angular `unit-test` (Vitest) builder once another spec loads the real `ai` module, and notebook-migration.service.spec.ts (added in this PR) does, transitively: it imports `NotebookMigrationService` → `NotebookMigrationLLM` → `ai`. When the real module wins (scheduling-dependent, green on ubuntu, hung on macOS), `generateText` is unmocked and `convertNotebookToWorkflow` times out on a real network call. It was also emitting the "vi.mock not at top level" warning. > > Fix: route the SDK call through a `callModel` seam on `NotebookMigrationLLM` and stub that in the spec, dropping the `ai/@ai-sdk/openai` module mocks entirely. Stubbing at the class boundary is per-instance and immune to module-load ordering, so it's deterministic across OSes. > > Since #5260 is already merged, this fix necessarily modifies that landed code, so it's part of this PR's diff. I applied the fix here on #5262 to get CI green. Flagging it because it's outside this PR's nominal scope (the notebook-migration orchestration service). @mengw15 do you want it kept here, or pulled into a dedicated follow-up PR against `migration-llm.*`? I lean toward keeping it here — this PR is what surfaces the failure, and CI needs it green — but happy to split it if you'd rather isolate the change. we can keep the change in this pr -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
