Mir

Merge lp://qastaging/~robertcarr/mir/fix-1227683 into lp://qastaging/mir

Proposed by Robert Carr
Status: Merged
Approved by: Daniel van Vugt
Approved revision: no longer in the source branch.
Merged at revision: 1178
Proposed branch: lp://qastaging/~robertcarr/mir/fix-1227683
Merge into: lp://qastaging/mir
Diff against target: 11 lines (+1/-1)
1 file modified
tests/acceptance-tests/test_client_input.cpp (+1/-1)
To merge this branch: bzr merge lp://qastaging/~robertcarr/mir/fix-1227683
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Daniel van Vugt Approve
Kevin DuBois Pending
Review via email: mp+193168@code.qastaging.launchpad.net

This proposal supersedes a proposal from 2013-10-28.

Commit message

test_client_input.cpp: Bump reception time-out in client test fixture.
(LP: #1227683)

Description of the change

Started investigating bug 1227683, the hidden clients test failure.

I was able to reproduce it quite consistently today (about 3/10 valgrind runs ><). My system is quite slow today somehow, I guess exaggerating the effect.

Through use of SESSION_MEDIATOR+INPUT+LEGACY_INPUT_REPORT log I found the second client was disconnecting before even being considered for the event. The event was then being dropped as there was no touched window.

Started investigating and found that the timeout fixed in this branch was still down at 2 seconds!

Bumped it to 60. Was able to run a 100 repetition run under valgrind with no failures.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Approve (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

Your description says you only bumped a timeout. Are you sure the other 4400 lines are related to fixing bug 1227683?

I'm fairly sure the moving of classes between components probably isn't related to the bug fix, at least.

review: Needs Information
Revision history for this message
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

Oh.... Proposed targeting the wrong branch. Please retarget to development-branch :)

review: Needs Resubmitting
Revision history for this message
Kevin DuBois (kdub) wrote : Posted in a previous version of this proposal

I'm curious too about moving the classes

review: Needs Information
Revision history for this message
Robert Carr (robertcarr) wrote :

Diff should look a little better :)

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

I would like to do manual testing of this to be sure it's fixed. But can't until our cross-compile setup agrees with trusty touch images (where I can reproduce the bug easily).

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Hmm, I can't reproduce the bug today even after fixing the cross compile setup. So blindly approved.

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches