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.

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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ dependencies:
logging: ">=0.11.3+1 <1.0.0"
meta: "^1.0.4"
path: "^1.4.1"
react: "^3.4.0"
react: "^3.4.1"
source_span: "^1.4.0"
transformer_utils: "^0.1.1"
w_flux: "^2.7.1"
Expand All @@ -24,6 +24,7 @@ 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"

transformers:
Expand Down
46 changes: 39 additions & 7 deletions test/over_react/component/dom_components_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,51 @@ 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);
const expectedStaticMethods = const<Symbol>[
// DOM
#a, #abbr, #address, #area, #article, #aside, #audio, #b, #base, #bdi, #bdo, #big, #blockquote, #body, #br,
#button, #canvas, #caption, #cite, #code, #col, #colgroup, #data, #datalist, #dd, #del, #details, #dfn, #dialog,
#div, #dl, #dt, #em, #embed, #fieldset, #figcaption, #figure, #footer, #form, #h1, #h2, #h3, #h4, #h5, #h6,
#head, #header, #hr, #html, #i, #iframe, #img, #input, #ins, #kbd, #keygen, #label, #legend, #li, #link, #main,
#map, #mark, #menu, #menuitem, #meta, #meter, #nav, #noscript, #object, #ol, #optgroup, #option, #output, #p,
#param, #picture, #pre, #progress, #q, #rp, #rt, #ruby, #s, #samp, #script, #section, #select, #small, #source,
#span, #strong, #style, #sub, #summary, #sup, #table, #tbody, #td, #textarea, #tfoot, #th, #thead, #time,
#title, #tr, #track, #u, #ul, #variable, #video, #wbr,
// SVG
#svgA, #altGlyph, #altGlyphDef, #altGlyphItem, #animate, #animateColor, #animateMotion, #animateTransform,
#svgAudio, #svgCanvas, #circle, #clipPath, #colorProfile, #cursor, #defs, #desc, #discard, #ellipse, #feBlend,
#feColorMatrix, #feComponentTransfer, #feComposite, #feConvolveMatrix, #feDiffuseLighting, #feDisplacementMap,
#feDistantLight, #feDropShadow, #feFlood, #feFuncA, #feFuncB, #feFuncG, #feFuncR, #feGaussianBlur, #feImage,
#feMerge, #feMergeNode, #feMorphology, #feOffset, #fePointLight, #feSpecularLighting, #feSpotLight, #feTile,
#feTurbulence, #filter, #font, #fontFace, #fontFaceFormat, #fontFaceName, #fontFaceSrc, #fontFaceUri,
#foreignObject, #g, #glyph, #glyphRef, #hatch, #hatchpath, #hkern, #svgIframe, #image, #line, #linearGradient,
#marker, #mask, #mesh, #meshgradient, #meshpatch, #meshrow, #metadata, #missingGlyph, #mpath, #path, #pattern,
#polygon, #polyline, #radialGradient, #rect, #svgScript, #svgSet, #solidcolor, #stop, #svgStyle, #svg,
#svgSwitch, #symbol, #text, #textPath, #svgTitle, #tref, #tspan, #unknown, #use, #svgVideo, #view, #vkern,
];

List<MethodMirror> methods = domClassMirror.staticMembers.values.toList();
List<Symbol> 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.map((m) => m.simpleName).toList();

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

if (methods.isNotEmpty) {
expect(methods, unorderedEquals(expectedStaticMethods), reason: '`expectedStaticMethods` needs to be updated');
}
});

for (var element in methods) {
String name = MirrorSystem.getName(element.simpleName);
for (var element in expectedStaticMethods) {
String name = MirrorSystem.getName(element);
String expectedTagName = name;
if (expectedTagName == 'variable') expectedTagName = 'var';
if (expectedTagName == 'svgSet') expectedTagName = 'set';
Expand All @@ -52,7 +84,7 @@ main() {
if (expectedTagName.startsWith(new RegExp('svg.'))) expectedTagName = expectedTagName.substring(3);

test('Dom.$name generates the correct type', () {
DomProps builder = domClassMirror.invoke(element.simpleName, []).reflectee;
DomProps builder = domClassMirror.invoke(element, []).reflectee;
ReactElement component = builder();
expect(component.type, equalsIgnoringCase(expectedTagName));
});
Expand Down
5 changes: 4 additions & 1 deletion test/over_react/component/resize_sensor_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,9 @@ void main() {

group('common component functionality:', () {
commonComponentTests(ResizeSensor);
});
},
// Mirrors don't work in the DDC.
tags: 'no-ddc'
);
});
}
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
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
5 changes: 4 additions & 1 deletion test/over_react/shared/map_proxy_tests.dart
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,10 @@ void mapProxyTests(Map mapProxyFactory(Map proxiedMap)) {
expect(proxy.values, equals(['value']));
verify(mock.values);
});
});
},
// Mirrors don't work in the DDC.
tags: 'no-ddc'
);
}

class MockMap extends Mock implements Map {}
2 changes: 1 addition & 1 deletion test/over_react/util/dom_util_test.dart
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
10 changes: 8 additions & 2 deletions test/over_react/util/event_helpers_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,10 @@ main() {
expect(syntheticKeyboardEvent.metaKey, isFalse);
expect(syntheticKeyboardEvent.repeat, isFalse);
expect(syntheticKeyboardEvent.shiftKey, isFalse);
});
},
// Mirrors don't work in the DDC.
tags: 'no-ddc'
);

test('wrapNativeMouseEvent', () {
var nativeMouseEvent = new MockMouseEvent();
Expand Down Expand Up @@ -126,7 +129,10 @@ main() {
expect(syntheticMouseEvent.screenX, isNull);
expect(syntheticMouseEvent.screenY, isNull);
expect(syntheticMouseEvent.shiftKey, isFalse);
});
},
// Mirrors don't work in the DDC.
tags: 'no-ddc'
);

test('fakeSyntheticFormEvent', () {
var element = new DivElement();
Expand Down
22 changes: 11 additions & 11 deletions test/over_react/util/handler_chain_util_test.dart
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
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']();
}