Re: [review-request] adamian/20150715_testfixes

[Ed Bartosh <ed.bartosh@linux.intel.com>]

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
> toaster@yoctoproject.org
> https://lists.yoctoproject.org/listinfo/toaster


-- 
--
Regards,
Ed