-
-
Notifications
You must be signed in to change notification settings - Fork 305
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
[feat] Increase test coverage #128
Comments
@awhitford @SirusCodes @hemilpanchiwala any suggestion on this one? |
TBH I'm not really good in testing 😅 but I would try to look into it and add something to the repo👍 |
This is a good question. I started poking around and ended up submitting a couple of pull requests because I found unused code and other opportunities for improvement. However, this lead me to be suspicious of the code coverage statistic. This report, from one of my changes, is a great example. It computes a decrease of code coverage by 10%, but practically every line is covered. The uncovered/red lines are for constructor assertions. I'm wondering if this is a bug or a configuration issue because those lines should have been covered. Beyond this statistic quality question, I noticed that some code is not being covered for various operations like |
I recorded an issue on CodeCov's Community Support: |
From SpinKit, it looks like maybe verifyTickersWereDisposed should be called to validate dispose/cleanup. And likely using pumpAndSettle with a duration will be necessary to allow the animations to completely finish. I like the idea of running widgets twice -- once that doesn't let the animation finish, and again when it does. This will test real-world interruptions and ensure that code cancels and cleans up correctly. |
I experimented with changing Smoke Test to be something like this: testWidgets('Check button smoke test', (WidgetTester tester) async {
// Build our app and trigger a frame.
await tester.pumpWidget(MyApp());
for (var label in labels) {
print('Testing $label');
await tester.idle();
expect(find.text(label), findsOneWidget);
await tester.idle();
await tester.pump();
final pumpCount = await tester.pumpAndSettle();
print(' > $label pumped $pumpCount');
await tester.tap(find.byIcon(Icons.play_circle_filled));
await tester.pump();
}
tester.verifyTickersWereDisposed();
} I think the
As far as I can tell, |
🤓 So |
There are a couple of tweaks (like PR #133), but basically changing testWidgets('Check button smoke test', (WidgetTester tester) async {
// Build our app and trigger a frame.
await tester.pumpWidget(MyApp());
// Exercise each Animation...
for (var label in labels) {
print('Testing $label');
expect(find.text(label), findsOneWidget);
final pumpCount = await tester.pumpAndSettle();
print(' > $label pumped $pumpCount');
await tester.tap(find.byIcon(Icons.play_circle_filled));
await tester.pump();
}
tester.verifyTickersWereDisposed();
}); will raise test coverage by 10.6%. Wavy in particular looks great. Another opportunity is to add unit tests to exercise the Tap & Pause functionality that some animations offer. |
A lot of comments for the follow-up but I think most of the @awhitford 's comments self answers themselves. Great investigation, waiting for your Pull Request. |
PR #135 makes the above mentioned change. Note that this new test code actually highlighted a couple of bugs that were fixed by #133:
|
#157 closes this issue. Thanks, @awhitford 👍 |
Current Codecov test coverage is
69%
, our goal is to increase this coverage to more than 90%. We can take reference fromflutter_spinkit
tests -- https://github.com/jogboms/flutter_spinkit/tree/master/test.The text was updated successfully, but these errors were encountered: