zulip-flutter
zulip-flutter copied to clipboard
lightbox test: Add video player regression tests
Adds tests mentioned in https://github.com/zulip/zulip-flutter/pull/587#discussion_r1597195082
Let's start using the "buddy review" system here, now that the start of the official GSoC period is imminent and it seems like everyone's actively engaged again.
@sm-sayedi, would you do the first review on this PR?
As a reminder since we're just starting to use this system: when you get a buddy review request, please prioritize it right after important bugs and your regressions, ahead of most of your own PRs. (In particular I don't think anyone is currently working on an issue that would come ahead of doing buddy reviews.)
Seems pretty decent to me. The tests are written cleverly. Good job!
Just one small comment below.
@sm-sayedi Thanks for that review!
@sumanthvrao I think this is ready for your review as @rajveermalviya's GSoC mentor, now that it's had a first round of buddy review and there's only one small comment outstanding. Please take a look!
Then @sm-sayedi after the thread above is resolved and buddy review is complete, please remove the "buddy review" label and add "maintainer review", and request a review from @chrisbobbe.
@sm-sayedi Thanks for the review, I pushed a new revision which as mentioned in https://github.com/zulip/zulip-flutter/pull/694#discussion_r1613450430 uses Stopwatch to track play/pause time instead of a Timer which entirely removes the concern of manually cancelling the timers in each test.
Thanks for the review @sm-sayedi, new revision pushed!
Thanks for the review @chrisbobbe, pushed a new revision.
Thanks, LGTM! Labeling for Greg's review.
Thanks for the review @gnprice, pushed a new revision, PTAL :)
Thanks for the review @gnprice, pushed a new revision, PTAL.
Thanks for the review @gnprice, pushed a new revision!