Implement SearchEngine.searchAllSubtypes() with the analysis driver.

Implement SearchEngine.searchAllSubtypes() with the analysis driver.

Inline getSubClasses().

Add a couple of tests for SearchEngineImpl2 itself.

R=brianwilkerson@google.com
BUG=

Committed: https://github.com/dart-lang/sdk/commit/9ab69ab9847ce40b0ff207f0040c8c3ca40e9819

[Brian Wilkerson, 2016-12-09 17:03:23]

lgtm

https://codereview.chromium.org/2564913002/diff/1/pkg/analysis_server/test/se...
File pkg/analysis_server/test/services/search/search_engine2_test.dart (right):

https://codereview.chromium.org/2564913002/diff/1/pkg/analysis_server/test/se...
pkg/analysis_server/test/services/search/search_engine2_test.dart:1: //
Copyright (c) 2014, the Dart project authors.  Please see the AUTHORS file
2016?

https://codereview.chromium.org/2564913002/diff/1/pkg/analysis_server/test/se...
pkg/analysis_server/test/services/search/search_engine2_test.dart:28: static
final MockSdk sdk = new MockSdk();
I would have expected that you needed to pass the resource provider into the
MockSdk constructor, and for use with driver I would have expected that you
needed to set generateSummaryFiles to true. I don't know why it works without.

https://codereview.chromium.org/2564913002/diff/1/pkg/analysis_server/test/se...
pkg/analysis_server/test/services/search/search_engine2_test.dart:42:
scheduler.start();
The tests I just added don't start() the scheduler. I invoked start() at one
point, but then the tests never exited, so I commented it out as an experiment
and the tests still passed (and exited), so I concluded that I shouldn't use
start() in tests because of the infinite loop in it. I'm guessing that was
wrong, and that I need to understand when to and when not to use it. (And it
might be good if you took another look at those tests to see whether they were
done the right way.)

[scheglov, 2016-12-09 17:29:58]

https://codereview.chromium.org/2564913002/diff/1/pkg/analysis_server/test/se...
File pkg/analysis_server/test/services/search/search_engine2_test.dart (right):

https://codereview.chromium.org/2564913002/diff/1/pkg/analysis_server/test/se...
pkg/analysis_server/test/services/search/search_engine2_test.dart:1: //
Copyright (c) 2014, the Dart project authors.  Please see the AUTHORS file
On 2016/12/09 17:03:22, Brian Wilkerson wrote:
> 2016?

Done. :-D

https://codereview.chromium.org/2564913002/diff/1/pkg/analysis_server/test/se...
pkg/analysis_server/test/services/search/search_engine2_test.dart:28: static
final MockSdk sdk = new MockSdk();
On 2016/12/09 17:03:23, Brian Wilkerson wrote:
> I would have expected that you needed to pass the resource provider into the
> MockSdk constructor, and for use with driver I would have expected that you
> needed to set generateSummaryFiles to true. I don't know why it works without.

We never access SDK files, from out of SDK, so we don't need them in the same
file system.
The SDK instance is also static, so shared across all of the tests.

The generateSummaryFiles flag does not affect MockSdk.getLinkedBundle(), it
generates the bundle anyway.
We just don't write the bundle to the file system, again because the tests don't
need it there.

https://codereview.chromium.org/2564913002/diff/1/pkg/analysis_server/test/se...
pkg/analysis_server/test/services/search/search_engine2_test.dart:42:
scheduler.start();
On 2016/12/09 17:03:23, Brian Wilkerson wrote:
> The tests I just added don't start() the scheduler. I invoked start() at one
> point, but then the tests never exited, so I commented it out as an experiment
> and the tests still passed (and exited), so I concluded that I shouldn't use
> start() in tests because of the infinite loop in it. I'm guessing that was
> wrong, and that I need to understand when to and when not to use it. (And it
> might be good if you took another look at those tests to see whether they were
> done the right way.)

Hm...
That's weird.
If we don't start the scheduler, then none of the asynchronous AnalysisDriver
request would work.
Because it's the loop in the scheduler that asks drivers to perform pieces of
work.

I updated ContextManagerTest.setUp() to start the scheduler and it does not get
not the infinite loop when I run it separately or as a part of the whole suite
of unit tests.

I would expect the loop in the scheduler to go sleep because its monitor is not
notified, and then the whole scheduler object with its await loop, and the
monitor, and all the rest being garbage collected.

[scheglov, 2016-12-09 17:32:52]

Description was changed from

==========
Implement SearchEngine.searchAllSubtypes() with the analysis driver.

Inline getSubClasses().

Add a couple of tests for SearchEngineImpl2 itself.

R=brianwilkerson@google.com
BUG=
==========

to

==========
Implement SearchEngine.searchAllSubtypes() with the analysis driver.

Inline getSubClasses().

Add a couple of tests for SearchEngineImpl2 itself.

R=brianwilkerson@google.com
BUG=

Committed:
https://github.com/dart-lang/sdk/commit/9ab69ab9847ce40b0ff207f0040c8c3ca40e9819
==========

[scheglov, 2016-12-09 17:32:53]

Committed patchset #1 (id:1) manually as
9ab69ab9847ce40b0ff207f0040c8c3ca40e9819 (presubmit successful).

[Brian Wilkerson, 2016-12-09 18:22:20]

https://codereview.chromium.org/2564913002/diff/1/pkg/analysis_server/test/se...
File pkg/analysis_server/test/services/search/search_engine2_test.dart (right):

https://codereview.chromium.org/2564913002/diff/1/pkg/analysis_server/test/se...
pkg/analysis_server/test/services/search/search_engine2_test.dart:42:
scheduler.start();
Yeah, I was quite surprised that the tests passed without it. Does it need to be
running in order for AnalysisDriver.getResult to return a result? (My
expectation is "yes" but I'm guessing to try to understand.)

[scheglov, 2016-12-09 18:31:10]

https://codereview.chromium.org/2564913002/diff/1/pkg/analysis_server/test/se...
File pkg/analysis_server/test/services/search/search_engine2_test.dart (right):

https://codereview.chromium.org/2564913002/diff/1/pkg/analysis_server/test/se...
pkg/analysis_server/test/services/search/search_engine2_test.dart:42:
scheduler.start();
On 2016/12/09 18:22:20, Brian Wilkerson wrote:
> Yeah, I was quite surprised that the tests passed without it. Does it need to
be
> running in order for AnalysisDriver.getResult to return a result? (My
> expectation is "yes" but I'm guessing to try to understand.)

You're right.
The answer is "yes".