Add helper to convert Observatory URI to WS URI #232
Conversation
This is to make it easier to reuse in devtools+devtools_server. See flutter/devtools#563 (comment).
dart/test/utils_test.dart
Outdated
| } | ||
|
|
||
| test('handles http URIs', | ||
| () => check('http://localhost:123/', 'ws://localhost:123/ws')); |
There was a problem hiding this comment.
nit: this formatting is a bit odd looking. I would suggest using () {
check('http://localhost:123/', 'wss://localhost:123/ws');
}
No need to write this code but if you have a similar situation in the future it might make sense to have a
Map<String, String> of input to expected uris and then have a single test case that
iterates through all key-value pairs in the map expecting that the uris match.
There was a problem hiding this comment.
I started it that way, but wasn't sure if having only a single test name without descriptions was good. However, it did turn out a mess, so I've changed it to a Map<String, String>. PTAL!
dart/test/utils_test.dart
Outdated
| import 'package:test/test.dart'; | ||
| import 'package:vm_service_lib/utils.dart'; | ||
|
|
||
| @TestOn('vm') |
There was a problem hiding this comment.
this test should run everywhere, not just on the VM, right?
There was a problem hiding this comment.
Good point, I've removed that.
|
@DanTup, let me know if you're not able to merge this PR. In this PR or a follow-on, we should rev the pubspec and changelog in order to publish this change. |
|
Pushed changes for the comments above, including version increase + changelog. PTAL. @devoncarew I don't have permission to merge here, so please do it if you're both happy. |
| /// for the VM service. | ||
| /// | ||
| /// If the URI is already a VM Service WebSocket URI it will not be modified. | ||
| Uri getVmWsUriFromObservatoryUri(Uri uri) { |
There was a problem hiding this comment.
Note: we may rename this to something like convertToWebSocketUrl({@required String serviceProtocolUrl}).
This is to make it easier to reuse in devtools+devtools_server (see flutter/devtools#563 (comment)). I don't know if putting it in a
utilsfile is the best place, so let me know if I should move this.