test: add 54 unit tests for IdeationEngine and Analyzers#518
test: add 54 unit tests for IdeationEngine and Analyzers#518nikolasdehor wants to merge 2 commits intoSynkraAI:mainfrom
Conversation
Cobre IdeationEngine (orquestrador) + PerformanceAnalyzer,
SecurityAnalyzer, CodeQualityAnalyzer, UXAnalyzer e ArchitectureAnalyzer.
Testes incluem:
- calculatePriority: quick-wins, effort multipliers, defaults
- isKnownGotcha: word overlap detection, edge cases
- countByArea: aggregação por área
- formatSuggestion/formatMarkdown: output formatting
- ideate: orquestração, filtragem de gotchas, save
- Analyzers: detecção de sync ops, secrets, eval, vulns, etc.
NOTA: Requer --transform='{}' por falta de babel.config no projeto.
|
@nikolasdehor is attempting to deploy a commit to the Pedro Valério Lopez's projects Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughAdds a comprehensive Jest test suite for the ideation-engine module (new tests covering constructor, analyzers, formatting, orchestration, memory, and save behavior; mocks for execSync, fs, and memory). Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
There was a problem hiding this comment.
Pull request overview
Adiciona uma suíte de testes unitários para o módulo .aios-core/core/ideation/ideation-engine.js, cobrindo o IdeationEngine (orquestração/formatters/salvamento) e validando o comportamento dos analyzers principais sem executar I/O real (mock de execSync e fs).
Changes:
- Cria
tests/core/ideation/ideation-engine.test.jscom cobertura paraIdeationEngine(constructor, calculatePriority, isKnownGotcha, countByArea, formatSuggestion, formatMarkdown, ideate, save). - Inclui testes para
PerformanceAnalyzer,SecurityAnalyzer,CodeQualityAnalyzereArchitectureAnalyzerusandoexecSyncmockado. - Mock de
gotchas-memorypara isolar o carregamento doideation-enginedurante os testes.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| beforeEach(() => { | ||
| jest.clearAllMocks(); | ||
| execSyncSpy.mockReturnValue(''); | ||
| engine = new IdeationEngine({ | ||
| rootPath: '/fake/project', | ||
| gotchasMemory: null, | ||
| }); |
There was a problem hiding this comment.
jest.clearAllMocks() não reseta implementações; como alguns testes alteram fs.existsSync.mockReturnValue(false), esse estado pode vazar para testes seguintes dentro do mesmo describe('IdeationEngine'). Para manter isolamento e evitar flakiness/ordem-dependência, reconfigure explicitamente os retornos padrão dos spies de fs (ex.: existsSync voltar a true) no beforeEach, ou use uma estratégia que resete implementações (com re-spy em seguida).
| engine.analyzers.security.analyze = jest.fn().mockRejectedValue( | ||
| new Error('analysis failed'), | ||
| ); | ||
|
|
||
| const result = await engine.ideate({ focus: 'security', save: false }); | ||
| expect(result.allSuggestions.length).toBe(0); |
There was a problem hiding this comment.
Este teste força um analyze() rejeitar e o IdeationEngine.ideate() vai emitir console.warn(...) (comportamento esperado). Para evitar ruído no output do Jest/CI, vale mockar console.warn localmente (e restaurar depois) ou no beforeEach deste bloco.
| engine.analyzers.security.analyze = jest.fn().mockRejectedValue( | |
| new Error('analysis failed'), | |
| ); | |
| const result = await engine.ideate({ focus: 'security', save: false }); | |
| expect(result.allSuggestions.length).toBe(0); | |
| const originalWarn = console.warn; | |
| console.warn = jest.fn(); | |
| try { | |
| engine.analyzers.security.analyze = jest.fn().mockRejectedValue( | |
| new Error('analysis failed'), | |
| ); | |
| const result = await engine.ideate({ focus: 'security', save: false }); | |
| expect(result.allSuggestions.length).toBe(0); | |
| } finally { | |
| console.warn = originalWarn; | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/core/ideation/ideation-engine.test.js`:
- Around line 13-16: Replace the current null mock with a constructor-shaped
mock so tests catch import/export mismatches: update the jest.mock for
gotchas-memory to return a module object that provides a class constructor named
GotchasMemory and also exposes that constructor as the default export (so
ideation-engine's `new GotchasMemory()` and any default-import usage both
behave). Locate the jest.mock call in the test file (mocking gotchas-memory) and
return an object with GotchasMemory: class { /* empty stub */ } and default: the
same class.
1. Mock gotchas-memory com shape de constructor em vez de null — funciona com import atual (sem destructuring) e com fix SynkraAI#519 (com destructuring). Captura mismatch de import/export. 2. Reconfigurar spies de fs no beforeEach (existsSync, mkdirSync, writeFileSync) para evitar leak de estado entre testes após clearAllMocks limpar as implementações. 3. Suprimir console.warn no teste de analyzer rejection para evitar ruído no output do CI.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/core/ideation/ideation-engine.test.js (1)
386-395: Hardenconsole.warnspy cleanup withtry/finally.If an assertion fails before cleanup,
console.warncan stay mocked and affect later tests. Prefer guaranteed restoration.🔧 Suggested change
- test('captura erros de analyzers sem propagar', async () => { - const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); - engine.analyzers.security.analyze = jest.fn().mockRejectedValue( - new Error('analysis failed'), - ); - - const result = await engine.ideate({ focus: 'security', save: false }); - expect(result.allSuggestions.length).toBe(0); - warnSpy.mockRestore(); - }); + test('captura erros de analyzers sem propagar', async () => { + const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); + try { + engine.analyzers.security.analyze = jest.fn().mockRejectedValue( + new Error('analysis failed'), + ); + + const result = await engine.ideate({ focus: 'security', save: false }); + expect(result.allSuggestions.length).toBe(0); + } finally { + warnSpy.mockRestore(); + } + });As per coding guidelines, "Verify error handling is comprehensive."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/core/ideation/ideation-engine.test.js` around lines 386 - 395, The test "captura erros de analyzers sem propagar" currently mocks console.warn via warnSpy but can leak the mock if an assertion throws; wrap the body that calls engine.ideate and assertions in a try/finally so warnSpy.mockRestore() is always executed. Locate the test block (the test function that sets warnSpy via jest.spyOn and stubs engine.analyzers.security.analyze) and move/make the mockRestore call into a finally clause after the await engine.ideate(...) and expect(...) so console.warn is restored regardless of test outcome.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/core/ideation/ideation-engine.test.js`:
- Around line 386-395: The test "captura erros de analyzers sem propagar"
currently mocks console.warn via warnSpy but can leak the mock if an assertion
throws; wrap the body that calls engine.ideate and assertions in a try/finally
so warnSpy.mockRestore() is always executed. Locate the test block (the test
function that sets warnSpy via jest.spyOn and stubs
engine.analyzers.security.analyze) and move/make the mockRestore call into a
finally clause after the await engine.ideate(...) and expect(...) so
console.warn is restored regardless of test outcome.
Aguardando Dependência@nikolasdehor Este PR depende do #521 (fix: destructuring import GotchasMemory) que ainda precisa de rebase. Uma vez que o #521 for mergeado, este PR pode ser atualizado e mergeado. Status: Aprovado, aguardando resolução da dependência #521. — Gage (DevOps) |
Descrição
Suite de testes abrangente para o módulo
ideation-engine— o maior módulo sem cobertura (832 linhas, 6 classes).Cobertura
Destaques
execSyncmockado viajest.spyOnpara evitar I/O realBugs encontrados
ideation-engine.jsfazrequire('../memory/gotchas-memory')mas o módulo exporta{ GotchasMemory }(named) —new GotchasMemory()falha silenciosamenteNota técnica
Requer
--transform='{}'por ausência debabel.config.jsno projeto (o jest.config padrão usa babel-jest como transform, mas o config não existe). Isso afeta TODOS os testes que carregam módulos em.aios-core/core/ideation/.Summary by CodeRabbit