marionetteScriptFinished() has no effect in performance_helper_atom.

RESOLVED FIXED in Firefox OS v1.3

Status

P1
normal
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: hub, Assigned: hub)

Tracking

({perf})

unspecified
1.3 C1/1.4 S1(20dec)
x86_64
Linux
Dependency tree / graph

Firefox Tracking Flags

(b2g-v1.3 fixed)

Details

(Whiteboard: [c=automation p=2 s= u=])

Attachments

(1 attachment)

46 bytes, text/x-github-pull-request
julienw
: review+
Details | Review | Splinter Review
(Assignee)

Description

5 years ago
marionetteScriptFinished() has no effect in performance_helper_atom.

This is a followup from bug 915156

In tests/performance/app.js: waitForPerfEvents(), we call executeAsyncScript().
The atom is in ./tests/performance/performance_helper_atom.js, when the function finish() is called (we checked it is), marionetteScriptFinished() is called, but does nothing.

Upon closer inspection, it seems that in marionette-listener.js (Gecko) the commandId as passed to asyncComplete is different from the global asyncTestCommandId.

Note sure exactly what's happening yet, but this cause perf test to succeed on a 50s timeout.
(Assignee)

Updated

5 years ago
Blocks: 837633
Depends on: 915156
Keywords: perf
Whiteboard: [c=automation p=2 s= u=]
(Assignee)

Updated

5 years ago
Blocks: 916017
(Assignee)

Updated

5 years ago
Priority: -- → P1
(Assignee)

Updated

5 years ago
Assignee: nobody → hub
Status: NEW → ASSIGNED
(Assignee)

Comment 1

5 years ago
It seems that it is a problem with order of things, aka a misunderstanding on how it works. See pull request.
(Assignee)

Comment 2

5 years ago
Created attachment 8346717 [details] [review]
Pull request
Attachment #8346717 - Flags: review?(felash)
(Assignee)

Comment 3

5 years ago
Comment on attachment 8346717 [details] [review]
Pull request

cancelling review due to some things I forgot.
Attachment #8346717 - Flags: review?(felash)
(Assignee)

Updated

5 years ago
Depends on: 949209
(Assignee)

Comment 4

5 years ago
Comment on attachment 8346717 [details] [review]
Pull request

This time we should be good. Look ! No timeout !
Attachment #8346717 - Flags: review?(felash)
(Assignee)

Comment 5

5 years ago
Remove blocker. Isn't really blocked.
No longer depends on: 949209
Comment on attachment 8346717 [details] [review]
Pull request

Added some cleanup comments on the PR.

I admit I still don't understand how marionetteScriptFinished works but I'll trust you on this ;)
(Assignee)

Comment 7

5 years ago
I have updated the PR addressing the cleanup comments. Travis is only perma-orange.
Comment on attachment 8346717 [details] [review]
Pull request

r=me with the 2 additional comments
Attachment #8346717 - Flags: review?(felash) → review+
(Assignee)

Comment 9

5 years ago
Merged. https://github.com/mozilla-b2g/gaia/commit/9738f1d3bff9cc8fb5c92adf2c02f53ad6a52999
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Assignee)

Comment 10

5 years ago
shall we backport to 1.4? should be easy and would allow the test to run better on 1.4
blocking-b2g: --- → 1.4?
(Assignee)

Comment 11

5 years ago
I meant 1.3?
blocking-b2g: 1.4? → 1.3?
Let's just do it, it's a=npotb (not part of the build).

John, could you assist please?
blocking-b2g: 1.3? → ---
Flags: needinfo?(jhford)
[v1.3 81af9e2] Merge pull request #14621 from hfiguiere/bug947259
status-b2g-v1.3: --- → fixed
Flags: needinfo?(jhford)

Updated

5 years ago
Target Milestone: --- → 1.3 C1/1.4 S1(20dec)
You need to log in before you can comment on or make changes to this bug.