Add broadcast tests #1273
Add broadcast tests #1273
Conversation
julien4215
force-pushed
the
broadcast-test
branch
3 times, most recently
from
d8228ea to
36da34c
Compare
julien4215
force-pushed
the
broadcast-test
branch
from
36da34c to
c1399b5
Compare
There was a problem hiding this comment.
On top of the comments I've made, you should change the broadcast round tests in the following way:
- stay simple, don't test the change of tab so remove both tests that test the change of tab (we don't really care to test the change of tab is working, we should focus first on the feature important logic); instead you can just check the tabs are presents with
find.text) - I assume the
pumpAndSettle()times out because you used a live broadcast source (the clocks are responsible of the timeout) - to simplify things and to improve the tests you should test content with a not yet started broadcast or a finished broadcast: check that the not yet started loads the overview, and the finished loads the boards
- don't check types but check content (text). Example in the overview tab, check that the dates, the location, the broadcast title are there. We don't care about the type of the widget unless if helps to discriminate text that might be present in several places.
- Once you have all the tests working correctly with the finished and upcoming round, you can add a test to test a live round; keep things simple: use a source that only loads one board and just checks that the clock is ticking with a few tester.pump() (we can improve tests later to check live position update is working but leave it to another PR).
- don't leave comments in the code that you don't understand how it works, leave them directly in GitHub.
| await tester.pumpWidget(app); | ||
|
|
||
| await tester.pump(); | ||
| // await tester.pump(); It might be interesting to know why it breaks the test |
There was a problem hiding this comment.
remove the comment, if you have questions or things you don't understand it is better to leave the comment directly in GitHub PR page as long as the review is not done.
| ); | ||
|
|
||
| await tester.pumpWidget(app); | ||
|
|
There was a problem hiding this comment.
It is always interesting to check what is rendered after pumpWidget (usually a loading state).
So when we read the test, we can follow the state transition and understand what is going on.
|
|
||
| expect(find.byKey(const Key('c1-whitebishop')), findsOneWidget); | ||
|
|
||
| await addSocketMessage( |
There was a problem hiding this comment.
remove the helper, and change to:
fakeSocket.addIncomingMessage(...);
await pumpEventQueue();Notice the pumpEventQueue() this is how we ensure the event is delivered to the listener.
There was a problem hiding this comment.
This doesn't work because tests use a different environment for async.
It needs to be wrapped in runAsync but even with this it doesn't work.
| }); | ||
| } | ||
|
|
||
| Future<void> addSocketMessage( |
There was a problem hiding this comment.
as mentioned earlier, remove this helper.
| // Load the round | ||
| await tester.pump(); | ||
|
|
||
| expect(find.byType(BroadcastBoardsTab), findsOneWidget); |
There was a problem hiding this comment.
This is useless, in general we don't want to check widget types (as explained earlier). Unless the check is meaningful, like checking the BoardThumbnail number matches the expected number of boards.
| await tester.pump(); | ||
|
|
||
| // Swipe to the overview tab | ||
| await tester.flingFrom(const Offset(50, 300), const Offset(500, 0), 1000); |
There was a problem hiding this comment.
Don't bother to use a gesture when you can use a tester.tap(find.text('Overview')).
| }); | ||
| } | ||
|
|
||
| final _broadcast = Broadcast( |
There was a problem hiding this comment.
If you have common test data in both files, it is better to put that in another file test/view/broadcast/example_data.dart that you can import from the test files.
No description provided.