[Piglit] [PATCH] Add dmesg option for reboot policy

[Dylan Baker]

On Wed, Nov 25, 2015 at 06:32:25AM -0800, yann.argotti at linux.intel.com wrote:
> > On Tue, Nov 24, 2015 at 04:14:23AM -0800, yann.argotti at linux.intel.com
> > wrote:
> 
> >
> > I don't like the aproach of passing the returncode through every level
> > of piglit at all. I'd much rather use an exception for this, either by
> > extending PiglitFatalError or by adding a new exception, and then
> > catching it either in the exceptions.handler decorator, or if we have to
> > in run/resume.
> 
> Understood, I will apply exception approach then.
> 
> >
> > I've got a couple more comments inline.
> >
> > Also, please figure out where the line-wrapping is coming from and
> > remove it (if you're pasting this into a window in your email client you
> > will get line-wrap which breaks git am and git apply). Using git
> > send-email is really the best way to do this.
> 
> I should have fix issue on my side now.
> 
> > [snip]
> >
> >> +
> >> +    def reboot_is_needed(self):
> >> +        """ Simply return the reboot_needed variable state """
> >> +        return self._reboot_needed
> >
> > This is what a property is for:
> > @property
> > def reboot_needed(self):
> > return self._reboot_needed
> >
> > Then you access the value as an attribute rather than as a function.
> >
> > If self.reboot_needed:
> >     print('reboot now!')
> 
> Thanks for highlighting this :)
> 
> >
> > There is a ton of duplication between LinuxDmesg and
> > LinuxMonitoredDmesg. I haven't looked too closely, but it seems like
> > LinuxMonitoredDmesg could inherit from LinuxDmesg?
> 
> LinuxMonitoredDmesg is quite close but couldn't inherit directly (or may
> need to refactor LinuxDmesg code). In addition to look for certain
> patterns in dmesg, this class doesn't replace pass with dmesg-warn, and
> warn and fail with dmesg-fail, like LinuxDmesg class is doing.
> But following various review & comments, I am going to modify the approach
> to allow pattern definition and as well as defining where & how to look
> for these patterns.

Okay, I'll look again when the next version arrives then.

> 
> >
> > [snip]
> >
> > Dylan
> >
> Thanks Dylan for your feedback.
> 
> Yann

Thanks for this, it sounds pretty useful.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20151125/4378e7d2/attachment.sig>