Riot: [RFC] abstraction layer for test commands

[MrKevinWeiss]

Description

To help with writing tests conforming to a convention I suggest we add an abstraction layer for use with writing tests.

The concept is instead of printing results without structure (though obvious for humans) we use an abstraction for the results. Not only should this simplify writing tests but also allow easier automation of testing. It allows changing format without changing test logic, for example, a serializer can replace the printf messages to save size.

Here is an example API.

The results of calling the functions would look something like:

# turo_data_int(TH_DEV, 12)
data: 12

# turo_data_bool(TH_DEV, true)
data: True

# turo_data_string(TH_DEV, "foo")
data: "foo"

# turo_data_array(TH_DEV, [1, -1, 256, 0xFFFF, 0xFFFFFFFF], INT32_TYPE, 5);
data: [1,-1,256,65536,-1]

# turo_data_string_object(TH_DEV, "foo", "bar")
data: {"foo": "bar"}

# turo_data_string_object(TH_DEV, "foo", 42)
data: {"foo": 42}

#turo_result_success()
result: Success

#turo_result_error()
result: Error

# turo_result_error_with_code(-ENODEV)
error_code: -12
result: Error

Note, for futureproofing I have a int turo for different contexts.

The format is chosen since it is very readable and easy enough for the firmware to create and for a higher level language to parse (I would rather have the parsing logic on the high level rather than in the firmware).
It is also doesn't require large buffers or states. The data printout is versatile. It is similar to what we have now for some applications. It is very human readable. It also allows debug messages in-between without interfering with the parsing.
I intentionally leave out all the different types of primitives, int32_t` covers many cases and this can get extended as needed.

Here are some questions I would like feedback on:

1. Should this be a different module or combined with the fmt library?
2. How is the naming of the module (test_helpers) and th namespacing?
3. Do I really need to specify each function as _print_?
4. For the array print should I just have a function for each type or have an enum situation?

Feedback is appreciated!

The long term goal will be to define a testing convention and integrate a riotctrl module and easily automate shell/api based testing.

Useful links

Discussed in #10624

[MrKevinWeiss]

ping @miri64 @kaspar030 @fjmolinas
Maybe some quick feedback on the questions at the bottom to help direct implementation :blush:

[MrKevinWeiss]

Actually it probably should fit in the test_utils area.

[MrKevinWeiss]

Here is an pr to show what it could look like

[miri64]

Here is an pr to show what it could look like

The link leads here ;-) but the PR number can be fetched from the anchor.

[fjmolinas]

Actually it probably should fit in the test_utils area.

better have it in sys, this might be usefull in other places as well IMO.

[fjmolinas]

Thanks for bringing this up @MrKevinWeiss Some questions regarding the current api:

• how would you handle more complex JSON, where its not a simple {"key" : "value"}?
• in the RFC "data" was supposed to be JSON, this is not the case here, and related to this why prefix with "data:"?

During the summit it was mentioned the idea of being able to switch between JSON and human readable output (I think @kaspar030 suggested this), have you given this some thought as well?

Should this be a different module or combined with the fmt library?

IMO different module

How is the naming of the module (test_helpers) and th namespacing?

Seems to me that if we are printing json, json should be somewhere in the name no?, I don't have a prenference regarding the turo name.

Do I really need to specify each function as print?

Maybe this api does not need to do the actual printing but only return a formatted string? It would make it more re-usable for other applications as well, and the you would simply printf("%s",, turo_string)

or the array print should I just have a function for each type or have an enum situation?

What is the use-cases you for-see for turo_data_array.

[MrKevinWeiss]

The link leads here ;-) but the PR number can be fetched from the anchor.

Just trying to get a little traffic 😈

[MrKevinWeiss]

how would you handle more complex JSON, where its not a simple {"key" : "value"}?

The idea would be that you can expand as needed, so lets say we want a {"key": <list_of_strings>} or a {"key": <ip_addr>}, we would just add it to the test api.

Right now we can do a lot with what we have.

[MrKevinWeiss]

in the RFC "data" was supposed to be JSON, this is not the case here, and related to this why prefix with "data:"?

I had json in the old RDM and implemented it in the test_helpers from RobotFramework. I want to go away from that because I noticed some limitations and complexity that didn't seem that useful.
By prefixing lines with data:, result:, and error_code: we can easily classify things for parses, which is what will come next (with riotctrl).

That isn't to say one could just implement a json printout for the implementation. I want to make it flexible and extendable because I don't know every case. At least we can start writing tests with these and future changes would be easier.

[MrKevinWeiss]

During the summit it was mentioned the idea of being able to switch between JSON and human readable output (I think @kaspar030 suggested this), have you given this some thought as well?

This is why I would like to switch to this module. @kaspar had a good point in saying that we do not want a compile time switch as it could lead to different behaviour (ie a command may not work in json because the buffer size is too small but works with the human readable).

The data: <something_that_can_be_parsed_in_one_line_in_python> seems to be much cleaner than json and easy to parse.

[MrKevinWeiss]

IMO different module

Ok I will do that! _(I assume you mean different from test_utils as well)_

[MrKevinWeiss]

Seems to me that if we are printing json, json should be somewhere in the name no?, I don't have a prenference regarding the turo name.

again it is not really json (currently), just some convention of data, but thanks, naming is the harding thing about programming it seems 😆

[MrKevinWeiss]

Maybe this api does not need to do the actual printing but only return a formatted string? It would make it more re-usable for other applications as well, and the you would simply printf("%s",, turo_string)

That is an idea, I was really targeting convenience. It may also make the abstraction implementation a bit harder, we would assume the result output would always be in string form... Maybe we want it to show on an lcd or just blink morse code on leds or something. I wanted whatever the implementation of the turo_data_int() to be selectable.

[MrKevinWeiss]

What is the use-cases you for-see for turo_data_array

Heh, in the test that I updated!

Another more practical example is for outputting the results of i2c_read_regs.

[MrKevinWeiss]

Thanks @fjmolinas for the feedback, I think it is really helpful and I hope others say what they want.

Some things changed from the original question once the implementation was done for example I settled on having a print array for each data type (as it is more typesafe and just cleaner).

[fjmolinas]

how would you handle more complex JSON, where its not a simple {"key" : "value"}?

The idea would be that you can expand as needed, so lets say we want a {"key": <list_of_strings>} or a {"key": <ip_addr>}, we would just add it to the test api.

Right now we can do a lot with what we have.

With this I meant JSON of the type {"key1" : "value1", "key2" : "value2", "key3" : { "subkey1" : "value"} }. But ok.

[fjmolinas]

That is an idea, I was really targeting convenience. It may also make the abstraction implementation a bit harder, we would assume the result output would always be in string form... Maybe we want it to show on an lcd or just blink morse code on leds or something. I wanted whatever the implementation of the turo_data_int() to be selectable.

Or send it out, it might not be over serial.

[MrKevinWeiss]

With this I meant JSON of the type {"key1" : "value1", "key2" : "value2", "key3" : { "subkey1" : "value"} }. But ok.

Maybe some aspects could be taken care from the riotctrl parser (ie. if all data is dicts than combine into one dict)

Or add some api like:

void turo_data_object_start(...);
void turo_data_object_continue(...);
void turo_data_object_end(...);

or just something with flags...

But I still think that keeping things simple is the best way to go. Lots can be done with what is provided and it doesn't lock us in.

[MrKevinWeiss]

Anyone see any big blockers here?

If we can somehow agree that the idea is OK we can move on to implementation discussion.

I think also ping @aabadie