Skip to content
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

[video_player] integration test is very flaky on iOS #86915

Closed
stuartmorgan opened this issue Jul 23, 2021 · 5 comments · Fixed by flutter/packages#6899
Closed

[video_player] integration test is very flaky on iOS #86915

stuartmorgan opened this issue Jul 23, 2021 · 5 comments · Fixed by flutter/packages#6899
Labels
a: tests "flutter test", flutter_test, or one of our tests c: flake Tests that sometimes, but not always, incorrectly pass p: video_player The Video Player plugin P2 Important issues not at the top of the work list package flutter/packages repository. See also p: labels. platform-ios iOS applications specifically team Infra upgrades, team productivity, code health, technical debt. See also team: labels. team-ios Owned by iOS platform team triaged-ios Triaged by iOS platform team

Comments

@stuartmorgan
Copy link
Contributor

Recently we're having to re-run the build-ipas+drive_examples shard that contains the video_player plugin pretty frequently. I haven't gone back through logs, but I feel like it's been the last week or so where it's been really bad.

Example failing run: https://cirrus-ci.com/task/4561521528274944

flutter: 00:10 +8 ~1 -1: Some tests failed.
Failure Details:
Failure in method: test video player view with local asset
══╡ EXCEPTION CAUGHT BY FLUTTER TEST FRAMEWORK ╞═════════════════
The following TestFailure object was thrown running a test:
  Expected: <true>
  Actual: <false>
When the exception was thrown, this was the stack:
#4      main.<anonymous closure>.<anonymous closure> (file:///private/var/folders/tn/f_9sf1xx5t14qm_6f83q3b840000gn/T/cirrus-ci-build/packages/video_player/video_player/example/integration_test/video_player_test.dart:222:7)
<asynchronous suspension>
<asynchronous suspension>
(elided one frame from package:stack_trace)
This was caught by the test expectation on the following line:
  file:///private/var/folders/tn/f_9sf1xx5t14qm_6f83q3b840000gn/T/cirrus-ci-build/packages/video_player/video_player/example/integration_test/video_player_test.dart line 222
The test description was:
  test video player view with local asset
═════════════════════════════════════════════════════════════════
end of failure 1
@stuartmorgan stuartmorgan added a: tests "flutter test", flutter_test, or one of our tests team Infra upgrades, team productivity, code health, technical debt. See also team: labels. plugin p: video_player The Video Player plugin P1 High-priority issues at the top of the work list labels Jul 23, 2021
@stuartmorgan
Copy link
Contributor Author

This is definitely still a common source of flake.

The assertion that is failing is: expect(_controller.value.isPlaying, true);.

@stuartmorgan stuartmorgan changed the title video_player integration test is very flaky on iOS [video_player] integration test is very flaky on iOS Aug 26, 2021
stuartmorgan added a commit to stuartmorgan/plugins that referenced this issue Aug 26, 2021
These tests are failing frequently, and interfering with the tree
staying open.

Tracking issues:
flutter/flutter#88837
flutter/flutter#86915
flutter/flutter#86757
stuartmorgan added a commit to flutter/plugins that referenced this issue Aug 26, 2021
These tests are failing frequently, and interfering with the tree
staying open.

Tracking issues:
flutter/flutter#88837
flutter/flutter#86915
flutter/flutter#86757
fotiDim pushed a commit to fotiDim/plugins that referenced this issue Sep 13, 2021
These tests are failing frequently, and interfering with the tree
staying open.

Tracking issues:
flutter/flutter#88837
flutter/flutter#86915
flutter/flutter#86757
amantoux pushed a commit to amantoux/plugins that referenced this issue Sep 27, 2021
These tests are failing frequently, and interfering with the tree
staying open.

Tracking issues:
flutter/flutter#88837
flutter/flutter#86915
flutter/flutter#86757
@stuartmorgan stuartmorgan added platform-ios iOS applications specifically package flutter/packages repository. See also p: labels. labels Feb 9, 2023
ValentinVignal pushed a commit to ValentinVignal/flutter__packages that referenced this issue Feb 23, 2023
These tests are failing frequently, and interfering with the tree
staying open.

Tracking issues:
flutter/flutter#88837
flutter/flutter#86915
flutter/flutter#86757
zhouyuanbo pushed a commit to zhouyuanbo/video_player_2.6.1 that referenced this issue Jun 1, 2023
These tests are failing frequently, and interfering with the tree
staying open.

Tracking issues:
flutter/flutter#88837
flutter/flutter#86915
flutter/flutter#86757
@flutter-triage-bot flutter-triage-bot bot added c: flake Tests that sometimes, but not always, incorrectly pass multiteam-retriage-candidate team-ios Owned by iOS platform team triaged-ios Triaged by iOS platform team labels Jul 7, 2023
tony-hristov pushed a commit to tony-hristov/webview_flutter that referenced this issue Sep 12, 2023
These tests are failing frequently, and interfering with the tree
staying open.

Tracking issues:
flutter/flutter#88837
flutter/flutter#86915
flutter/flutter#86757
@flutter-triage-bot flutter-triage-bot bot removed the triaged-ios Triaged by iOS platform team label Dec 10, 2023
@flutter-triage-bot
Copy link

This issue is marked P1 but has had no recent status updates.

The P1 label indicates high-priority issues that are at the top of the work list. This is the highest priority level a bug can have if it isn't affecting a top-tier customer or breaking the build. Bugs marked P1 are generally actively being worked on unless the assignee is dealing with a P0 bug (or another P1 bug). Issues at this level should be resolved in a matter of months and should have monthly updates on GitHub.

Please consider where this bug really falls in our current priorities, and label it or assign it accordingly. This allows people to have a clearer picture of what work is actually planned. Thanks!

@vashworth vashworth added P2 Important issues not at the top of the work list and removed P1 High-priority issues at the top of the work list labels Dec 11, 2023
@vashworth vashworth added the triaged-ios Triaged by iOS platform team label Dec 11, 2023
@vashworth
Copy link
Contributor

We should reenable this to see if it's still flaky.

cc @hellohuanlin

@stuartmorgan
Copy link
Contributor Author

stuartmorgan commented Jun 10, 2024

I just saw this flake on Android. Maybe there's an issue with the test design.

Edit: looking at this test again, I don't actually see anything here that waits for it to start. It waits for animation to finish, but I don't see any reason to expect that would happen only after the video has loaded. I'm going to rework this and try re-enabling it.

stuartmorgan added a commit to stuartmorgan/packages that referenced this issue Jun 10, 2024
This attempts to address two sources of flake:
- A test that playing doesn't continue past the duration flakily fails,
  at least on Android, with a position a small amount past the duration.
  This seems like an unexpected library behavior that we wouldn't want
  to expose to clients, so rather than change the test, this makes the
  logic that updates the `Value` clamp the position to the duration.
- A test that asset videos play has been restructured to actually wait
  for the future that should start playing to complete before checking
  whether it's playing. The test was previously not actually waiting
  for anything other than animations to complete, and there's no reason
  the placeholder layout couldn't have completed before the asset
  loaded. The fact that the test was already disabled for iOS is strong
  evidence that the flake we are seeing on Android is a problem with
  the test itself, so hopefully this addresses both platforms.

Fixes flutter/flutter#86915
auto-submit bot pushed a commit to flutter/packages that referenced this issue Jun 10, 2024
This attempts to address two sources of flake:
- A test that playing doesn't continue past the duration flakily fails, at least on Android, with a position a small amount past the duration. This seems like an unexpected library behavior that we wouldn't want to expose to clients, so rather than change the test, this makes the logic that updates the `Value` clamp the position to the duration.
- A test that asset videos play has been restructured to actually wait for the future that should start playing to complete before checking whether it's playing. The test was previously not actually waiting for anything other than animations to complete, and there's no reason the placeholder layout couldn't have completed before the asset loaded. The fact that the test was already disabled for iOS is strong evidence that the flake we are seeing on Android is a problem with the test itself, so hopefully this addresses both platforms.

Fixes flutter/flutter#86915
arc-yong pushed a commit to Arctuition/packages-arc that referenced this issue Jun 14, 2024
This attempts to address two sources of flake:
- A test that playing doesn't continue past the duration flakily fails, at least on Android, with a position a small amount past the duration. This seems like an unexpected library behavior that we wouldn't want to expose to clients, so rather than change the test, this makes the logic that updates the `Value` clamp the position to the duration.
- A test that asset videos play has been restructured to actually wait for the future that should start playing to complete before checking whether it's playing. The test was previously not actually waiting for anything other than animations to complete, and there's no reason the placeholder layout couldn't have completed before the asset loaded. The fact that the test was already disabled for iOS is strong evidence that the flake we are seeing on Android is a problem with the test itself, so hopefully this addresses both platforms.

Fixes flutter/flutter#86915
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a: tests "flutter test", flutter_test, or one of our tests c: flake Tests that sometimes, but not always, incorrectly pass p: video_player The Video Player plugin P2 Important issues not at the top of the work list package flutter/packages repository. See also p: labels. platform-ios iOS applications specifically team Infra upgrades, team productivity, code health, technical debt. See also team: labels. team-ios Owned by iOS platform team triaged-ios Triaged by iOS platform team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants