Closed Bug 636746 Opened 13 years ago Closed 13 years ago

Add Mozmill version to test report

Categories

(Testing Graveyard :: Mozmill, defect)

defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: whimboo, Assigned: k0scist)

References

Details

(Whiteboard: [mozmill-2.0+][mozmill-1.5.4+])

Attachments

(2 files, 1 obsolete file)

We already had a couple of situations when an upgrade of Mozmill caused finally a downgrade on that machine. In our case it happened multiple times for release candidates of Mozmill 1.5.2 on Windows 7, which were downgraded Mozmill to 1.5.1rc1.

We really have to include the version of Mozmill in the test report, so we can make sure to mark reports as invalid if an older version of Mozmill has been used.

For now I can include it in our automation scripts but finally I would like to see that included in Mozmill itself. I would propose a property name  like "mozmill_version" in the root of the report.
As a result of my recent work with reporting installed add-ons, we can determine the version of Mozmill installed. See bug 636700, which uses the Add-on Manager API (https://developer.mozilla.org/en/Addons/Add-on_Manager) to add details of the installed add-ons to the persisted object. This is currently only for endurance tests, however I have also raised bug 636773 to add this to Mozmill.
Depends on: 636773
While this would be a good idea, it will not work for other applications which do not support the addons manager. To safe us from creating very specialist code, the Python process should get out the version and attach it to the report. And well, it would be the same as what we are already doing for the --version option.
No longer depends on: 636773
Whiteboard: [mozmill-2.0?] → [mozmill-2.0+]
Assignee: nobody → ahalberstadt
TestResults class now has a property called 'mozmill_version' which QA can use in their reports.
Attachment #516076 - Flags: review?(fayearthur+bugs)
Status: NEW → ASSIGNED
Comment on attachment 516076 [details] [diff] [review]
Patch 1.0 - Add mozmill_version property to the TestResults class

looks good.
Attachment #516076 - Flags: review?(fayearthur+bugs) → review+
master: https://github.com/mozautomation/mozmill/commit/76915c9a5bfdb4325fd94b6aaf7203331b38d8ab

Henrik, can you confirm that this is what you were looking for?
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Currently it doesn't work for me because I get the following error on OS X:

  File "/Volumes/data/build/tools/mozmill-2.0/mozmill/mozmill/report.py", line 72, in stop
    results = self.get_report()
TypeError: get_report() takes exactly 2 arguments (1 given)

Is that related to your patch, Andrew?
mozmill_version is not part of the report with the latest changeset on master. I also see a - "report_type": "NotImplementedError" - which seems like to be a fundamental failure. I will file that as a new bug.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Lets see if bug 640129 is the reason for.

In general this patch looks trivial and if we should have another hotfix release I would nominate it because it's a big help for us.
Depends on: 640129
Whiteboard: [mozmill-2.0+] → [mozmill-2.0+][mozmill-1.5.x?]
the report_type NotImplementedError was intended as a stop-gap measure when mozmill gained the ability to run both restart and non-restart tests in the same process:

https://github.com/mozautomation/mozmill/blob/master/mozmill/mozmill/report.py#L75

The entire report type thing should be removed, IMHO.  If anyone agrees and no one disagrees, I'll ticket but its not part of this bug.
So I'm guessing I need to add the version to the "get_report" method instead of the TestResults class?  Why do we even have the TestResults class then?

Jeff, are we going to have the "get_metadata_from_egg" code in both report.py and each of the modules then?
(In reply to comment #10)
> So I'm guessing I need to add the version to the "get_report" method instead of
> the TestResults class?  Why do we even have the TestResults class then?

TestResults is for gathering results.  i.e. as it says in the code, "put your results here".  This separates the concerns of running the tests versus doing stuff with the results.  As an example, let's say you wanted to run two different versions of the MozMill class, as we formerly had to do for MozMill and MozMillRestart.  How do you persist data?  Answer: TestResults

But yes, TestResults does not do reporting.  The various plugin modules -- logger.py, report.py, another_report_module.py -- take these results and do what they want to them.

> 
> Jeff, are we going to have the "get_metadata_from_egg" code in both report.py
> and each of the modules then?

No. I was thinking of one function (def get_metadata_from_egg) as a module-level function in runner.py.  This will be called in runner.py given the package name 'mozrunner' and the data living e.g. at the module-level in e.g. mozrunner.runner.  This function will be called again from mozmill/__init__.py with the package name 'mozmill' and that resulting data living in e.g. the mozmill module.  But that's all really a separate bug
I forgot to (didn't know I had to) add the mozmill_version to results object in reports.py.  This should expose the 'mozmill_version' parameter.

I also removed the lstrip in __init__.py because that should be handled in the 'get_metadata_from_egg' function (see bug 639951)
Attachment #516076 - Attachment is obsolete: true
Attachment #519451 - Flags: review?(fayearthur+bugs)
Comment on attachment 519451 [details] [diff] [review]
Patch 1.1 - Add mozmill_version to report in reports.py

looks good, if it's been tested.
Attachment #519451 - Flags: review?(fayearthur+bugs) → review+
master: https://github.com/mozautomation/mozmill/commit/ea2b09e4ec323c3e0c643908bbbb22229fa18fc3

I tested using --report=stdout and the mozmill_version attribute is there.
Henrik, can you verify?
Status: REOPENED → RESOLVED
Closed: 13 years ago13 years ago
Resolution: --- → FIXED
Looks great now. I see - "mozmill_version": "2.0a" - in the report.

If we get another 1.5.x-hotfix release I would really like to see this fix included. Would that be possible?
Status: RESOLVED → VERIFIED
(In reply to comment #15)
> Looks great now. I see - "mozmill_version": "2.0a" - in the report.
> 
> If we get another 1.5.x-hotfix release I would really like to see this fix
> included. Would that be possible?

Woudn't spin one just for this, but yeah if we do one, we could pick this up.
Whiteboard: [mozmill-2.0+][mozmill-1.5.x?] → [mozmill-2.0+][mozmill-1.5.x+]
Whiteboard: [mozmill-2.0+][mozmill-1.5.x+] → [mozmill-2.0+][mozmill-1.5.3+]
(In reply to comment #16) 
> > If we get another 1.5.x-hotfix release I would really like to see this fix
> > included. Would that be possible?
> 
> Woudn't spin one just for this, but yeah if we do one, we could pick this up.

Clint, you sadly missed this patch for Mozmill 1.5.3. :( Lets mark it to be part of Mozmill 1.5.4.
Whiteboard: [mozmill-2.0+][mozmill-1.5.3+] → [mozmill-2.0+][mozmill-1.5.4+]
I thought this had already landed on hotfix-1.5?

If not, can we land it now and go ahead and respin a 1.5.4 with just this fix once QA finishes testing?
(In reply to comment #18)
> I thought this had already landed on hotfix-1.5?
> 
> If not, can we land it now and go ahead and respin a 1.5.4 with just this fix
> once QA finishes testing?

Not before 1.5.3 has been tagged.
(In reply to comment #19)
> (In reply to comment #18)
> > I thought this had already landed on hotfix-1.5?
> > 
> > If not, can we land it now and go ahead and respin a 1.5.4 with just this fix
> > once QA finishes testing?
> 
> Not before 1.5.3 has been tagged.

You don't understand.  I'm not going to release 1.5.3.  I'll take this change as one last change to the idea of the "next 1.5.x release", spin a true 1.5.4rc, let you verify it, then tag and release 1.5.4 as our next 1.5.x release.

That's what I'm proposing.
Lets talk about it during the meeting.
Andrew, does your patch apply cleanly on hotfix-1.5? If yes you can now check-in. The 1.5.3 tag has been created and we allow additional fixes.
Andrew, do you have an update?
Blocks: 630922
Clint or Heather, can one of you please take care of this bug? Would be really appreciated to get this finally landed for 1.5.4.
Assignee: halbersa → jhammel
Component: Mozmill Utilities → Mozmill
Jeff, would you please be so kind and help out here? As it looks like it is a bit more as a simple backport, but we run over and over in issues we really would like to see the Mozmill version in reports.
Whiteboard: [mozmill-2.0+][mozmill-1.5.4+] → [mozmill-2.0+][mozmill-1.5.4+][mozmill-1.5.4-needed]
Attachment #544277 - Flags: review?(fayearthur+bugs)
Comment on attachment 544277 [details] [diff] [review]
adds version to test report

looks good
Attachment #544277 - Flags: review?(fayearthur+bugs) → review+
Thanks Jeff! Verified the fix on hotfix-1.5 with mozmill 1.5.4b6.
Whiteboard: [mozmill-2.0+][mozmill-1.5.4+][mozmill-1.5.4-needed] → [mozmill-2.0+][mozmill-1.5.4+]
Depends on: 683901
Product: Testing → Testing Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: