Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UIP-2413: Get tests passing with the DDC #86

Merged
merged 5 commits into from
Jun 27, 2017
Merged

UIP-2413: Get tests passing with the DDC #86

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
03e9a87
Use over_react_test, rip out duplicated test utils
greglittlefield-wf Jun 13, 2017
5b80d10
Merge remote-tracking branch 'greg/use_over_react_test' into UIP-2413…
jacehensley-wf Jun 22, 2017
8a42ba9
Get tests passing with DDC
jacehensley-wf Jun 22, 2017
452adae
Address CR comments
jacehensley-wf Jun 23, 2017
0fa13a4
Point to released react-dart
jacehensley-wf Jun 23, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,15 @@ dev_dependencies:
coverage: "^0.7.2"
dart_dev: "^1.7.6"
mockito: "^0.11.0"
over_react_test: "^1.0.0"
test: "^0.12.6+2"

dependency_overrides:
react:
git:
url: [email protected]:clairesarsam-wf/react-dart.git
ref: 9e3a349fd6765afb4531192c21a5e7bc74c5d7a0

transformers:
- over_react:
$exclude: [
Expand Down
17 changes: 11 additions & 6 deletions test/over_react/component/dom_components_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,20 @@ import 'package:over_react/over_react.dart';
import 'package:react/react_client.dart';
import 'package:test/test.dart';


main() {
group('Dom component:', () {
ClassMirror domClassMirror = reflectClass(Dom);

List<MethodMirror> methods = domClassMirror.staticMembers.values.toList();
List<MethodMirror> methods = [];
ClassMirror domClassMirror;

setUpAll(() {
expect(methods, isNotEmpty, reason: 'should have properly reflected the static members.');
domClassMirror = reflectClass(Dom);

// staticMembers is not implemented for the DDC and will throw is this test is loaded even if it's not run.
try {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't need this try-catch, then, if it's not run in the DDC, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It throws even if the test isn't run 😢

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, the contents of setUpAll are evaluated even when the group is disabled? Is that a test package bug?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if it's evaluated, I think the DDC might just be checking for instances of that being used, because it throws if it's in a test too

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But if I use skip: it's fine. So maybe it is a test package bug.

methods = domClassMirror.staticMembers.values.toList();

expect(methods, isNotEmpty, reason: 'should have properly reflected the static members.');
} catch(e) {}
});

for (var element in methods) {
Expand All @@ -57,5 +62,5 @@ main() {
expect(component.type, equalsIgnoringCase(expectedTagName));
});
}
});
}, tags: 'no-ddc');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this disabled in the DDC? Should have a comment

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't use staticMethod. I'll add a comment

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, we should really still test these in the DDC if possible.

What about if we enumerated these, and just checked them against the reflected values, like so?

const expectedStaticMethods = const <Symbol> [#div, #span, ...];
var actualMethods;
try {
  actualMethods = domClassMirror.staticMembers.values.map((m) => m.simpleName).toList();
} catch(e) {}
if (actualMethods != null) {
  expect(actualMethods, unorderedEquals(expectedStaticMethods),
      reason: '`expectedStaticMethods` needs to be updated');
}

for (var method in expectedStaticMethods) {
  String name = MirrorSystem.getName(method);
...

}
2 changes: 1 addition & 1 deletion test/over_react/component/resize_sensor_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,6 @@ void main() {

group('common component functionality:', () {
commonComponentTests(ResizeSensor);
});
}, tags: 'no-ddc');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this disabled in the DDC? Should have a comment

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't use instanceMembers in the DDC. I'll add a comment

});
}
2 changes: 1 addition & 1 deletion test/over_react/component_declaration/component_base_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ main() {
test('a single child is passed in', () {
var child = 'Only child';
var renderedNode = renderAndGetDom(Dom.div()(child));
List<Text> children = renderedNode.childNodes.where((node) => node.nodeType != Node.COMMENT_NODE).toList();
var children = new List<Text>.from(renderedNode.childNodes.where((node) => node.nodeType != Node.COMMENT_NODE));

expect(children.length, equals(1));
expect(children[0].data, equals(child));
Expand Down
6 changes: 0 additions & 6 deletions ...react/component_declaration/transformer_integration_tests/component_integration_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,6 @@ import '../../../test_util/test_util.dart';

main() {
group('transformed component integration:', () {
test('props class cannot be instantiated directly', () {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was this test removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prop classes are made abstract by the transformer and the DDC was failing on instantiating an abstract object.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And this test was leftover from before that was the case, so it's not really necessary anymore

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

expect(() {
new ComponentTestProps();
}, throwsA(const isInstanceOf<AbstractClassInstantiationError>()));
});

test('component class can be instantiated directly', () {
var instance;
expect(() {
Expand Down
2 changes: 1 addition & 1 deletion test/over_react/shared/map_proxy_tests.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ void mapProxyTests(Map mapProxyFactory(Map proxiedMap)) {
expect(proxy.values, equals(['value']));
verify(mock.values);
});
});
}, tags: 'no-ddc');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this disabled in the DDC? Should have a comment

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mocks don't work in the DDC. I'll add a comment

}

class MockMap extends Mock implements Map {}
2 changes: 1 addition & 1 deletion test/over_react/util/dom_util_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ main() {
test('root contains the other element', () {
var rootInstance = render(DomTest());
var rootNode = findDomNode(rootInstance);
var otherNode = getDomByTestId(rootInstance, 'innerComponent');
var otherNode = getComponentRootDomByTestId(rootInstance, 'innerComponent');

expect(isOrContains(rootNode, otherNode), isTrue);
});
Expand Down
4 changes: 2 additions & 2 deletions test/over_react/util/event_helpers_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ main() {
expect(syntheticKeyboardEvent.metaKey, isFalse);
expect(syntheticKeyboardEvent.repeat, isFalse);
expect(syntheticKeyboardEvent.shiftKey, isFalse);
});
}, tags: 'no-ddc');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this disabled in the DDC? Should have a comment


test('wrapNativeMouseEvent', () {
var nativeMouseEvent = new MockMouseEvent();
Expand Down Expand Up @@ -126,7 +126,7 @@ main() {
expect(syntheticMouseEvent.screenX, isNull);
expect(syntheticMouseEvent.screenY, isNull);
expect(syntheticMouseEvent.shiftKey, isFalse);
});
}, tags: 'no-ddc');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this disabled in the DDC? Should have a comment


test('fakeSyntheticFormEvent', () {
var element = new DivElement();
Expand Down
22 changes: 11 additions & 11 deletions test/over_react/util/handler_chain_util_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ main() {
/// Shared tests for [CallbackUtil] subclasses supporting different arities.
///
/// Expects callback arguments to be typed to [TestGenericType].
void sharedTests(CallbackUtil callbackUtil, int arity) {
void sharedTests<T extends Function>(CallbackUtil<T> callbackUtil, int arity) {
List generateArgs() {
return new List.generate(arity, (_) => new TestGenericType());
}
Expand Down Expand Up @@ -158,7 +158,7 @@ main() {
test('calls all functions in order', () {
var calls = [];

List<Function> functions = new List.generate(5, (index) {
List functions = new List.generate(5, (index) {
return createTestChainFunction(onCall: (args) {
calls.add(['function_$index', args]);
});
Expand All @@ -180,7 +180,7 @@ main() {
});

test('returns false when any function returns false', () {
List<Function> functions = new List.generate(5, (_) => createTestChainFunction());
List functions = new List.generate(5, (_) => createTestChainFunction());
functions.insert(2, createTestChainFunction(returnValue: false));

var chained = callbackUtil.chainFromList(functions);
Expand All @@ -189,7 +189,7 @@ main() {
});

test('returns null when no function returns false', () {
List<Function> functions = new List.generate(5, (_) => createTestChainFunction());
List functions = new List.generate(5, (_) => createTestChainFunction());

var chained = callbackUtil.chainFromList(functions);

Expand All @@ -201,7 +201,7 @@ main() {
test('null functions', () {
var calls = [];

List<Function> functions = new List.generate(5, (index) {
List functions = new List.generate(5, (index) {
return createTestChainFunction(onCall: (args) {
calls.add(['function_$index', args]);
});
Expand All @@ -227,7 +227,7 @@ main() {
});

test('an empty list of functions', () {
var chained = callbackUtil.chainFromList([]);
var chained = callbackUtil.chainFromList(<T>[]);

expect(chained, const isInstanceOf<Function>());
expect(() => Function.apply(chained, generateArgs()), returnsNormally);
Expand All @@ -236,7 +236,7 @@ main() {

if (arity != 0) {
test('has arguments typed to the specified generic parameters', () {
List<Function> functions = new List.generate(5, (_) => createTestChainFunction());
List functions = new List.generate(5, (_) => createTestChainFunction());

functions.forEach((function) {
expect(() => Function.apply(function, generateArgs()), returnsNormally,
Expand Down Expand Up @@ -264,19 +264,19 @@ main() {
}

group('CallbackUtil0Arg', () {
sharedTests(const CallbackUtil0Arg(), 0);
sharedTests<Callback0Arg>(const CallbackUtil0Arg(), 0);
});

group('CallbackUtil1Arg', () {
sharedTests(const CallbackUtil1Arg<TestGenericType>(), 1);
sharedTests<Callback1Arg>(const CallbackUtil1Arg<TestGenericType>(), 1);
});

group('CallbackUtil2Arg', () {
sharedTests(const CallbackUtil2Arg<TestGenericType, TestGenericType>(), 2);
sharedTests<Callback2Arg>(const CallbackUtil2Arg<TestGenericType, TestGenericType>(), 2);
});

group('CallbackUtil3Arg', () {
sharedTests(const CallbackUtil3Arg<TestGenericType, TestGenericType, TestGenericType>(), 3);
sharedTests<Callback3Arg>(const CallbackUtil3Arg<TestGenericType, TestGenericType, TestGenericType>(), 3);
});
});
});
Expand Down
9 changes: 9 additions & 0 deletions test/over_react/util/react_wrappers_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1133,3 +1133,12 @@ class PlainObjectPropsMap {
class PlainObjectStyleMap {
external get width;
}

/// Helper component that renders whatever you tell it to. Necessary for rendering components with the 'ref' prop.
final RenderingContainerComponentFactory =
react.registerComponent(() => new RenderingContainerComponent()) as ReactComponentFactory; // ignore: avoid_as

class RenderingContainerComponent extends react.Component {
@override
render() => props['renderer']();
}
Loading