From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by yocto-www.yoctoproject.org (Postfix, from userid 118) id 66B1AE009E4; Wed, 29 Jul 2015 14:27:51 -0700 (PDT) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on yocto-www.yoctoproject.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 X-Spam-HAM-Report: * -5.0 RCVD_IN_DNSWL_HI RBL: Sender listed at http://www.dnswl.org/, high * trust * [192.55.52.88 listed in list.dnswl.org] * -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% * [score: 0.0000] Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by yocto-www.yoctoproject.org (Postfix) with ESMTP id 5C574E00992 for ; Wed, 29 Jul 2015 14:27:47 -0700 (PDT) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga101.fm.intel.com with ESMTP; 29 Jul 2015 14:27:17 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.15,572,1432623600"; d="scan'208";a="615480673" Received: from linux.intel.com ([10.23.219.25]) by orsmga003.jf.intel.com with ESMTP; 29 Jul 2015 14:27:17 -0700 Received: from linux.intel.com (vmed.fi.intel.com [10.237.72.65]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by linux.intel.com (Postfix) with ESMTP id B3B236A4083; Wed, 29 Jul 2015 14:26:31 -0700 (PDT) Date: Thu, 30 Jul 2015 00:27:11 +0300 From: Ed Bartosh To: "Damian, Alexandru" Message-ID: <20150729212711.GA22626@linux.intel.com> References: MIME-Version: 1.0 In-Reply-To: Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo User-Agent: Mutt/1.5.21 (2010-09-15) Cc: "toaster@yoctoproject.org" Subject: Re: [review-request] adamian/20150715_testfixes X-BeenThere: toaster@yoctoproject.org X-Mailman-Version: 2.1.13 Precedence: list Reply-To: ed.bartosh@linux.intel.com List-Id: Web based interface for BitBake List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 29 Jul 2015 21:27:51 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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