pkg/shelf: change helper method on Request and Response

pkg/shelf/test/test_change.dart

Index: pkg/shelf/test/test_change.dart
diff --git a/pkg/shelf/test/test_change.dart b/pkg/shelf/test/test_change.dart
new file mode 100644
index 0000000000000000000000000000000000000000..236e81ec5fc849dc0a18ae296823b13db6ec824b
--- /dev/null
+++ b/pkg/shelf/test/test_change.dart
@@ -0,0 +1,56 @@
+// 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 shelf.test_change;
+
+import 'package:unittest/unittest.dart';
+
+import 'package:shelf/src/message.dart';
+
+/// Shared test method used by [Request] and [Response] tests to validate
+/// the behavior of `change` with different `headers` and `context` values.
+void testChange(Message factory({Map<String, String> headers,

[nweiz, 2014/04/29 19:48:48]

Rather than calling this method in request_test and response_test, I think it'd
be cleaner to just have this be its own test file that iterates over the two
classes and defines tests for each.

[kevmoo, 2014/04/30 14:27:52]

I like the idea that these are relatively isolated. We don't run this file. It's
clear if Request or Response test stars to fail what is broken.

This is logically a helper, it just happens to define tests

[nweiz, 2014/04/30 20:21:39]

It's confusing to read. The flow of control bounces back and forth between
files, making it very difficult to follow. It's unclear when looking at the
request/response tests what tests are being run, and it's unclear when looking
at this file what the tests are being run on. It also makes the test files more
coarse-grained which can make debugging more difficult.

When writing a package that exposes a few classes with a lot of functionality,
the pattern is usually to separate test files based on the features those
classes expose. You should do so here rather than bending over backwards to only
have one test file per class.

[kevmoo, 2014/05/02 16:18:01]

Done.

+  Map<String, Object> context})) {
+  test('with empty headers returns indentical instance', () {
+    var request = factory(headers: {'header1': 'header value 1'});
+    var copy = request.change(headers: {});
+
+    expect(copy.headers, same(request.headers));
+  });
+
+  test('with empty context returns identical instance', () {
+    var request = factory(context: {'context1': 'context value 1'});
+    var copy = request.change(context: {});
+
+    expect(copy.context, same(request.context));
+  });
+
+  test('new header values are added', () {
+    var request = factory(headers: {'test':'test value'});
+    var copy = request.change(headers: {'test2': 'test2 value'});
+
+    expect(copy.headers, {'test':'test value', 'test2':'test2 value'});
+  });
+
+  test('existing header values are overwritten', () {
+    var request = factory(headers: {'test':'test value'});
+    var copy = request.change(headers: {'test': 'new test value'});
+
+    expect(copy.headers, {'test':'new test value'});
+  });
+
+  test('new context values are added', () {
+    var request = factory(context: {'test':'test value'});
+    var copy = request.change(context: {'test2': 'test2 value'});
+
+    expect(copy.context, {'test':'test value', 'test2':'test2 value'});
+  });
+
+  test('existing context values are overwritten', () {
+    var request = factory(context: {'test':'test value'});
+    var copy = request.change(context: {'test': 'new test value'});
+
+    expect(copy.context, {'test':'new test value'});
+  });
+}