All, The "host list", "host create", and "job create" command line options for the atest CLI module do not support labels with commas. In our deployment we have several labels which contain commas, leading to some bad parsing when trying to use the CLI tools. I've added quoting support to the topic_common.item_parse_info.get_values.__get_items(...) function to fix this.
For example, previously 'atest host list --label label0,"comma,label"' would
not be parsed properly into two labels. Now the labels are parsed as
'label0' and 'comma,label'.
Any option using topic_common.item_parse_info gets this behavior for free.
Specifically for our needs, I've changed the "atest host list" and "atest
job create" methods from using <string>.split(',') to
topic_common.item_parse_info for labels.
Implementation wise, I modified the
topic_common.item_parse_info.get_values.__get_items(...) regex so that it
wouldn't split up items in quotes and changed the stripping so that leading
and trailing quotes were removed from the split items. The new regex is ([,
]|\\\".*?\\\".*?) if space splitting is enabled and ([,]|\\\".*?\\\".*?)
otherwise.
Testing was done through 18 new unit tests, running the gamut of good quotes
and mismatched quotes at the parser level as well as individual unit tests
for the "host list", "host create", and "job create" methods.
All unit tests pass, but if there are any deployments out there that were
using the CLI tools and have quotes in their labels (or any option relying
on topic_common.item_parse_info), they will no longer parse properly.
Another more minor quibble is consistency from a user point of view. I've
only added support for quotes to the topic_common.item_parse_info class, but
there are a couple other areas which are still using <string>.split(',') to
parse their options. Specifically status names, host names, test names, and
dependencies in the host.py and job.py files. I've left these alone for now,
but if anyone objects I can roll them in.
I've attached the patch file for the changes, but if you prefer a more GUI
driven experience, you can see the change list here:
http://codereview.chromium.org/2740001/show
Risk: Low-Medium (CLI infrastructure and tests modified)
Visibility: CLI users
Regards,
Dale Curtis
Software Engineer in Test @ Google
issue2740001_21001.diff
Description: Binary data
_______________________________________________ Autotest mailing list [email protected] http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
