Conversation
|
@gemini-cli /review |
There was a problem hiding this comment.
This is the only file that uses dart:io in the app_dart/lib/src folder since its shared between gae_server and local_server.
There was a problem hiding this comment.
This is the "what's missing from non-dart:io" types.
There was a problem hiding this comment.
this is the big "load real data into firestore" chunk, which tries to match dev_cocoon.dart. However, dev_ccoon.dart was very loose with its data. Since we're using a real backend, we have to have commits from flutter and cocoon. This changes the UI and so the screenshots are to be updated.
|
I'm going to move the gemini change out and see if that helps run review. |
Updates the dashboard code to use a "real" server in the browser. Currently, `DevelopmentCocoonService` in `dashboard/lib/service/dev_cocoon.dart` generates fake data manually. While effective, it doesn't test end-to-end with the backend logic or API surface, leading to a false sense of security (and having caused multiple 'push to prod to fix'). By using `IntegrationServer`, we can run the real `app_dart` logic against in-memory fakes (Firestore, BigQuery, etc.), ensuring the frontend is tested against the same logic the production server uses.
1. defangs app_dart/lib/** by removing app_engine and dart:io.
* gae_server.dart passes in the needed "dart:io" handlers
* server.dart now handles an abstract Request, which can be real (app engine, local server) or fake (tests / dashboard local).
* "dynamic config" no longer looks up the config.yaml from the read only filesystem and instead we build dynamic config with build hooks - `generated_config.dart`.
* dynamic_config with compield defaults means some tests need to
ensure the flags they are testing are set correctly.
2. Removes dev_cocoon and its hardcoded mockery (partial reason for the image diffs)
3. Adds `IntegrationServerAdapter` (which is an `AppEngineCocoonService`) to wrap and make fake http calls to the `IntegrationServer`.
|
Fixed failing tests in app_dart: dynamic_config being defaulted to compile-time settings means some tests were assuming the flags were in a different state - fixed. |
|
@gemini-cli /review |
| /// A collection of HTTP status codes. | ||
| /// | ||
| /// This is used instead of 'dart:io' to keep the library platform-neutral. | ||
| class HttpStatus { |
There was a problem hiding this comment.
can we use https://api.flutter.dev/flutter/dart-html/HttpStatus-class.html or something similar instead?
There was a problem hiding this comment.
This core library is deprecated, and scheduled for removal in late 2025. It has been replaced by package:web. The migration guide has more details.
package:web looks like it only supports web: https://pub.dev/packages/web -- via imports of import 'dart:js_interop';
@kevmoo - is there a platform agnostic HttpStatus library?
There was a problem hiding this comment.
Great question! Short answer, no. Although I wonder if we should add one to https://github.com/dart-lang/http/tree/master/pkgs/http
There was a problem hiding this comment.
We should. Consty-data-only types should be platform agnostic :)
3 participants
Updates the dashboard code to use a "real" server in the browser. Currently,
DevelopmentCocoonServiceindashboard/lib/service/dev_cocoon.dartgenerates fake data manually. While effective, it doesn't test end-to-end with the backend logic or API surface, leading to a false sense of security (and having caused multiple 'push to prod to fix'). By usingIntegrationServer, we can run the realapp_dartlogic against in-memory fakes (Firestore, BigQuery, etc.), ensuring the frontend is tested against the same logic the production server uses.generated_config.dart.IntegrationServerAdapter(which is anAppEngineCocoonService) to wrap and make fake http calls to theIntegrationServer.