google / mobly

E2E test framework for tests with complex environment requirements.
https://github.com/google/mobly
Apache License 2.0
628 stars 175 forks source link

[MEP] Support pdb feature for better debugging functionality #739

Closed johnklee closed 3 years ago

johnklee commented 3 years ago

Here I use MEP which stands for Mobly Enhancement Proposal (Similar to PEPs as Python Enhancement Proposals). To return, I am a newbie to use mobly and spend a lot of time to trace the code of this repo. However, I realized mobly doesn't support pdb in nature and therefore I have to type in the command below in order to leverage the pdb in order to learn how mobly work:

johnkclee$ python -m pdb my_helloworld_test.py -c sample_config.yaml
> /usr/local/google/home/johnkclee/Github/mobly/my_helloworld_test.py(1)<module>()
-> from mobly import base_test
(Pdb)

Then I have to setup breakpoint manually to stop at certain line of code in order to figure out how mobly work which is not quite friendly for newcomer like me. So I have add a few code into mobly and make it able to break at any position I want. Take below script for example:

class HelloWorldTest(base_test.BaseTestClass): def setup_class(self):

Registering android_device controller module declares the test's

# dependency on Android device hardware. By default, we expect at least one
# object is created from this.
self.ads = self.register_controller(android_device)
self.dut = self.ads[0]
# Start Mobly Bundled Snippets (MBS).
self.dut.load_snippet('mbs', 'com.google.android.mobly.snippet.bundled')

def test_hello(self): self.dut.mbs.makeToast('Hello World!') self.pdb("test")

if name == 'main': test_runner.main()

Then I can execute the script without argument `-m pdb` and can still break at any line with `self.pdb(...)`. For example:
```console
johnkclee$ python my_helloworld_test.py -c sample_config.yaml
[SampleTestBed] 05-03 08:35:33.808 INFO Test output folder: "/tmp/SampleTestBed/05-03-2021_08-35-33-807"
[SampleTestBed] 05-03 08:35:33.808 WARNING pdb: Skip PDBable...
[SampleTestBed] 05-03 08:35:33.809 INFO ==========> HelloWorldTest <==========
[SampleTestBed] 05-03 08:35:36.562 INFO [AndroidDevice|07311JECB08252] Launching snippet apk com.google.android.mobly.snippet.bundled with protocol 1.0
[SampleTestBed] 05-03 08:35:36.990 INFO [Test] test_hello
[SampleTestBed] 05-03 08:35:37.011 WARNING pdb: test
--Return--
> /usr/local/google/home/johnkclee/Github/mobly/my_helloworld_test.py(18)test_hello()->None
-> self.pdb("test")
(Pdb) list
 13         # Start Mobly Bundled Snippets (MBS).
 14         self.dut.load_snippet('mbs', 'com.google.android.mobly.snippet.bundled')
 15     
 16       def test_hello(self):
 17         self.dut.mbs.makeToast('Hello World!')
 18  ->     self.pdb("test")
 19     
 20     
 21     if __name__ == '__main__':
 22       test_runner.main()
[EOF]
(Pdb)

I have code ready from my local and this is a proposal for mobly to be more friendly to be studied by users first time to use it. If this proposal is approved. I will fork a branch for my local implementation and ask for review by requesting merge later.

Many thanks!

xpconanfan commented 3 years ago

Thanks for the proposal. Can you upload the PR so we can take a look and decide? :)

johnklee commented 3 years ago

The commit for reference is as below from my fork: https://github.com/johnklee/mobly/commit/bb97e6b6f940005f42fb21d0dec7a583744e4d30

If it looks good to you, I will create a PR then. Many thanks!

johnklee commented 3 years ago

By the way, I tried to use flake8 to confirm that my code doesn't break any PEP8 regulations and found below issues:

johnkclee$ flake8 mobly/
mobly/base_instrumentation_test.py:577:67: W504 line break after binary operator
mobly/base_instrumentation_test.py:697:30: W503 line break before binary operator
mobly/base_instrumentation_test.py:697:67: W504 line break after binary operator
mobly/base_instrumentation_test.py:721:9: E125 continuation line with same indent as next logical line
mobly/base_instrumentation_test.py:735:7: E125 continuation line with same indent as next logical line
mobly/base_instrumentation_test.py:831:69: W504 line break after binary operator
mobly/suite_runner.py:100:7: E722 do not use bare 'except'
mobly/logger.py:26:91: E501 line too long (92 > 90 characters)
mobly/records.py:23:1: F401 'sys' imported but unused
mobly/records.py:98:1: F811 redefinition of unused 'Error' from line 38
mobly/records.py:471:5: E741 ambiguous variable name 'l'
mobly/records.py:646:5: E741 ambiguous variable name 'l'
mobly/controller_manager.py:122:5: E722 do not use bare 'except'
mobly/controller_manager.py:207:5: E125 continuation line with same indent as next logical line
mobly/test_runner.py:17:1: F401 'inspect' imported but unused
mobly/test_runner.py:80:7: E722 do not use bare 'except'
mobly/test_runner.py:168:3: E722 do not use bare 'except'
mobly/controllers/iperf_server.py:19:1: F401 'subprocess' imported but unused
mobly/controllers/iperf_server.py:31:5: E722 do not use bare 'except'
mobly/controllers/iperf_server.py:40:5: E722 do not use bare 'except'
mobly/controllers/monsoon.py:36:1: F401 'mobly.controllers.android_device' imported but unused
mobly/controllers/monsoon.py:147:9: F841 local variable 'e' is assigned to but never used
mobly/controllers/monsoon.py:152:9: F841 local variable 'e' is assigned to but never used
mobly/controllers/monsoon.py:528:9: E741 ambiguous variable name 'l'
mobly/controllers/monsoon.py:755:5: F841 local variable 'history_deque' is assigned to but never used
mobly/controllers/monsoon.py:791:5: F841 local variable 'e' is assigned to but never used
mobly/controllers/monsoon.py:800:5: E722 do not use bare 'except'
mobly/controllers/attenuator.py:36:1: F401 'logging' imported but unused
mobly/controllers/attenuator.py:67:5: F841 local variable 'instances' is assigned to but never used
mobly/controllers/sniffer_lib/local/tshark.py:31:53: W504 line break after binary operator
mobly/controllers/sniffer_lib/local/tshark.py:40:12: F524 '...'.format(...) is missing argument(s) for placeholder(s): 1
mobly/controllers/sniffer_lib/local/local_base.py:23:1: F401 'signal' imported but unused
mobly/controllers/android_device_lib/sl4a_client.py:110:9: E722 do not use bare 'except'
mobly/controllers/android_device_lib/sl4a_client.py:129:7: E722 do not use bare 'except'
mobly/controllers/android_device_lib/sl4a_client.py:143:7: E722 do not use bare 'except'
mobly/controllers/android_device_lib/event_dispatcher.py:65:7: E722 do not use bare 'except'
mobly/controllers/android_device_lib/event_dispatcher.py:272:7: E265 block comment should start with '# '
mobly/controllers/android_device_lib/event_dispatcher.py:295:11: E722 do not use bare 'except'
mobly/controllers/android_device_lib/snippet_client.py:41:78: W504 line break after binary operator
mobly/controllers/android_device_lib/snippet_client.py:44:74: W504 line break after binary operator
mobly/controllers/android_device_lib/snippet_client.py:132:7: E722 do not use bare 'except'
mobly/controllers/android_device_lib/snippet_client.py:207:5: E722 do not use bare 'except'
mobly/controllers/android_device_lib/snippet_client.py:315:54: W504 line break after binary operator
mobly/controllers/android_device_lib/snippet_client.py:316:11: E129 visually indented line with same indent as next logical line
mobly/controllers/android_device_lib/adb.py:37:91: E501 line too long (92 > 90 characters)
mobly/controllers/android_device_lib/service_manager.py:18:1: F401 'contextlib' imported but unused
mobly/controllers/android_device_lib/jsonrpc_shell_base.py:35:5: F901 'raise NotImplemented' should be 'raise NotImplementedError'
mobly/controllers/android_device_lib/jsonrpc_shell_base.py:42:5: F901 'raise NotImplemented' should be 'raise NotImplementedError'
mobly/controllers/android_device_lib/jsonrpc_client_base.py:48:3: F401 'encodings.idna' imported but unused
mobly/controllers/android_device_lib/jsonrpc_client_base.py:57:1: F401 'sys' imported but unused
mobly/controllers/android_device_lib/services/logcat.py:242:5: E722 do not use bare 'except'
mobly/controllers/attenuator_lib/telnet_scpi_client.py:73:25: W605 invalid escape sequence '\S'
mobly/controllers/attenuator_lib/telnet_scpi_client.py:79:77: W504 line break after binary operator

I can fix them from this MEP too. FYI.

johnklee commented 3 years ago

It will be quite useful/helpful if the execution of the test case will enter PDB console when any exception happens. So I add a decorator debug_on in module mobly/utils.py (line 98-111) for my convenience to trace the code of mobly and conduct the debugging right in the context which cause the exception. FYI

xpconanfan commented 3 years ago

Sorry, but I don't think we'll take this change as the benefit is not significant compared to the change needed. It is not difficult to add a couple of lines for pdb if someone needs it.

Feel free to fix the warnings though :)

johnklee commented 3 years ago

Understood. Thanks for the feedback and reply. I will fix those warnings in this issue if you don't mind. And in this case, I may have to change the title of this issue too...

xpconanfan commented 3 years ago

Can just open another one :) Also pls break the fixes into multiple PRs