-
Notifications
You must be signed in to change notification settings - Fork 2
fix(test builder): add toggle case to per-method entity test generator (#289) #290
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -165,4 +165,67 @@ void main() { | |
| expect(controllerContent, contains('isToggling')); | ||
| expect(controllerContent, contains('_presenter.toggleTodo')); | ||
| }); | ||
|
|
||
| // Regression test for #289: PR #287 added 'toggle' to the entity-methods | ||
| // default used by the di/test plugins (['get', 'update', 'toggle']) so a | ||
| // canonical `zfa make <Entity> --preset=crud --with=vpc,state,di,test` | ||
| // routes through the per-method generators. The test builder's method | ||
| // switch had no `toggle` case, so the test plugin crashed with | ||
| // `Unknown method: toggle` before emitting any file. This locks in the | ||
| // fix: with the test plugin on + toggle in the methods list, generation | ||
| // succeeds AND the per-method toggle usecase test file is emitted with the | ||
| // expected shape. | ||
| test( | ||
| '#289 — toggle test file is generated when test plugin is on (no Unknown method: toggle crash)', | ||
| () async { | ||
| final generator = CodeGenerator( | ||
| config: GeneratorConfig( | ||
| name: 'Todo', | ||
| methods: const ['get', 'update', 'toggle'], | ||
| generateData: true, | ||
| generateLocal: true, | ||
| generateUseCase: true, | ||
| generateVpcs: true, | ||
| generateState: true, | ||
| generateDi: true, | ||
| generateTest: true, | ||
| outputDir: outputDir, | ||
| ), | ||
| outputDir: outputDir, | ||
| options: const GeneratorOptions( | ||
| dryRun: false, | ||
| force: true, | ||
| verbose: false, | ||
| ), | ||
| ); | ||
|
|
||
| final result = await generator.generate(); | ||
| expect( | ||
| result.success, | ||
| isTrue, | ||
| reason: 'Generation crashed: ${result.errors.join('; ')}', | ||
| ); | ||
|
|
||
| // The per-method test builder must emit a toggle-specific test file. | ||
| final toggleTestFile = File( | ||
| '$outputDir/../../test/domain/usecases/todo/toggle_todo_usecase_test.dart', | ||
| ); | ||
| expect( | ||
| toggleTestFile.existsSync(), | ||
| isTrue, | ||
| reason: 'toggle_todo_usecase_test.dart should be generated', | ||
| ); | ||
|
|
||
| final content = toggleTestFile.readAsStringSync(); | ||
|
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧪 Tests · 🟡 Minor · ⚡ Quick win Regression test never compiles the generated test file — it can't catch the type error above The new test asserts The test utilities already expose Proposed fixAfter the final analyze = await runDartAnalyze(workspace, [
'test/domain/usecases/todo/toggle_todo_usecase_test.dart',
]);
expect(analyze.exitCode, 0, reason: analyze.stdout);(plus the equivalent 🤖 Prompt for AI Agents |
||
| // Class name follows the usecase generator's pattern. | ||
| expect(content, contains('ToggleTodoUseCase')); | ||
| // The mock repository must be exercised via its toggle method. | ||
| expect(content, contains('mockRepository.toggle(')); | ||
| // The params constructor must match the usecase generator's signature. | ||
| expect(content, contains('ToggleParams<String, TodoFields>')); | ||
| // The Field constant must come from the entity's Fields class | ||
| // (config.queryField defaults to 'id'). | ||
| expect(content, contains('TodoFields.id')); | ||
| }, | ||
| ); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness · 🟠 Major · 🔨 Medium
Emitted toggle test file does not typecheck —
field: TodoFields.idis aField, not aTodoFieldsThe
togglebranch constructsToggleParams<String, ${entityName}Fields>(id: ..., field: ${entityName}Fields.<queryField>, value: true)(here and again in_getFallbackValuesat line 104).ToggleParams<I, F>declaresfieldas typeF, and the whole generated stack instantiates it asF = ${entityName}Fields. But${entityName}Fields.idis astatic const Field<Entity, IdType>property (see zorphy'sfields_class_generator.dartand real generated entities), andabstract final class TodoFieldshas no instances — so no value of typeTodoFieldscan ever be constructed.dart analyzeon a faithful minimal repro with the realToggleParamsand a real-shapedTodoFieldsfails:Impact: in a real Flutter project,
zfa make <Entity> --preset=crud --with=vpc,state,di,testwith toggle in the methods list now succeeds (no crash — good) but emitstoggle_<entity>_usecase_test.dartthat failsdart analyze. The crash is replaced by silently-broken generated code.Note this mirrors a latent inconsistency in the pre-existing toggle stack (the presenter also types
fieldas${entityName}Fieldswhile the datasource consumesparams.fieldas aFieldviaexisting.copyWithField(params.field, ...)) — so the root fix is upstream: alignToggleParams'sFparameter with the actual field-descriptor type (e.g.Field<Entity, bool>), or provide a constructible field value. Within this PR's scope, at minimum the emitted test should not reference${entityName}Fields.<queryField>as a value.Proposed fix
The cleanest fix is to stop treating the
Fieldsclass as a value type. Either:ToggleParams<I, F>'sFto the field descriptor type and generateToggleParams<String, Field<Todo, bool>>(field: TodoFields.isCompleted, ...)across usecase/presenter/datasource/test — a broader, consistent fix; orBecause
abstract final class TodoFieldshas no instances, option 2 is impossible without a core change — so this needs the upstream alignment (option 1). Verify withdart analyzeon a generated workspace'stest/domain/usecases/after the change.🤖 Prompt for AI Agents