-
Notifications
You must be signed in to change notification settings - Fork 5
Migrate from using unittest package to the test package #5
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
base: master
Are you sure you want to change the base?
Conversation
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.
Thanks a lot for the update, but could you please revert the try-catch as commented?
example/demo.dart
Outdated
test('bad', () => sc.check(badProperty)); | ||
|
||
test('bad', () { | ||
try { |
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.
The purpose of demo.dart
is to show that if you write unit tests that way, they will fail when your property is violated. The goal is for the user to run dart example/demo.dart and to observe the output on the console to get a feeling of what it is like to use the lib. So I'd rather not catch the exceptions.
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.
If I don't catch the error, then the test fails. I figured that was due to the unittest API at the time not having a way to expect an error to be thrown.
I tried expect(sc.check(badProperty), throwsA(equals('falsified after 11 tests\n' ' argument 1: [true]\n' ' argument 2: [false]\n' ''));
But I couldn't get it to work as expected (no pun intended).
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.
The point is that demo.dart
is not a test of the propcheck
library. Instead it is a file meant to showcase the library and its tests fail on purpose to showcase the error messages.
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.
Okay, then I think I did the try-catch before I realized enumerators need to be updated. Once that is done, I can retest this propcheck branch to ensure nothing broke (at least according to the tests).
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.
Okay, this is the error that occurs when I run demo.dart. I added it to a gist with a version of the output from the master branch for comparison.
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.
LGTM except for the nit. Once it's fixed I'll merge the change. Thanks again!
group('smallcheck', () { | ||
final sc = new SmallCheck(depth: 10); | ||
test('good', () => sc.check(goodProperty)); | ||
|
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.
Nit: can you please drop this newline? (Same below.)
Part of a larger effort to migrate to Dart 2, but figured I would separate out the efforts.