[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
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. > + } 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 ? > + 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. I think it would be better to call subunit_sanitize_testname only once. > + } 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. > + 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 ? 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. > + } else { > + # Unexpected output > + chomp; > + logm("*** $_"); I guess the error recovery is to continue until you see "]" and hope. Fair enough. Thanks, Ian. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |