-
Notifications
You must be signed in to change notification settings - Fork 164
Refactor search isolate control + minimal test. #8954
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 |
---|---|---|
@@ -0,0 +1,59 @@ | ||
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file | ||
// for details. All rights reserved. Use of this source code is governed by a | ||
// BSD-style license that can be found in the LICENSE file. | ||
|
||
import 'dart:async'; | ||
|
||
import 'package:pub_dev/fake/server/fake_search_service.dart'; | ||
import 'package:pub_dev/service/entrypoint/search.dart'; | ||
import 'package:shelf/shelf_io.dart'; | ||
import 'package:test/test.dart'; | ||
|
||
import '../../shared/test_services.dart'; | ||
|
||
void main() { | ||
group('Main search isolate controller', () { | ||
testWithProfile( | ||
'update the package index isolate', | ||
fn: () async { | ||
final snapshotServer = await setupLocalSnapshotServer(); | ||
final renewController = StreamController<Completer>.broadcast(); | ||
final processTerminationCompleter = Completer(); | ||
final handlerStartedCompleter = Completer(); | ||
try { | ||
final port = await _detectFreePort(); | ||
final doneFuture = runSearchInstanceController( | ||
port: port, | ||
renewPackageIndex: renewController.stream, | ||
processTerminationSignal: () async { | ||
handlerStartedCompleter.complete(); | ||
return await processTerminationCompleter.future; | ||
}, | ||
renewWait: Duration.zero, | ||
snapshot: 'http://localhost:${snapshotServer.server.port}/', | ||
); | ||
await handlerStartedCompleter.future; | ||
|
||
// force renew | ||
final c = Completer(); | ||
renewController.add(c); | ||
await c.future; | ||
|
||
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. Don't we need to expect something? 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. At this point the fact that it runs without issues and the future completes is already a good test. I will add search queries and actually changing index at a follow-up pr, when the fake-service is also fixed. |
||
processTerminationCompleter.complete(); | ||
await doneFuture; | ||
} finally { | ||
await snapshotServer.close(); | ||
await renewController.close(); | ||
} | ||
}, | ||
timeout: Timeout.factor(8), | ||
); | ||
}); | ||
} | ||
|
||
Future<int> _detectFreePort() async { | ||
final server = await IOServer.bind('localhost', 0); | ||
final port = server.server.port; | ||
await server.close(); | ||
return port; | ||
} |
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.
Where do you call this with an https url? Or is it preparing for something?
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.
More like completeness. Though, we could make the bucket content available through such URL, which would be
https
... and that would remove the special case here. While the intention was not to prepare for something, it can be :)