[tahoe-lafs-trac-stream] [tahoe-lafs] #1296: 'tahoe debug trial' command

[tahoe-lafs]

#1296: 'tahoe debug trial' command
-----------------------------------+----------------------------------------
     Reporter:  davidsarah         |       Owner:  davidsarah                       
         Type:  defect             |      Status:  assigned                         
     Priority:  major              |   Milestone:  1.8.2                            
    Component:  code-frontend-cli  |     Version:  1.8.1                            
   Resolution:                     |    Keywords:  bbfreeze setuptools review-needed
Launchpad Bug:                     |  
-----------------------------------+----------------------------------------

Comment (by warner):

 A preliminary review pass looks good. I definitely prefer the
 {{{sys.argv=['trial'..]}}} and {{{twisted_trial.run()}}} approach: it
 feels like that should be reliable. I think I'd rather get the extra
 arguments from {{{parseArgs()}}} instead of {{{sys.argv[3:]}}} though,
 as the latter feels fragile. Something like:

 {{{
    def parseArgs(self, *args):
        self.trial_args = args
 ...
 def trial(config):
     sys.argv = ['trial'] + list(config.trial_args)
     # This does not return
     twisted_trial.run(); sys.exit(0)
 }}}

 (and the {{{sys.exit(0)}}} is for paranoia, to avoid surprises if we're
 wrong. it's on the same line as twisted_trial.run() keep the code-coverage
 numbers good. It might be appropriate to raise an exception or do an
 assert or something there instead.)

 I think trial runs just fine without the {{{'allmydata'}}} or
 {{{'allmydata.test'}}} hint: when run without arguments, it scans
 recursively below the current directory for test files. If an argument
 *is* necessary, I'd lean towards using {{{'allmydata'}}} rather than the
 more-specific {{{'allmydata.test'}}}, as we may want to put tests
 elsewhere at some point (my #466 signed-introducer work creates
 {{{allmydata.util.ecdsa.test}}}, for example).

 I'd make the help synopsis say "run Tahoe test suite (using Trial)",
 since users probably care slightly more about what the command does than
 which particular test framework is involved, and unless you already known
 Trial,
 you won't be able to figure out what the command does without a hint.

 I'm a little wary of introducing {{{bin/tahoe.pyscript}}} in places we
 don't need it, especially in the Makefile. I'd be happier if there were
 some clean way to make {{{TAHOE=bin/tahoe}}} on non-windows. But I know
 I'm in the minority when it comes to the Makefile, so I'm only a -0 on
 that.

 I'll have to do some comparison testing to see whether I'm happy with
 changing {{{make quicktest}}} to use the new scheme. It feels like
 mapping that to 'tahoe debug trial' ought to be slightly faster (one
 fewer subprocess?).

 The other Makefile changes (turning {{{$(PYTHON) bin/tahoe stop}}} into
 {{{$(TAHOE) stop}}} are probably good ones, but maybe they ought to go
 into a separate patch, since they don't touch {{{tahoe debug trial}}}
 directly. (not a separate ticket, just a separate patch, so future code
 archaeologists have an easier time following the history)

-- 
Ticket URL: <http://tahoe-lafs.org/trac/tahoe-lafs/ticket/1296#comment:9>
tahoe-lafs <http://tahoe-lafs.org>
secure decentralized storage