If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Deactivate screensaver on Rev3 Fedora when running talos

RESOLVED FIXED

Status

Release Engineering
General
RESOLVED FIXED
7 years ago
4 years ago

People

(Reporter: karlt, Assigned: dustin)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [buildslaves][hardware][simple][buildduty])

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

7 years ago
+++ This bug was initially created as a clone of Bug #585286 +++

Bug 585286 seems to have successfully deactivated the screensaver on mochitest and reftest machines, thanks.

AFAICS the extra commands added there are not being run on talos machines.

The existence of bug 585286 suggests that the screensaver is also sometimes running on talos machines, which would be expected to affect results.

Updated

7 years ago
Whiteboard: [buildslaves][hardware][buildduty]
Off to John for prioritization
Assignee: nobody → joduinn
Strictly, bug 585286 added code to factory for unittests to run 'xset s reset', see attachment 472788 [details] [diff] [review]. See attachment 466896 [details] [diff] [review] for karl's take on doing the same for talos.
Summary: Deactivate screensaver on Rev3 Fedora talos machines → Deactivate screensaver on Rev3 Fedora when running talos
(In reply to comment #1)
> Off to John for prioritization

If the screen saver is not disabled, and this impacts production talos numbers, this should be fixed quickly.


:karlt: can we close this as a DUP of (already fixed) bug 585286 - or is there something else we still need to do ?
Assignee: joduinn → nobody
Whiteboard: [buildslaves][hardware][buildduty] → [buildslaves][hardware]
(In reply to comment #3)
> (In reply to comment #1)
> > Off to John for prioritization
> 
> If the screen saver is not disabled, and this impacts production talos numbers,
> this should be fixed quickly.
> 
> 
> :karlt: can we close this as a DUP of (already fixed) bug 585286 - or is there
> something else we still need to do ?

That bug was for unit tests, this bug is for talos.  This should be very simple to fix and should be able to use almost the same code.

If nobody has fixed this by the next time that I test a mozilla-tests patch, I will take it.  The code that needs to be added would be

+        if 'fed' in self.OS:
+            self.addStep(ShellCommand(
+                name='disable_screensaver',
+                command=['xset', 's', 'reset'],
+            ))

Not sure exactly where to put this in TalosFactory.
Whiteboard: [buildslaves][hardware] → [buildslaves][hardware][simple]

Comment 5

7 years ago
in TalosFactory.addCleanupSteps() would be my suggestion - this step is run before anything is downloaded or processed.

Comment 6

7 years ago
dustin, assigning this to you - ping me when you take a look at it so I can walk you thru it
Assignee: nobody → dustin
(Assignee)

Comment 7

7 years ago
Created attachment 490940 [details] [diff] [review]
601783.patch
Attachment #490940 - Flags: feedback?(bear)

Updated

7 years ago
Attachment #490940 - Flags: feedback?(bear) → feedback+
(Assignee)

Comment 8

7 years ago
Verified on mozilla-tests2.  Before I land this, jhford: why use 'xset s reset' instead of 'xset s off'?
Flags: needs-reconfig?
Whiteboard: [buildslaves][hardware][simple] → [buildslaves][hardware][simple][buildduty]
(Reporter)

Comment 9

7 years ago
'xset s off' only changes the screen saver timeout.

man XSetScreenSaver:
"A timeout of 0 disables
 the screen saver (but an activated screen saver is not deactivated)"

The screen saver timeout already seems to be 0.
See bug 585286 comment 1 and 11.
Comment on attachment 490940 [details] [diff] [review]
601783.patch

Landed in changeset:   1248:8e866243561c, merged to the production-0.8 branch in 1250:30537b778f04
Attachment #490940 - Flags: review+
Attachment #490940 - Flags: checked-in+
Is this done now?
Flags: needs-reconfig?
(Reporter)

Comment 12

7 years ago
"argv: ['xset', '-s', 'reset']"

"xset:  unknown option -s"

That should be ['xset', 's', 'reset'].
(Assignee)

Comment 13

7 years ago
Created attachment 495604 [details] [diff] [review]
m570843-puppet-manifests-r1.patch

Wow, how did I manage to screw that up?
Attachment #490940 - Attachment is obsolete: true
Attachment #495604 - Flags: review?(karlt)
(Assignee)

Comment 14

7 years ago
Created attachment 495612 [details] [diff] [review]
m601783-buildbotcustom-r2.patch

Correct patch (sorry)
Attachment #495604 - Attachment is obsolete: true
Attachment #495612 - Flags: review?(karlt)
Attachment #495604 - Flags: review?(karlt)
(Reporter)

Comment 15

7 years ago
Comment on attachment 495612 [details] [diff] [review]
m601783-buildbotcustom-r2.patch

Thanks :)
Attachment #495612 - Flags: review?(karlt) → review+
(Assignee)

Updated

7 years ago
Flags: needs-reconfig?
Comment on attachment 495612 [details] [diff] [review]
m601783-buildbotcustom-r2.patch

Landed on default and production-0.8.
Attachment #495612 - Flags: review+
Attachment #495612 - Flags: checked-in+
Clearing reconfig flag.
Flags: needs-reconfig?
(Reporter)

Comment 18

7 years ago
That's running successfully now, thanks.
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.