Closed Bug 782848 Opened 8 years ago Closed 8 years ago

autolog support for moztest

Categories

(Testing :: Mozbase, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: mihneadb, Assigned: mihneadb)

References

Details

Attachments

(2 files, 3 obsolete files)

No description provided.
Assignee: nobody → mbalaur
Attached patch implement the functionality (obsolete) — Splinter Review
Attachment #652169 - Flags: review?(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.
Attachment #652169 - Flags: review?(jgriffin) → review-
This adds the serialize method as requested.
Attachment #652169 - Attachment is obsolete: true
Attachment #652940 - Flags: review?(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
Attachment #652940 - Flags: review?(jgriffin) → review+
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.
Attachment #652943 - Flags: review?(jgriffin) → review+
(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.
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.
Attached patch update: make output a submodule (obsolete) — Splinter Review
Attachment #652943 - Attachment is obsolete: true
Attachment #652987 - Flags: review?(jgriffin)
fixed a small thing - was assigning msg inside a loop in autolog (no need for that)
Attachment #652987 - Attachment is obsolete: true
Attachment #652987 - Flags: review?(jgriffin)
Attachment #652988 - Flags: review?(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!
Attachment #652988 - Flags: review?(jgriffin) → review+
You need to log in before you can comment on or make changes to this bug.