-
Notifications
You must be signed in to change notification settings - Fork 58
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
Changes from 4 commits
03e9a87
5b80d10
8a42ba9
452adae
0fa13a4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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: [ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,12 +22,6 @@ import '../../../test_util/test_util.dart'; | |
|
||
main() { | ||
group('transformed component integration:', () { | ||
test('props class cannot be instantiated directly', () { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why was this test removed? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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(() { | ||
|
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 😢
There was a problem hiding this comment.
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?There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.