[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [OSSTEST PATCH v13 19/24] TestSupport: Implement target_subunit_cmd a subunit stream parser into substeps
On Tue, Jul 25, 2017 at 07:00:47PM +0100, Ian Jackson wrote: > Anthony PERARD writes ("[OSSTEST PATCH v13 19/24] TestSupport: Implement > target_subunit_cmd a subunit stream parser into substeps"): > > target_subunit_cmd can be used like target_cmd, but the command would > > needs to output a subunit v1 stream, which will be parsed and turned > > into osstest substeps. The command can be `| subunit-2to1` in order to > > turn a subunit v2 stream into v1. > > > > Currently, time is not taken into account, and all substeps will have > > bogus timestamp as the output of the command is parsed after it has > > runned. > > > > This is a description of the subunit v1 protocol, taken from > > python-subunit README, or https://pypi.python.org/pypi/python-subunit > > What a lot of code! > > > + while (<$stdout>) { > > + if (/^time: (\d+)-(\d+)-(\d+) (\d+):(\d+):(\d+)(\.\d+)?Z$/) { > > + # This is the timestamp for the next events > > I'm not sure what your ( ) are doing here. No real reason. I'll remove them. > > + } elsif (/^test(?:ing)?:? (.+)\n/) { > > + # Start of a new test. > > + $logfilename = subunit_sanitize_testname($1) . '.log'; > > + $fh = open_unique_stashfile(\$logfilename); > > This name might clash with existing logfile names, which might be > generated later. Can you put "subunit-" on the front maybe ? Will do. > > + substep_start(subunit_sanitize_testname($1), $logfilename); > > And here, I think you should start the parameter you pass to > substep_start with '/' so that it gets appended to the testid for the > whole script, for a similar reason. OK. > I think it would be better to call subunit_sanitize_testname only > once. OK. > > + } elsif (/^(success(?:ful)?|failure|skip|error|xfail|uxsuccess): > > + \ (.+?)(\ \[(\ multipart)?)?$/x) { > > + # Result of a test, with its output. > > + my $result = $1; > > + my $testname = $2; > > + my $have_details = $3; > > + my $is_multipart = $4; > > I would normally write this: > my ($result, $testname, $have_... ) = ($1,$2,$3,$4,$5) > although I don't really mind much that you have written it as you > have. > > > + if ($have_details) { > > + if ($is_multipart) { > > + # Test output > > + while (<$stdout>) { > > + # part content-type > > + # from > > https://tools.ietf.org/html/rfc6838#section-4.2 > > + my $restricted_name = > > qr'[a-zA-Z0-9][a-zA-Z0-9!#$&^_.+-]*'; > > + if (m{ ^Content-Type:\s+ > > + $restricted_name/$restricted_name # > > type/sub-type > > + # parameters > > + (?:\s*;\s*$restricted_name=[^,]+ > > + (?:,\s*$restricted_name=[^,]+)*) > > + \s*$ > > + }xi) { > > I don't understand why you are trying to match this Content-Type so > precisely. AFAICT from the grammar, all you need to do is see whether > there is something vaguely like a c-t header. I think I start by looking at what kind of characters could be part of type and sub-type, and just start writing a more complicated regex. So is the following would be enough for you? m{^Content-Type: [^/ ]+/[^/ ]+(?:;.+)?$} > > + print $fh $_ or die $!; > > + > > + # part name > > + my $line = <$stdout>; > > + print $fh $line or die $!; > > + > > + # Read chunks of a part > > + while (<$stdout>) { > > + if (/^([0-9A-F]+)\r$/i) { > > + my $chunk_size = hex($1); > > What makes you think the digits are in hex ? I tried with [0-9] (because DIGITS), but that was not enought. Then I've check the subunit implementation, there are using "%X" which is hex. > Since you have to go to the effort of separating out all of this > stuff, it might be worth printing these multipart objects with one > object per logfile. Although I won't insist on that because I suspect > that multipart results are rare. There are usually 3 part per tests, with those names: pythonlogging:'' stdout stderr And sometime, there is also one name 'traceback'. I think stdout and stderr are usually empty. I think having one file per part will make it more complicated to read logs of a failed test. > > + } else { > > + # Unexpected output > > + chomp; > > + logm("*** $_"); > > I guess the error recovery is to continue until you see "]" > and hope. Fair enough. That one of the reason for the subunit-v2, with a binary protocol, better recovery. -- Anthony PERARD _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |