tests for keyword/identifier replacementOffset - asserts fixes #23906 and fixes #23947

tests for keyword/identifier replacementOffset - asserts fixes #23906 and fixes #23947

requires these CLs:
https://codereview.chromium.org/1281793004/
https://codereview.chromium.org/1287603003/

R=brianwilkerson@google.com

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

[danrubel, 2015-08-12 01:10:27]

danrubel@google.com changed reviewers:
+ brianwilkerson@google.com

[Brian Wilkerson, 2015-08-12 13:59:25]

LGTM

https://codereview.chromium.org/1278033006/diff/1/pkg/analysis_server/test/do...
File pkg/analysis_server/test/domain_completion_test.dart (right):

https://codereview.chromium.org/1278033006/diff/1/pkg/analysis_server/test/do...
pkg/analysis_server/test/domain_completion_test.dart:402: String completion =
'export';
(I'd actually find it easier to read without the local variable...)

https://codereview.chromium.org/1278033006/diff/1/pkg/analysis_server/test/do...
pkg/analysis_server/test/domain_completion_test.dart:404: relevance:
DART_RELEVANCE_HIGH);
Should we also test for the absence of invalid suggestions?

https://codereview.chromium.org/1278033006/diff/1/pkg/analysis_server/test/do...
pkg/analysis_server/test/domain_completion_test.dart:432: relevance:
DART_RELEVANCE_HIGH);
We should also test for 'export', 'library' and 'part'.

https://codereview.chromium.org/1278033006/diff/1/pkg/analysis_server/test/do...
pkg/analysis_server/test/domain_completion_test.dart:474: relevance:
DART_RELEVANCE_HIGH);
I think we're suggesting 'export' at this point so that if the user backspaces
twice the list will be complete. (If that's not accurate, please let me know.)
If 'export' makes sense, so do 'import', 'part', and 'typedef' (and possibly
'void').

https://codereview.chromium.org/1278033006/diff/1/pkg/analysis_server/test/se...
File pkg/analysis_server/test/services/completion/keyword_contributor_test.dart
(right):

https://codereview.chromium.org/1278033006/diff/1/pkg/analysis_server/test/se...
pkg/analysis_server/test/services/completion/keyword_contributor_test.dart:871:
assertSuggestKeywords([]);
I would have expected that 'assertNoSuggestions()' included keyword suggestions.

https://codereview.chromium.org/1278033006/diff/1/pkg/analysis_server/test/se...
pkg/analysis_server/test/services/completion/keyword_contributor_test.dart:877:
// TODO(danrubel) should not suggest declaration keywords
I'd rather see a test that is correct that is marked as 'fail' than a test with
a TODO to make it correct. I think it will be easier for us to discover later
that there's work to be done.

[danrubel, 2015-08-15 19:17:17]

https://codereview.chromium.org/1278033006/diff/1/pkg/analysis_server/test/do...
File pkg/analysis_server/test/domain_completion_test.dart (right):

https://codereview.chromium.org/1278033006/diff/1/pkg/analysis_server/test/do...
pkg/analysis_server/test/domain_completion_test.dart:402: String completion =
'export';
On 2015/08/12 13:59:25, Brian Wilkerson wrote:
> (I'd actually find it easier to read without the local variable...)

Done.

https://codereview.chromium.org/1278033006/diff/1/pkg/analysis_server/test/do...
pkg/analysis_server/test/domain_completion_test.dart:404: relevance:
DART_RELEVANCE_HIGH);
On 2015/08/12 13:59:25, Brian Wilkerson wrote:
> Should we also test for the absence of invalid suggestions?

Good point. Added.

https://codereview.chromium.org/1278033006/diff/1/pkg/analysis_server/test/do...
pkg/analysis_server/test/domain_completion_test.dart:432: relevance:
DART_RELEVANCE_HIGH);
On 2015/08/12 13:59:25, Brian Wilkerson wrote:
> We should also test for 'export', 'library' and 'part'.

Done.

https://codereview.chromium.org/1278033006/diff/1/pkg/analysis_server/test/do...
pkg/analysis_server/test/domain_completion_test.dart:474: relevance:
DART_RELEVANCE_HIGH);
On 2015/08/12 13:59:25, Brian Wilkerson wrote:
> I think we're suggesting 'export' at this point so that if the user backspaces
> twice the list will be complete.

That is correct.

https://codereview.chromium.org/1278033006/diff/1/pkg/analysis_server/test/se...
File pkg/analysis_server/test/services/completion/keyword_contributor_test.dart
(right):

https://codereview.chromium.org/1278033006/diff/1/pkg/analysis_server/test/se...
pkg/analysis_server/test/services/completion/keyword_contributor_test.dart:871:
assertSuggestKeywords([]);
On 2015/08/12 13:59:25, Brian Wilkerson wrote:
> I would have expected that 'assertNoSuggestions()' included keyword
suggestions.

Yup. That 2nd line is unnecessary. Removed.

https://codereview.chromium.org/1278033006/diff/1/pkg/analysis_server/test/se...
pkg/analysis_server/test/services/completion/keyword_contributor_test.dart:877:
// TODO(danrubel) should not suggest declaration keywords
On 2015/08/12 13:59:25, Brian Wilkerson wrote:
> I'd rather see a test that is correct that is marked as 'fail' than a test
with
> a TODO to make it correct. I think it will be easier for us to discover later
> that there's work to be done.

Fair enough. Added.

[danrubel, 2015-08-15 19:17:57]

Committed patchset #2 (id:20001) manually as
e2f6739b4472b3f25e0d85f74772915834a699b5 (presubmit successful).