Hello Zygmunt,

On Tue, 27 Mar 2012 13:48:30 +0200 Zygmunt Krynicki zygmunt.krynicki@linaro.org wrote:

W dniu 27.03.2012 13:32, Paul Sokolovsky pisze:

...

Hello,

Yong Qin is working on the blueprint https://blueprints.launchpad.net/lava-android-test/+spec/modify-android-buil... to add arbitrary custom client-side scripts to Android Build. He submitted first implementation of it as https://code.launchpad.net/~liuyq0307/linaro-android-build-tools/run-custom/... and documented as https://wiki.linaro.org/Platform/Android/AndroidBuild-LavaIntegration .

Unfortunately, I'm not thrilled with that implementation, more specifically, its "user interface" (i.e. any parts which user directly faces) by the following reasons:

1. The idea behind Android Build's build config was that they're

short and easy for human to parse, essentially one glance-over would enough to get a good idea what is built here, even for outsider. Consequently, the configs should not be overloaded with details not related to building. If there's a need for integration with other systems, we have good pattern of externalizing such details and then just referring to them with a single variable in a build config.

2. The whole approach in

https://wiki.linaro.org/Platform/Android/AndroidBuild-LavaIntegration seems like trying to encode hierarchical structure in the shell syntax, which is not much supporting of that. The end result looks pretty much like representation of graph structure in raw assembler

• it's spaghetti mix of data pieces and labels, requiring long time

to wrap hand around to understand it, and cumbersome and error-prone to write.

So, I would like to propose alternative syntax solving the issues above. I probably should start with saying that if the talk is about LAVA, then using native LAVA JSON request immediately comes to mind. Well, I guess human-writability wasn't a design goal for that, so I skip it. It still makes sense to stick to general-purpose hierarchical structure syntax though. Except that JSON has 2 problems: a) it doesn't support comments natively, so we'll need to pre-process it; b) error reporting/localization may be still no ideal.

Hi, just jumping into the conversation briefly to look at small technical aspect. I have not really been tracking this command effort and I don't understand what it's about.

Yeah, me too, I just spent some time reviewing Yong Qin's examples and code. And my main concern about it is external interface/syntax which users face on android-build.linaro.org. I'm ok with any underlying implementation.

On JSON: I think that comment 2 is inaccurate. We have very precise syntax error reporting (down to line/column and text range on some errors) and even better format reporting (the javascript expression that pinpoints the part of json document that does not match the schema, same for the schema actually).

Yes, I meant a bit different thing, let me elaborate:

1. LAVA has API which accepts JSON. You say that you parse that JSON very carefully, that's cool! But that native API syntax unlikely was designed to be easily written by human from scratch, right? 2. So, let us on android-build side (could be reused if proved successful) to develop another, deliberately human-optimized syntax, and let it be still based on JSON. 3. Then, android-build side script will a) read such human-friendly test def (example: https://wiki.linaro.org/Platform/Android/AndroidBuild-LavaIntegration/pfalco...), b) transform it into native LAVA syntax (essentially, dereference URLs and add machine-readable fields), and c) submit as such. 4. My concern with error reporting and comment support was for 3a step. Well, I just checked that python's json module does down-to-char error reporting, which is cool. And comments can be handled by a simple regexp substitution, so we're all good.

So, with this plan, there won't be any code changes from LAVA team (assuming that running arbitrary test commands already supported by its API), but we'd for sure appreciate sanity and approach review.

Anyway, you have my full support for native json formats. I think that if comments are an issue I can provide a parser that simply ignores comments. We could then keep the human readable documents and strictly machine readable, schema-backed data.

Thanks ZK

...

Anyway, here's my try, it is presented as a comment to https://wiki.linaro.org/Platform/Android/AndroidBuild-LavaIntegration and then full example at https://wiki.linaro.org/Platform/Android/AndroidBuild-LavaIntegration/pfalco...

Let's discuss if that covers our needs and constraints.

Thanks, Paul

Linaro.org | Open source software for ARM SoCs Follow Linaro: http://www.facebook.com/pages/Linaro http://twitter.com/#%21/linaroorg - http://www.linaro.org/linaro-blog

---

linaro-validation mailing list linaro-validation@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-validation

On Tue, 27 Mar 2012 09:22:30 -0500 Andy Doan andy.doan@linaro.org wrote:

On 03/27/2012 06:32 AM, Paul Sokolovsky wrote:

...

So, I would like to propose alternative syntax solving the issues above. I probably should start with saying that if the talk is about LAVA, then using native LAVA JSON request immediately comes to mind. Well, I guess human-writability wasn't a design goal for that, so I skip it. It still makes sense to stick to general-purpose hierarchical structure syntax though. Except that JSON has 2 problems: a) it doesn't support comments natively, so we'll need to pre-process

I'm fine with the overall idea, but I'm not a big fan of adding support for comments for two reasons.

1. are these files really so exotic they need comments. ie:

Well, don't even let me start to rant how stupid was for JSON founding fathers to explicitly exclude comment support ;-). My logic though would be very simple: it's human readable, so excluding humans from its processors was really unwise. Well, even CPU vendors include NOP in instruction sets, not saying that "LD A,A" or "JMP $+N" should be enough for fillers.

# run monkey { "commands": [ "monkey 2000"], } I don't see those type of comments as useful

How about:

# We run 1000 iterations above - that must complete successfully for # build to be good. Let's now run 2000 iterations as additional optional # test of build stability.

?

2. if comments are needed couldn't we work around it and still be

valid JSON by doing something like:

{ "comment": "Let's pretend this is useful", "commands": [ "monkey 2000"], }

Well, I don't know about other folks, but 80% of my comment usage is not to add human-readable textual notes, but to temporarily disable some code/data, to be re-enabled later.

That said, I'm not the one who'd be writing those tests, so if folks who are target audience of this feature can testify they don't need meta-syntax comments, then well... On the other hand, supporting them is one preprocessing call to re.sub(), so no implementation chore. Methodologically, we can just consider that we're using "Extended JSON", it's certainly better for this case than inventing adhoc syntax or using XML.

On 29 March 2012 06:27, Paul Sokolovsky paul.sokolovsky@linaro.org wrote:

Hello Zach,

On Wed, 28 Mar 2012 12:05:46 -0500 Zach Pfeffer zach.pfeffer@linaro.org wrote:

...

On 27 March 2012 06:32, Paul Sokolovsky paul.sokolovsky@linaro.org wrote:

...

Hello,

Yong Qin is working on the blueprint https://blueprints.launchpad.net/lava-android-test/+spec/modify-android-buil... to add arbitrary custom client-side scripts to Android Build. He submitted first implementation of it as https://code.launchpad.net/~liuyq0307/linaro-android-build-tools/run-custom/... and documented as https://wiki.linaro.org/Platform/Android/AndroidBuild-LavaIntegration .

Unfortunately, I'm not thrilled with that implementation, more specifically, its "user interface" (i.e. any parts which user directly faces) by the following reasons:

1. The idea behind Android Build's build config was that they're

short and easy for human to parse, essentially one glance-over would enough to get a good idea what is built here, even for outsider. Consequently, the configs should not be overloaded with details not related to building. If there's a need for integration with other systems, we have good pattern of externalizing such details and then just referring to them with a single variable in a build config.

2. The whole approach in

https://wiki.linaro.org/Platform/Android/AndroidBuild-LavaIntegration seems like trying to encode hierarchical structure in the shell syntax, which is not much supporting of that. The end result looks pretty much like representation of graph structure in raw assembler

• it's spaghetti mix of data pieces and labels, requiring long time

to wrap hand around to understand it, and cumbersome and error-prone to write.

So, I would like to propose alternative syntax solving the issues above. I probably should start with saying that if the talk is about LAVA, then using native LAVA JSON request immediately comes to mind. Well, I guess human-writability wasn't a design goal for that, so I skip it. It still makes sense to stick to general-purpose hierarchical structure syntax though. Except that JSON has 2 problems: a) it doesn't support comments natively, so we'll need to pre-process it; b) error reporting/localization may be still no ideal.

Anyway, here's my try, it is presented as a comment to https://wiki.linaro.org/Platform/Android/AndroidBuild-LavaIntegration and then full example at https://wiki.linaro.org/Platform/Android/AndroidBuild-LavaIntegration/pfalco...

Let's discuss if that covers our needs and constraints.

Paul, thanks for the alternative proposal.

The request was:

Provide a easy way to pass a test into LAVA from an android-build configuration.

Yong Qin's approach seems reasonable because its of the form:

MY_DEFINE="runmycommand --paramone 1 --paramtwo 2"

Well, not quite, example at https://wiki.linaro.org/Platform/Android/AndroidBuild-LavaIntegration shows 20 cross-referencing lines for defining tests (with build configuration per se taking 5).

Right, so this is simpler.

...

I don't like Paul's proposal because if you need something that complicated you should just use the LAVA python interface. I just want something simple.

Something like:

LAVA_TEST_1="runmycommand --paramone 1 --paramtwo 2" LAVA_TEST_PARSE_1="some regex"

LAVA_TEST_2="runmyothercommand --paramone 1" LAVA_TEST_PARSE_2="some other regex"

..and that's it.

Well, if it's about "parsing", it's never that simple. Yong Qin's example shows what typical parsing regexp looks like.

Sure.

...

LAVA would then run LAVA_TEST_1 and LAVA_TEST_2 etc... and use LAVA_TEST_PARSE_1 and LAVA_TEST_PARSE_2 to parse the results nad send them back to the android-build.

All the tests we run on Android will be baked into the build, so we don't have to worry about installing them.

I don't even want to have to do this:

LAVA_TESTS="LAVA_TEST_1 LAVA_TEST_2"

...because that's adding too much structure.

But if you have too many things without structure, mess ensues. In particular, it's unclear when these tests run in respect to LAVA_TEST_PLAN.

At a higher level all your going to have is a flat list:

LAVA_TEST_1="runmycommand --paramone 1 --paramtwo 2" LAVA_TEST_PARSE_1="some regex"

LAVA_TEST_2="runmyothercommand --paramone 1" LAVA_TEST_PARSE_2="some other regex"

LAVA_TEST_3="runmycommand --paramone 1 --paramtwo 2" LAVA_TEST_PARSE_3="some regex"

LAVA_TEST_4="runmyothercommand --paramone 1" LAVA_TEST_PARSE_4="some other regex"

LAVA_TEST_5="runmycommand --paramone 1 --paramtwo 2" LAVA_TEST_PARSE_5="some regex"

LAVA_TEST_6="runmyothercommand --paramone 1" LAVA_TEST_PARSE_6="some other regex"

etc...

...that's pretty simple structure wise.

What I'm advocating for is just this simple solution, which is simpler even than Yong Qin's.

...

This is really just a way to get a test in without needing to add a bunch of python code to LAVA. The flat list of commands is easy to see, and there's no tree structure to contend with. If you need more structure you'd write some python or write the structure into a script.

Well, all in all, what you described is quite different from what is presented at https://wiki.linaro.org/Platform/Android/AndroidBuild-LavaIntegration (and implemented in 100 lines (50% of existing code size) at https://code.launchpad.net/~liuyq0307/linaro-android-build-tools/run-custom/...) , so probably lack of proper spec to start with was the issue here.

These are just sketches. The spec is:

Provide a simple way to pass a test to LAVA from Android build and we have a couple of examples now:

1. Youg Qins 2. Yours 3. Mine

I presented mine because its extremely simple and if you want anything more complicated you'll put it in the test repo.

If we reduce the requirements to just supporting textual (no external URL references) content in

+1

LAVA_TEST_${N} LAVA_TEST_PARSE_${N}

and well defined order to run them (for example, after LAVA_TEST_PLAN), then it certainly will be good enough to put into build configs(though parsing regexps would still look hairy there imho).

Cool. Lets do it.

-- Best Regards, Paul

Linaro.org | Open source software for ARM SoCs Follow Linaro: http://www.facebook.com/pages/Linaro http://twitter.com/#%21/linaroorg - http://www.linaro.org/linaro-blog