Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add timing to logs and bump up the timeout further for start_app. #192

Merged
merged 3 commits into from
Apr 26, 2017

Conversation

dthkao
Copy link
Contributor

@dthkao dthkao commented Apr 26, 2017

This change is Reviewable

@xpconanfan
Copy link
Collaborator

Review status: 0 of 1 files reviewed at latest revision, 3 unresolved discussions, some commit checks failed.


mobly/controllers/android_device_lib/jsonrpc_client_base.py, line 51 at r1 (raw file):

# Maximum time to wait for the app to start on the device (10 minutes).
# TODO: This timeout is set high in order to allow for retries in start_app.
APP_START_WAIT_TIME = 10*60

no spaces around "*"?
maybe we need to run yapf on this file..


mobly/controllers/android_device_lib/jsonrpc_client_base.py, line 188 at r1 (raw file):

        self.check_app_installed()
        self._do_start_app()
        expiration_time = time.time() + wait_time

maybe we can use start_time in this line so we don't need to call time.time twice


mobly/controllers/android_device_lib/jsonrpc_client_base.py, line 194 at r1 (raw file):

            if self._is_app_running():
              self._log.debug('Successfully started %s after %d seconds.',
                              self.app_name, time.time()-start_time)

need spaces around "-"


Comments from Reviewable

@dthkao
Copy link
Contributor Author

dthkao commented Apr 26, 2017

Review status: 0 of 1 files reviewed at latest revision, 3 unresolved discussions.


mobly/controllers/android_device_lib/jsonrpc_client_base.py, line 51 at r1 (raw file):

Previously, xpconanfan (Ang Li) wrote…

no spaces around "*"?
maybe we need to run yapf on this file..

Done.


mobly/controllers/android_device_lib/jsonrpc_client_base.py, line 188 at r1 (raw file):

Previously, xpconanfan (Ang Li) wrote…

maybe we can use start_time in this line so we don't need to call time.time twice

Done.


mobly/controllers/android_device_lib/jsonrpc_client_base.py, line 194 at r1 (raw file):

Previously, xpconanfan (Ang Li) wrote…

need spaces around "-"

Done.


Comments from Reviewable

@xpconanfan
Copy link
Collaborator

:lgtm:


Review status: 0 of 1 files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@adorokhine
Copy link
Collaborator

Review status: 0 of 1 files reviewed at latest revision, 2 unresolved discussions.


mobly/controllers/android_device_lib/jsonrpc_client_base.py, line 50 at r2 (raw file):

# Maximum time to wait for the app to start on the device (10 minutes).
# TODO: This timeout is set high in order to allow for retries in start_app.

Not clear what the action item for this TODO is; could you please reword so that someone else would have enough context to do it later if needed?


mobly/controllers/android_device_lib/jsonrpc_client_base.py, line 193 at r2 (raw file):

            self._log.debug('Attempting to start %s.', self.app_name)
            if self._is_app_running():
                self._log.debug('Successfully started %s after %d seconds.',

How about %.1f seconds?


Comments from Reviewable

@dthkao
Copy link
Contributor Author

dthkao commented Apr 26, 2017

Review status: 0 of 1 files reviewed at latest revision, 2 unresolved discussions.


mobly/controllers/android_device_lib/jsonrpc_client_base.py, line 50 at r2 (raw file):

Previously, adorokhine (Alexander Dorokhine) wrote…

Not clear what the action item for this TODO is; could you please reword so that someone else would have enough context to do it later if needed?

Done.


mobly/controllers/android_device_lib/jsonrpc_client_base.py, line 193 at r2 (raw file):

Previously, adorokhine (Alexander Dorokhine) wrote…

How about %.1f seconds?

Done.


Comments from Reviewable

@adorokhine
Copy link
Collaborator

:lgtm:


Reviewed 1 of 1 files at r3.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@dthkao dthkao merged commit 5960e21 into master Apr 26, 2017
@xpconanfan xpconanfan deleted the startAppLogging branch May 2, 2017 23:52
@xpconanfan xpconanfan modified the milestones: Mobly Release 1.3.1, Mobly Release 1.4 May 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants