On Tue, Mar 30, 2010 at 2:43 PM, Martin Bligh <[email protected]> wrote:

> On Tue, Mar 30, 2010 at 2:30 PM, John Admanski <[email protected]>
> wrote:
> > Isn't setting the variable in os.environ kind of heavyweight...couldn't
> you
> > set it as part of the utils.system command invocation? That would avoid
> the
> > mess of having to save off values and adding more cleanup and in general
> > introducing unnecessary state.
>
> Tests are a separate subprocess anyway, so I don't think we even need to
> clean this up. That said, part of utils.system sounds fine too.
>
>
True, although I still think it's better to avoid messing with the environ
if you don't really have to. But I'm sure that setting LC_ALL instead of
LANG is the right thing; otherwise people who have LC_ALL or LC_TIME set
will run into the exact same problem that this patch is supposed to address.

And I know there are systems out there with one of those set, because when I
went to try tweaking LANG on my system to try this out it had no effect
because it turned out LC_TIME was defined. :)

-- John

> Also, I think LC_TIME will override LANG so you probably want to set that
> > (or even just LC_ALL) if you really want to be safe. Unless you looked at
> > the source and see that it uses LANG, but I believe the standard is that
> > LC_ALL overrides LC_TIME which overrides LANG.
> > -- John
> >
> > On Tue, Mar 30, 2010 at 2:19 PM, Lucas Meneghel Rodrigues <
> [email protected]>
> > wrote:
> >>
> >> Set LANG=C to make sure the output of hwclock will have
> >> the same output in all tested systems, regardless of their
> >> language settings. After the test is done, we restore
> >> current system language settings.
> >>
> >> Thanks to Jason Wang, who spotted the the problem, and for
> >> Gregory Smith, that pointed out the solution.
> >>
> >> Signed-off-by: Lucas Meneghel Rodrigues <[email protected]>
> >> ---
> >>  client/tests/hwclock/hwclock.py |   30 +++++++++++++++++++++++++++---
> >>  1 files changed, 27 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/client/tests/hwclock/hwclock.py
> >> b/client/tests/hwclock/hwclock.py
> >> index 12f8c54..990959c 100644
> >> --- a/client/tests/hwclock/hwclock.py
> >> +++ b/client/tests/hwclock/hwclock.py
> >> @@ -1,16 +1,40 @@
> >>  from autotest_lib.client.bin import test, utils
> >>  from autotest_lib.client.common_lib import error
> >> -import re
> >> +import re, os, logging
> >>
> >>  class hwclock(test.test):
> >>     version = 1
> >>
> >> -    def run_once(self, seconds=1):
> >> +    def setup(self):
> >> +        """
> >> +        Set up language environment, necessary to get the expected
> format
> >> +        for hwclock output.
> >> +        """
> >> +        self.lang = os.environ['LANG']
> >> +        logging.info('Current system locale is set to %s', self.lang)
> >> +        logging.info('Setting system locale to C')
> >> +        os.environ['LANG'] = 'C'
> >> +
> >> +
> >> +    def run_once(self):
> >> +        """
> >> +        Set hwclock back to a date in 1980 and verify if the changes
> took
> >> +        effect in the system.
> >> +        """
> >> +        logging.info('Setting hwclock to 2/2/80 03:04:00')
> >>         utils.system('/sbin/hwclock --set --date "2/2/80 03:04:00"')
> >>         date = utils.system_output('/sbin/hwclock')
> >>         if not re.match('Sat *Feb *2 *03:04:.. 1980', date):
> >> -            raise error.TestFail('Failed to set hwclock back to the
> >> eighties')
> >> +            raise error.TestFail('Failed to set hwclock back to the
> >> eighties. '
> >> +                                 'Output of hwclock is %s', date)
> >>
> >>
> >>     def cleanup(self):
> >> +        """
> >> +        Restore hardware clock to current system time and also restore
> >> language
> >> +        settings.
> >> +        """
> >> +        logging.info('Setting the hardware clock to the system time')
> >>         utils.system('/sbin/hwclock --systohc --noadjfile --utc')
> >> +        logging.info('Restoring system language settings')
> >> +        os.environ['LANG'] = self.lang
> >> --
> >> 1.6.6.1
> >>
> >> _______________________________________________
> >> Autotest mailing list
> >> [email protected]
> >> http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
> >
> >
> > _______________________________________________
> > Autotest mailing list
> > [email protected]
> > http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
> >
> >
>
_______________________________________________
Autotest mailing list
[email protected]
http://test.kernel.org/cgi-bin/mailman/listinfo/autotest

Reply via email to