[tbot] Testcases duplicated

[Harald Seiler]

Hi Stefano,

On Thu, 2018-12-20 at 14:26 +0100, Stefano Babic wrote:
> On 20/12/18 13:56, Harald Seiler wrote:
> > Hi Stefano,
> > 
> > On Thu, 2018-12-20 at 13:40 +0100, Stefano Babic wrote:
> > > Hi,
> > > 
[...]
> > > board tc:
> > > ----------
> > > 
> > > import contextlib
> > > import typing
> > > import time
> > > import tbot
> > > from tbot.machine import linux
> > > from tbot import tc
> > > from swupdate import *
> > > 
> > > @tbot.testcase
> > > def gtuswupdate(
> > >     mach: typing.Optional[linux.LinuxMachine] = None,
> > > ) -> None:
> > > 
> > >         .......
> > >         tc.testsuite(
> > >             checkswupdate,
> > >             swupdateweb,
> > >             mach=lh,
> > >             path=path,
> > >             ip=ip
> > >         )
> > 
> > One thing first:  Are you sure you want tc.testsuite in this case?
> 
> Yes
> 
> > tc.testsuite will call ALL testcases, regardless of failures.  I think
> > you'd want to abort if "checkswupdate" fails?
> 
> No
> 
> >  Or have I misunderstood
> > your intentions here?
> 
> I want just to put the version of the tool in the log file.
> 

Ah, I see, I thought `checkswupdate` was a check if swupdate is installed
correctly.  But as that is not the case, your code makes sense!

> > If I am correct, you are better off using:
> > 
> > 	@tbot.testcase
> > 	def gtuswupdate(
> > 	    mach: typing.Optional[linux.LinuxMachine] = None,
> > 	) -> None:
> > 
> > 	    .......
> > 	    checkswupdate(mach=lh, path=path, ip=ip)
> > 	    swupdateweb(mach=lh, path=path, ip=ip)
> 
> ok - understood.
> 
> > 	
> > This will abort if `checkswupdate` fails and not call `swupdateweb`
> > regardless of that.
> 
> Got it.
> 
> > > This works when I have just one board. If I add a second board, and I
> > > want to add a "newboardupdate", I have to import the testcases and tbot
> > > reports "duplicated testcases" (all files seems to be loaded). What is
> > > wrong ?
> > 
> > Ah, I see what is going on here ;)  This is actually intended, but for other
> > reasons and I might look into fixing this specific failure you are seeing here,
> > because it is just annoying ...
> > 
> > What is going on:
> > 
> > 	from swupdate import *
> > 
> > imports all globals from the swupdate module into your current namespace.  This
> > means, when tbot looks for globals that have a testcase signature, it will not only
> > find the ones defined in board-testcases.py, but also the ones imported from swupdate.
> > 
> > If your second file now does the same, tbot will find the swupdate testcases in there
> > a second time!  This is why the error is reported ... The quick solution is to not
> > import all testcases from swupdate into your own namespace, but keep them in theirs:
> > 
> > 	import swupdate as swu
> 
> Ah, ok - thanks !
> 
> > 	...
> > 
> > 	@tbot.testcase
> > 	def testcase():
> > 		swu.checkswupdate()
> > 		swu.swupdateweb()
> > 
> > Generally I prefer this style anyway, because you can more easily see where a name is
> > imported from (* imports are discouraged [1]).
> 
> Nice - this solves ,y issue, thanks !
> 
> > But I can also understand that this is irritating and as said before, I will look into
> > making duplicate detection smarter ...
> 
> Or maybe the users must be smarter...
> 

Well, at least the error message shouldn't be confusing.  Anyway, fixed in
4099432289625155f821eef131d72f5b651d3482 [1].

> Regards,
> Stefano
> 

[1]: https://github.com/Rahix/tbot/commit/4099432289625155f821eef131d72f5b651d3482
-- 
Harald

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-62  Fax: +49-8142-66989-80   Email: hws at denx.de