On 18.5.2016 18:43, Stephen Warren wrote: > On 05/18/2016 07:11 AM, Michal Simek wrote: >> Large file transfers, flash erasing and more complicated tests >> requires more time to finish. Provide a way to setup specific >> timeout directly in test. >> >> For example description for 50s test: >> timeout = 50000 >> with u_boot_console.temporary_timeout(timeout): >> u_boot_console.run_command(...) > >> diff --git a/test/py/u_boot_console_base.py >> b/test/py/u_boot_console_base.py > >> self.console.eval_bad_patterns() >> >> + > > Nit: No need for two blank lines. > >> +class ConsoleSetupTimeout(object): >> + """Context manager (for Python's with statement) that temporarily >> setup > > Nit: s/set/sets up/ > >> + timeout for specific comnand. This is useful when execution time >> is greater > > Nit: s/comnand/command/ > >> + then default 30s.""" >> + >> + def __init__(self, console, timeout): >> + self.console = console >> + self.console.orig_timeout = self.console.p.timeout >> + self.console.p.timeout = timeout > > I'd suggest storing console.p not console. That way, even if console.p > is replaced because of an issue, you still have a handle to it and don't > have to make the code in __exit__ conditional: > > self.p = console.p > self.orig_timeout = self.p.timeout > self.p.timeout = timeout > >> + >> + def __enter__(self): >> + return self >> + >> + def __exit__(self, extype, value, traceback): >> + self.console.p = self.console.get_spawn() >> + self.console.p.timeout = self.console.orig_timeout > > This then doesn't need the conditionals you mentioned in your response > to this patch, and can then be: > > self.p.timeout = self.orig_timeout > >> + def temporary_timeout(self, timeout): >> + """Temporarily setup different timeout for commands. > > Nit: s/setup/set up/ > >> + Args: >> + timeout: Time in miliseconds. > > Nit: s/miliseconds/milliseconds/ >
I have fixed all of these comments and send v2. Thanks, Michal _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
