Hi Alex,
As you're in tts it's good time to pylint it.
The code is relatively small, so it shouldn't take much time.
JFYI, here are some pylint statistics for tts code:
$ pylint --report n *.py |wc -l
455
$ pylint *.py |grep rated
Your code has been rated at 2.49/10
Regards,
Ed
On Thu, Jul 30, 2015 at 12:27:11AM +0300, Ed Bartosh wrote:
> Hi Alex,
>
> Your patchset looks ok for me.
>
> Just a bit of nitpicking over code quality:
>
> 1. Your change in shellutils.py caused new pylint error:
> E: 91, 0: inconsistent use of tabs and spaces in indentation
> (syntax-error)
>
> 2.
> - if len(result.errors) > 0:
> - map(lambda x: config.logger.error("Exception on test: %s" %
> pprint.pformat(x)), result.errors)
> + for x in result.errors:
> + config.logger.error("Exception on test: %s:\n%s\n" %
> (pprint.pformat(x[0]),
> + "\n".join(["-- %s" % x for x in x[1].split("\n")])))
>
> I'd suggest not to use format arguments for logging. You can read the
> detailed explanations here: http://docs.pylint.org/features.html#messages
> Here is pylint warning caused by this:
> W: 49,23: Specify string format arguments as logging function parameters
> (logging-not-lazy)
>
> There are a lot of this kind of code in your patchset and in the tts
> code, so fixing all of them in separate commit would be good.
>
> 3. one character long variable names(x, e in your changes) are not very
> readable:
> C: 40, 4: Invalid variable name "e" (invalid-name)
>
> 4. it would be better to assign os.path.join(self.crtdir, "toaster.sqlite")
> to local
> variable:
> + if os.path.exists(os.path.join(self.crtdir,
> "toaster.sqlite")):
> + os.remove(os.path.join(self.crtdir, "toaster.sqlite"))
>
> 5. Mixed 2 different set of changes in commit " fix HTML5 compliance
> test". It's better to move logging improvements to another commit.
>
> 6. Typo in "execute tests in numeric order" commit message:
> undeeded -> unneeded
>
> 7. As your tests have the same prefix default sorting should be enough,
> e.g.
> In [1]: sorted(["Test01PySystemStart", "Test00PyCompilable",
> "Test02HTML5Compliance"])
> Out[1]: ['Test00PyCompilable', 'Test01PySystemStart', 'Test02HTML5Compliance']
> + # sorting the tests by the numeric order in the class name
> + tests = sorted(tests, key = lambda x: int(re.search(r"[0-9]+",
> x[1]).group(0)))
>
> 8. Mixed functionality and code readability fixes in "run pylint on the
> toaster files"
>
> Regards,
> Ed
>
> On Thu, Jul 16, 2015 at 03:44:05PM +0100, Damian, Alexandru wrote:
> > Hello,
> >
> > I've improved the TTS tests a bit. At this moment, the tests spot YOCTO
> > #7955 (Toaster fails to retrieve recipe and machine information from the
> > layer index) correctly.
> >
> > The patchset is at:
> >
> > http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/log/?h=adamian/20150715_testfixes
> >
>
> >
> > The toaster unit tests and the selenium tests are not run - it's going to
> > be a different patchset on top of this one.
> >
> > Please review and merge at your own convenience.
> >
> > Cheers,
> > Alex
> >
> >
> > --
> > Alex Damian
> > Yocto Project
> > SSG / OTC
>
> > --
> > _______________________________________________
> > toaster mailing list
> > [email protected]
> > https://lists.yoctoproject.org/listinfo/toaster
>
>
> --
> --
> Regards,
> Ed
> --
> _______________________________________________
> toaster mailing list
> [email protected]
> https://lists.yoctoproject.org/listinfo/toaster
--
--
Regards,
Ed
--
_______________________________________________
toaster mailing list
[email protected]
https://lists.yoctoproject.org/listinfo/toaster
