782848 - autolog support for moztest

Status: RESOLVED FIXED

(Testing :: Mozbase, defect)

Comment 1

[Mihnea Dobrescu-Balaur (:mihneadb)]

Attachment: implement the functionality

Comment 2

[Jonathan Griffin (:jgriffin)]

Comment on attachment 652169 [details] [diff] [review]
implement the functionality

Review of attachment 652169 [details] [diff] [review]:
-----------------------------------------------------------------

Looks great for a WIP!  See comments below.

::: moztest/moztest/output.py
@@ +115,5 @@
> +        self.es_server = es_server
> +        self.rest_server = rest_server
> +
> +    def serialize(self, results_collection, file_obj):
> +        file_obj.write(self.make_testgroup(results_collection)._to_json())

So, looking at the mozautolog code, I think what we want for serialize is to output the data generated by http://hg.mozilla.org/automation/mozautolog/file/df09d85e4935/mozautolog/mozautolog.py#l640, through line 662.  I suggest putting that code into a separate method in mozautolog, and then having both RESTfulAutologTestGroup.submit() and AutologOutput.serialize() call that method.

@@ +118,5 @@
> +    def serialize(self, results_collection, file_obj):
> +        file_obj.write(self.make_testgroup(results_collection)._to_json())
> +
> +    def make_testgroup(self, results_collection):
> +        context = results_collection.contexts[0]

We should iterate through the contexts, and create a separate test group for each.

@@ +120,5 @@
> +
> +    def make_testgroup(self, results_collection):
> +        context = results_collection.contexts[0]
> +        testgroup = RESTfulAutologTestGroup(
> +             testgroup='moztest',

We should allow the caller to specify the testgroup name, probably in the ctor.

@@ +123,5 @@
> +        testgroup = RESTfulAutologTestGroup(
> +             testgroup='moztest',
> +             os=context.os,
> +             platform=context.arch,
> +             harness='moztest',

We should allow the caller to specify the harness name, probably in the ctor.

@@ +134,5 @@
> +            testsuite=results_collection.suite_name,
> +            elapsedtime=results_collection.time_taken,
> +            passed=len(list(results_collection.tests_with_result('PASS'))),
> +            failed=len(failed),
> +            todo=len(list(results_collection.tests_with_result('SKIPPED')))

todo should also include known fails.  I think we may need to add a generic 'FAIL' result to moztest; a known fail would have expected_result == actual_result == 'FAIL'.  Right now, there are two failure states, 'UNEXPECTED-FAIL' and 'KNOWN-FAIL', but this makes the harness refer to the expected result in order to supply the correct actual result.

It might be better just to have 'FAIL', and then make two properties in TestResult, unexpectedFail, and knownFail, which could return true/false accordingly.

@@ +137,5 @@
> +            failed=len(failed),
> +            todo=len(list(results_collection.tests_with_result('SKIPPED')))
> +            )
> +        testgroup.set_primary_product(
> +            tree='b2g'

We need to allow tree, and other product info like revision and product name, to be stored in a context and populated accordingly.  We should either add this to the existing TestContext, or create something new like a ProductContext for this.  I think our lives will be simpler if we just add it to TestContext.  Jeff, do you have an opinion?

@@ +141,5 @@
> +            tree='b2g'
> +            )
> +        for f in failed:
> +            testgroup.add_test_failure(
> +                test='%s.%s' % (f.test_class, f.name),

class_name can sometimes be an empty string, which would generate a weird name here (".name"); maybe add a 'longname' (or 'fullname', or something) attr to TestResult that returns an appropriately formatted long name, depending on whether or not test_class is an empty string?

@@ +143,5 @@
> +        for f in failed:
> +            testgroup.add_test_failure(
> +                test='%s.%s' % (f.test_class, f.name),
> +                text='\n'.join(f.output),
> +                status=f.result_actual

We actually want a calculated result here, i.e., one of PASS, KNOWN-FAIL, UNEXPECTED-FAIL, UNEXPECTED-SUCCESS, SKIPPED, rather than the naked result_actual.

Comment 3

[Mihnea Dobrescu-Balaur (:mihneadb)]

Attachment: add serialize to mozautolog

This adds the serialize method as requested.

Comment 4

[Mihnea Dobrescu-Balaur (:mihneadb)]

Attachment: improve moztest as requested and add the autolog output functionality

Comment 5

[Jonathan Griffin (:jgriffin)]

Comment on attachment 652940 [details] [diff] [review]
add serialize to mozautolog

Review of attachment 652940 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks! Landed as http://hg.mozilla.org/automation/mozautolog/rev/5bd020decef4

Comment 6

[Jonathan Griffin (:jgriffin)]

Comment on attachment 652943 [details] [diff] [review]
improve moztest as requested and add the autolog output functionality

Review of attachment 652943 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good!  I think the next step is to hook it up to something real.  Let's try replacing Marionette's autolog posting code with moztest and see how it fares; I'll file a separate bug for that.

::: moztest/moztest/output.py
@@ +5,4 @@
>  
>  from abc import abstractmethod
>  from contextlib import closing
> +from mozautolog import RESTfulAutologTestGroup

I think this import should go in AutologOutput's __init__, so you don't have to have the package installed in order to use XUnit output, for example.

@@ +168,5 @@
> +
> +    def post(self, *data):
> +        # import pdb;pdb.set_trace()
> +        for d in data:
> +            print type(d)

Extra print and debugging import.

Comment 7

[Mihnea Dobrescu-Balaur (:mihneadb)]

(In reply to Jonathan Griffin (:jgriffin) from comment #6)
> Comment on attachment 652943 [details] [diff] [review]
> improve moztest as requested and add the autolog output functionality
> 
> Review of attachment 652943 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good!  I think the next step is to hook it up to something real. 
> Let's try replacing Marionette's autolog posting code with moztest and see
> how it fares; I'll file a separate bug for that.
> 
> ::: moztest/moztest/output.py
> @@ +5,4 @@
> >  
> >  from abc import abstractmethod
> >  from contextlib import closing
> > +from mozautolog import RESTfulAutologTestGroup
> 
> I think this import should go in AutologOutput's __init__, so you don't have
> to have the package installed in order to use XUnit output, for example.

Hm, so it is either that or splitting output into files. Which one do you think is better?

> 
> @@ +168,5 @@
> > +
> > +    def post(self, *data):
> > +        # import pdb;pdb.set_trace()
> > +        for d in data:
> > +            print type(d)
> 
> Extra print and debugging import.

Right, I left that to test some more after I made eventual changes.

Comment 8

[Jonathan Griffin (:jgriffin)]

Probably splitting into files would be a good idea.  We'll eventually need several output classes, no need to have them all in one monolithic file.

Comment 9

[Mihnea Dobrescu-Balaur (:mihneadb)]

Attachment: update: make output a submodule

Comment 10

[Mihnea Dobrescu-Balaur (:mihneadb)]

Attachment: improve moztest as requested and add the autolog output functionality

fixed a small thing - was assigning msg inside a loop in autolog (no need for that)

Comment 11

[Jonathan Griffin (:jgriffin)]

Comment on attachment 652988 [details] [diff] [review]
improve moztest as requested and add the autolog output functionality

Review of attachment 652988 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, thanks!

Comment 12

[Mihnea Dobrescu-Balaur (:mihneadb)]

https://github.com/mozilla/mozbase/commit/af2d425bec0e132d87916238e230d07a5f182b0e
https://github.com/mozilla/mozbase/commit/8a59ea49482949ef497923c82b3cc51c92994d4e
https://github.com/mozilla/mozbase/commit/ad41f168a1b6a784099cb52f94cbc9eda5191252