Page MenuHomePhabricator
Create Task
Maniphest T392974

Make sure FFMPEG is killed if screenshot fails
Closed, ResolvedPublic
Actions

Description

There are issues where ffmpeg isn't killed in CI see https://phabricator.wikimedia.org/T324766#10778643

One thing to make our code safer is to catch taking a screenshot fails and then always try to kill ffmpeg.

Details

Related Changes in Gerrit:
SubjectRepoBranchLines +/-
selenium: set explicit timeout when spawning ffmpegmediawiki/coremaster+23 -11
selenium: Verify that ffmpeg really exitsmediawiki/coremaster+21 -2
selenium: Do not hide screenshots errorsmediawiki/coremaster+8 -0
selenium: Always try to stop FFMPEG.mediawiki/coremaster+5 -2
selenium: log the actual ffmpeg error.mediawiki/coremaster+2 -2
Customize query in gerrit

Related Objects

Event Timeline

Peter created this task.Apr 30 2025, 5:44 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 30 2025, 5:44 AM
gerritbot added a comment.Apr 30 2025, 5:47 AM

Change #1139980 had a related patch set uploaded (by Phedenskog; author: Phedenskog):

[mediawiki/core@master] Always try to kill ffmpeg.

https://gerrit.wikimedia.org/r/1139980

gerritbot added a project: Patch-For-Review.Apr 30 2025, 5:48 AM
gerritbot added a comment.Apr 30 2025, 11:47 AM

Change #1139943 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/core@master] selenium: set explicit timeout when spawning ffmpeg

https://gerrit.wikimedia.org/r/1139943

Daimona subscribed.Apr 30 2025, 11:48 AM

I had also made the patch above (set a timeout) for T392963. I think it would be nice to have both safeguards in place.

Daimona mentioned this in T392963: CI is overwhelmed and lots of jobs are failing randomly (2025-04-29).Apr 30 2025, 12:03 PM
zeljkofilipin added a project: User-zeljkofilipin.Apr 30 2025, 1:55 PM
zeljkofilipin moved this task from Backlog 🪒 to Deep work 🌊 on the User-zeljkofilipin board.
gerritbot added a comment.Apr 30 2025, 10:15 PM

Change #1139980 merged by jenkins-bot:

[mediawiki/core@master] selenium: Always try to stop FFMPEG.

https://gerrit.wikimedia.org/r/1139980

ReleaseTaggerBot added a project: MW-1.44-notes (1.44.0-wmf.28; 2025-05-06).Apr 30 2025, 11:00 PM
Peter added a comment.May 1 2025, 7:17 AM

I'll do a follow up later where we can make sure that the process really exists and if not we try to kill it harder.

gerritbot added a comment.May 2 2025, 4:53 PM

Change #1140761 had a related patch set uploaded (by Phedenskog; author: Phedenskog):

[mediawiki/core@master] selenium: log the actual ffmpeg error.

https://gerrit.wikimedia.org/r/1140761

gerritbot added a comment.May 2 2025, 7:57 PM

Change #1140761 merged by jenkins-bot:

[mediawiki/core@master] selenium: log the actual ffmpeg error.

https://gerrit.wikimedia.org/r/1140761

gerritbot added a comment.May 5 2025, 3:21 AM

Change #1141565 had a related patch set uploaded (by Phedenskog; author: Phedenskog):

[mediawiki/core@master] selenium: Verify that ffmpeg is stopped, else hard kill it.

https://gerrit.wikimedia.org/r/1141565

gerritbot added a comment.May 5 2025, 3:37 AM

Change #1141565 abandoned by Phedenskog:

[mediawiki/core@master] selenium: Verify that ffmpeg is stopped, else hard kill it.

Reason:

This is a nicer solution: https://gerrit.wikimedia.org/r/c/mediawiki/core/+/1139943

https://gerrit.wikimedia.org/r/1141565

Peter closed this task as Resolved.May 5 2025, 7:37 AM

This is good enough what we have now.

gerritbot added a comment.May 5 2025, 9:47 AM

Change #1141565 restored by Phedenskog:

[mediawiki/core@master] selenium: Verify that ffmpeg is stopped, else hard kill it.

https://gerrit.wikimedia.org/r/1141565

zeljkofilipin subscribed.May 15 2025, 10:27 AM

@Peter an un-merged patch (1139943) from @Daimona references this task. Should the task be reopened, or should the commit be referencing another open task?

gerritbot added a comment.May 16 2025, 2:23 PM

Change #1147006 had a related patch set uploaded (by Phedenskog; author: Phedenskog):

[mediawiki/core@master] selenium: Do not hide screenshots errors

https://gerrit.wikimedia.org/r/1147006

gerritbot added a comment.May 21 2025, 1:02 PM

Change #1147006 abandoned by Phedenskog:

[mediawiki/core@master] selenium: Do not hide screenshots errors

Reason:

This happens afterTest so failure here will not fail the test.

https://gerrit.wikimedia.org/r/1147006

Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits