Closed
Bug 480077
Opened 16 years ago
Closed 16 years ago
automation.py.in : additional fix to bug 472706 for |runApp()| return value(s)
Categories
(Testing :: General, defect)
Testing
General
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla1.9.2a1
People
(Reporter: sgautherie, Assigned: sgautherie)
References
()
Details
(Keywords: fixed1.9.1, regression, Whiteboard: [fixed1.9.1b4])
Attachments
(1 file, 2 obsolete files)
No description provided.
|
Assignee | |
Updated•16 years ago
|
|
||
Updated•16 years ago
|
|
|
||
Updated•16 years ago
|
|
|
Assignee | |
Comment 1•16 years ago
|
||
Assignee: nobody → sgautherie.bz
Status: NEW → ASSIGNED
Attachment #364029 -
Flags: review?(jwalden+bmo)
|
|
Assignee | |
Comment 2•16 years ago
|
||
Comment on attachment 364029 [details] [diff] [review]
(Av1) Don't forget to return |start|
(untested, but seems obvious)
|
||
Comment 3•16 years ago
|
||
Comment on attachment 364029 [details] [diff] [review]
(Av1) Don't forget to return |start|
Test before landing, of course. :-)
Attachment #364029 -
Flags: review?(jwalden+bmo) → review+
|
|
Assignee | |
Comment 4•16 years ago
|
||
Actually, returning |start| (only) feels inconsistent to me:
I would expect |runApp()| to return either both start and stop times, or none.
In other words, we should either time the app itself or its caller, not start with the former and end with the latter, shouldn't we :-/
My case is runtests.py.in (for bug 469518 comment 5)...
|
|
Assignee | |
Comment 5•16 years ago
|
||
(In reply to comment #4)
> In other words, we should either time the app itself or its caller,
In the former case, we could replace |start| by |duration| !?
|
|
Assignee | |
Comment 6•16 years ago
|
||
http://mxr.mozilla.org/mozilla-central/search?string=runApp%5C%28®exp=on&case=on&find=%5C.py
{
Found 6 matching lines in 5 files
/build/automation.py.in
/build/leaktest.py.in
/build/pgo/profileserver.py.in
/layout/tools/reftest/runreftest.py
/testing/mochitest/runtests.py.in
}
out of which only runtests.py.in actually uses |start| yet.
So, my proposal is to merge
{
488 # print test run times
489 finish = datetime.now()
490 log.info(" started: %s", str(start))
491 log.info("finished: %s", str(finish))
}
into automation.runApp(), remove the |start| return value and print something like
{
log.info("application run for: %s", str(datetime.now() - start))
}
|
|
Assignee | |
Comment 8•16 years ago
|
||
Iirc, Ted said he doesn't care much, but would accept this proposal :-)
|
|
Assignee | |
Comment 9•16 years ago
|
||
Comment 6 proposal, instead of Av1.
(Untested, but +/- trivial)
Attachment #365556 -
Flags: review?(jwalden+bmo)
|
|
||
Updated•16 years ago
|
Attachment #365556 -
Flags: review?(jwalden+bmo) → review+
|
|
Assignee | |
Updated•16 years ago
|
Attachment #364029 -
Attachment is obsolete: true
|
|
Assignee | |
Comment 10•16 years ago
|
||
Bv1, but timing the actual application only.
Attachment #365556 -
Attachment is obsolete: true
|
|
Assignee | |
Comment 11•16 years ago
|
||
Comment on attachment 365681 [details] [diff] [review]
(Bv1a) Replace external times by internal duration ++
[Checkin: See comment 11 & 12+13]
http://hg.mozilla.org/mozilla-central/rev/3622aadb857b
Attachment #365681 -
Attachment description: (Bv1a) Replace external times by internal duration ++ → (Bv1a) Replace external times by internal duration ++
[Checkin: Comment 11]
|
|
Assignee | |
Updated•16 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Depends on: 476163
Resolution: --- → FIXED
Whiteboard: [needs 1.9.1 landing: after bug 476163]
|
|
Assignee | |
Updated•16 years ago
|
Whiteboard: [needs 1.9.1 landing: after bug 476163] → [needs 1.9.1 landing: after bug 460515]
|
||
Comment 12•16 years ago
|
||
Pushed to 1.9.1:
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/4fb4287c24a0
Keywords: fixed1.9.1
Whiteboard: [needs 1.9.1 landing: after bug 460515]
|
|
Assignee | |
Updated•16 years ago
|
Attachment #365681 -
Attachment description: (Bv1a) Replace external times by internal duration ++
[Checkin: Comment 11] → (Bv1a) Replace external times by internal duration ++
[Checkin: See comment 11 & 12]
|
|
Assignee | |
Updated•16 years ago
|
Whiteboard: [fixed1.9.1b4]
|
|
Assignee | |
Comment 13•16 years ago
|
||
Comment on attachment 365681 [details] [diff] [review]
(Bv1a) Replace external times by internal duration ++
[Checkin: See comment 11 & 12+13]
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/1805229f044e
(Cv1-191) Resync' automation.py.in with m-c, missed part
Attachment #365681 -
Attachment description: (Bv1a) Replace external times by internal duration ++
[Checkin: See comment 11 & 12] → (Bv1a) Replace external times by internal duration ++
[Checkin: See comment 11 & 12+13]
|
|
Assignee | |
Updated•16 years ago
|
Flags: in-testsuite-
You need to log in
before you can comment on or make changes to this bug.
Description
•