Skip to content

Conversation

isoos
Copy link
Collaborator

@isoos isoos commented Sep 12, 2025

  • Part of Create better integration test(s) for search isolate + search handler + search client. #8891. Started using it in fake_server, seems to work, but tests are very flaky with the current code, so submitting only the stable parts and will continue working on the full end-to-end testing in subsequent PRs.
  • Updated the local snapshot loading from an arbitrary URL + created a simple web server for it. While the previous file could have been used too, this reduces the complexity of on-demand updates for search.
  • In the search entrypoint, the new runSearchInstanceController will use a Stream for triggering index reloads. This allows better control than the previous scheduling, and also provides an easy feedback through the completer.

@isoos isoos requested a review from sigurdm September 12, 2025 13:52
final c = Completer();
renewController.add(c);
await c.future;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we need to expect something?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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.

/// an in-memory package index from it.
Future<InMemoryPackageIndex> loadInMemoryPackageIndexFromUrl(String url) async {
late String textContent;
if (url.startsWith('http://') || url.startsWith('https://')) {
Copy link
Contributor

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?

Copy link
Collaborator Author

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 :)

@isoos isoos merged commit 07a9ef1 into dart-lang:master Sep 15, 2025
61 of 62 checks passed
@isoos isoos deleted the search-isolate branch September 15, 2025 08:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants