Merge lp://qastaging/~liuyq0307/lava-android-test/support-custom-command into lp://qastaging/lava-android-test

18 + parser.add_argument('-O', '--run-option', help=("Specified in the job file for using in the real test action,"
19 + " so that we can customize some test when need"))

This is probably too long. I'd split it before help= if I were you. Running pep8 lava_android_test will tell you more.

+ except Exception as strerror:
37 + raise LavaCommandError("Test execution error: %s" % strerror)

By convention the exception should be named 'exc'. Using strerror is confusing as it may imply this is the C equivalent which it is not.

52 + parser.add_argument('-c', '--android-command', action='append', help=("Specified in the job file for using in the real test action,"
53 + " so that we can customize some test when need"))
54 + parser.add_argument('-p', '--parse-regex', help=("Specified the regular expression used for analyzing command output"))

Probably too long as well

74 + tip_msg = ''
75 + if self.args.serial:
76 + tip_msg = "Run following custom test(s) on device(%s):\n\t\t%s" % (self.args.serial, '\n\t\t'.join(ADB_SHELL_STEPS))
77 + else:
78 + tip_msg = "Run following custom test(s):\n\t\t%s" % ('\n\t\t'.join(ADB_SHELL_STEPS))

Line 74 is not needed. Python scoping will ensure that tip_msg is present after the if/else statement.

81 + inst = AndroidTestInstaller()
82 + run = AndroidTestRunner(adbshell_steps=ADB_SHELL_STEPS)
83 + parser = AndroidTestParser(pattern=PATTERN)
84 + test = AndroidTest(testname=test_name, installer=inst,
85 + runner=run, parser=parser)
86 + test.parser.results = {'test_results':[]}
87 + test.setadb(self.adb)

I cannot help to notice that we're duplicating part of lava-test. Could we consider depending on lava-test and reuse the TestArtifacts class defined there? I know this is a long topic so we can postpone this discussion but I want to reverse the trend so that we go towards unified APIs for both host-driven (lava-android-test, upcoming lava-uboot-test) and target-driven (lava-test) frameworks.

188 + #make every user can write/read this directory
189 + os.chmod(config.tempdir_host, 0777)

Why is this needed?

>
> 81 + inst = AndroidTestInstaller()
> 82 + run = AndroidTestRunner(adbshell_steps=ADB_SHELL_STEPS)
> 83 + parser = AndroidTestParser(pattern=PATTERN)
> 84 + test = AndroidTest(testname=test_name, installer=inst,
> 85 + runner=run, parser=parser)
> 86 + test.parser.results = {'test_results':[]}
> 87 + test.setadb(self.adb)
>
> I cannot help to notice that we're duplicating part of lava-test. Could we
> consider depending on lava-test and reuse the TestArtifacts class defined
> there? I know this is a long topic so we can postpone this discussion but I
> want to reverse the trend so that we go towards unified APIs for both host-
> driven (lava-android-test, upcoming lava-uboot-test) and target-driven (lava-
> test) frameworks.
Yes, I agree this, especial we will have more similar test tools.
There are some common parts between lava-test and lava-android-test now.
        lava-tools
            ^
            |
     lava-test-base
     ^ ^ ^
     | | |
  Linux Android Others

I create a BP about this.
https://blueprints.launchpad.net/lava-android-test/+spec/linaro-android-test-redesign

> 188 + #make every user can write/read this directory
> 189 + os.chmod(config.tempdir_host, 0777)
>
> Why is this needed?
Now there is a problem like this:
1. when use lava-instance to run android job, it will create /tmp/lava-android-test with the apache user,
2. then when I run lava-android-test as a normal user, I can't create temporary directory in /tmp/lava-android-test in this case.
so I think we can change the /tmp/lava-android-test to 777 when create it, then normal user can also use that directory.

464 + help=("Specified the regular expression used for \
465 + analyzing command output"))

This will not work as you think. The general consensus is to use

("text text text"
 " text text text" # repeat as many times as you want
 " more text")

Thanks for the blueprint! +1 :)

1. when use lava-instance to run android job, it will create /tmp/lava-android-test with the apache user,

Huh? What? Could you explain how this happens please? Lava runs as a designated user. Apache should _never_ be part of that.

2. then when I run lava-android-test as a normal user, I can't create temporary directory in /tmp/lava-android-test in this case.
so I think we can change the /tmp/lava-android-test to 777 when create it, then normal user can also use that directory.