On Mon 09 Jul 2012 at 03:45PM, Tim Foster wrote: > Hi Dan, > > On 07/ 9/12 12:18 PM, Dan Price wrote: > >https://cr.opensolaris.org/action/browse/pkg/dp/pkg-progress-cleanup/ > > src/um/update-refresh.sh > > It could be worthwhile to redirect stdout as well as stderr too if > we really want root to never get spammed by this script (though that > should never happen, I know :) Perhaps bonus points for using > $DEBUG to determine whether to redirect the streams and use the "-q" > option, but I'm not really particularly bothered either way.
Yeah, it's hard to know what the right thing is. I mean, I guess I could rip out the -q and just do the stdout redirection. I was sort-of trying to follow along with the way we'd done things elsewhere, and when I socialized 'refresh -q' various other folks said (roughly) "yes, that's good." Any other opinions? > src/tests/api/t_printengine.py > > Rather than using sys.stdout, could you use a StringIO? That's a good call. I had to restructure things a bit because inside the printengine test routine we are calling fileno() on the input file handle to see if the associated fd isatty(), and that of course fails if you hand it a stringio. So I fixed that interaction up. > If you did > that, I wonder would you be able to then check for some of the test > patterns written by printengine.py? Definitely it could be further improved... I think I'd like to put this in for now, and then we can expand upon it. The idea here is more to have a "smoke test" of the functionality. So much of the tricky parts of what the printengine does has to be validated by a human anyway looking at a screen anyway, so my main idea here was to make sure that the test code for doing that doesn't rot. > src/modules/client/printengine.py > > Not this bug, but was hoping this would fix a minor pkglint-related > glitch that when finishing lint runs against a repository, we don't > print a newline at the end: > > --- > $ pkglint -c /tmp/cache.$$ -l http://localhost:9090 > Lint engine setup... > > Ignoring -l option, existing image found. > Starting lint run... > > Lint phase 1 0/1$ > --- Well if the last thing it prints is '0/1' (Zero of 1) and then exits... then we likely need to make some additional call somewhere. File a bug if not already done, and let's work on it. > src/tests/api/t_elf.py, line 55 > > Do we really want this? > > The tests will fail on any test box that has, say > /kernel/drv/amd64/README.nvidia, or > /kernel/drv/amd64/nvidia.snv_100.gz - I know we don't package those > files, but is it valid to assume everything in 'scan_dirs' is going > to be an elf binary? I had thought it was, but I guess it's a fair point. I reworked these again, this time grouping things by type of test. I also fixed a couple of other references to the root image which seemed easily avoidable. > Perhaps it would be ok to run on every file in > 'scan_dirs' that has an 'elfhash' attribute under the current image, > but that feels a bit weird too.. My original rewrite was to scan every elf binary in core-os and system/kernel which was present on the current image-- since I thought that would build up a nice test corpus and really exercise the code, but the testsuite has logic to specifically forbid access to the system's image, and defeating that seemed wrong, so I threw that code away and invented this code :p New review is here: https://cr.opensolaris.org/action/browse/pkg/dp/pkg-progress-cleanup-2/ Parts which have changed are 7179267 (elf) and 7182385 (t_printengine). -dp -- Daniel Price, Solaris Kernel Engineering _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
