Implement analysis.updateOptions

pkg/analysis_server/test/domain_analysis_test.dart

 // Copyright (c) 2014, 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.
 
 library test.domain.analysis;
 
 import 'dart:async';
 
 import 'package:analysis_server/src/computer/computer_highlights.dart';
 import 'package:analysis_server/src/analysis_server.dart';
@@ skipping 95 matching lines @@
           // has been implemented.
           // expect(server.getPriorityFiles(), unorderedEquals(fileList));
         }
 
         setPriorityFiles(['/p1/a.dart', '/p2/b.dart']);
         setPriorityFiles(['/p2/b.dart', '/p2/c.dart']);
         setPriorityFiles([]);
       });
     });
 
-    test('updateOptions', () {
-      var request = new Request('0', ANALYSIS_UPDATE_OPTIONS);
-      request.setParameter(
-          OPTIONS,
-          {
-            'analyzeAngular' : true,
-            'enableDeferredLoading': true,
-            'enableEnums': false
-          });
-      var response = handler.handleRequest(request);
-      // TODO(scheglov) implement
-      expect(response, isNull);
+    group('updateOptions', () {
+      test('invalid', () {
+        var request = new Request('0', ANALYSIS_UPDATE_OPTIONS);
+        request.setParameter(OPTIONS, {'not-an-option' : true});
+        var response = handler.handleRequest(request);
+        expect(response, isResponseFailure('0'));
+      });
+
+      test('valid', () {
+        var request = new Request('0', ANALYSIS_UPDATE_OPTIONS);
+        request.setParameter(
+            OPTIONS,
+            {
+              'analyzeAngular' : true,
+              'enableDeferredLoading': true,
+              'enableEnums': false
+            });
+        var response = handler.handleRequest(request);
+        expect(response, isResponseSuccess('0'));
+        // TODO(brianwilkerson) Figure out how to test that the options were

[Paul Berry, 2014/06/17 20:40:54]

Personally I'd prefer to see this addressed before the CL gets committed.

[Brian Wilkerson, 2014/06/17 21:30:12]

And yet you gave it an LGTM...

I added tests.

[Paul Berry, 2014/06/17 22:24:56]

I assumed that LGTM + comments meant "there's some stuff that I would have done
differently, but I'm not going to be a stickler about it--it's your call whether
to take my advice".  Or maybe I was just being unintentionally vague :)
They look good, thanks.

You might want to consider adding a test to confirm that if options are supplied
before a context is created, they are correctly applied to the newly created
context.  Your call :)

[Brian Wilkerson, 2014/06/18 17:01:30]

That would be good, but I don't know how to find the analysis context that owns
the options that need to be tested. I suppose we can expose that information for
testing purposes, but that goes against my training, so I'm hesitant.

+        // actually changed.
+      });
     });
 
     test('updateSdks', () {
       var request = new Request('0', ANALYSIS_UPDATE_SDKS);
       request.setParameter(
           ADDED,
           ['/dart/sdk-1.3', '/dart/sdk-1.4']);
       request.setParameter(
           REMOVED,
           ['/dart/sdk-1.2']);
@@ skipping 1549 matching lines @@
       // subscribe
       helper.addAnalysisSubscriptionHighlights(helper.testFile);
       // wait, has regions
       return helper.waitForOperationsFinished().then((_) {
         var highlights = helper.getHighlights(helper.testFile);
         expect(highlights, isNot(isEmpty));
       });
     });
   });
 }