zulip-flutter icon indicating copy to clipboard operation
zulip-flutter copied to clipboard

lightbox test: Add video player regression tests

Open rajveermalviya opened this issue 1 year ago • 1 comments

Adds tests mentioned in https://github.com/zulip/zulip-flutter/pull/587#discussion_r1597195082

rajveermalviya avatar May 21 '24 16:05 rajveermalviya

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.)

gnprice avatar May 22 '24 19:05 gnprice

Seems pretty decent to me. The tests are written cleverly. Good job!

Just one small comment below.

sm-sayedi avatar May 23 '24 20:05 sm-sayedi

@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.

gnprice avatar May 23 '24 22:05 gnprice

@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.

rajveermalviya avatar May 24 '24 13:05 rajveermalviya

Thanks for the review @sm-sayedi, new revision pushed!

rajveermalviya avatar May 24 '24 19:05 rajveermalviya

Thanks for the review @chrisbobbe, pushed a new revision.

rajveermalviya avatar Jun 06 '24 09:06 rajveermalviya

Thanks, LGTM! Labeling for Greg's review.

chrisbobbe avatar Jun 06 '24 16:06 chrisbobbe

Thanks for the review @gnprice, pushed a new revision, PTAL :)

rajveermalviya avatar Jun 11 '24 21:06 rajveermalviya

Thanks for the review @gnprice, pushed a new revision, PTAL.

rajveermalviya avatar Jun 13 '24 04:06 rajveermalviya

Thanks for the review @gnprice, pushed a new revision!

rajveermalviya avatar Jun 14 '24 05:06 rajveermalviya