(Submitting for dalecurtis. For some reason, Patchwork doesn't want to pick
up his patches...)
All,
First off, sorry for the spam, hopefully this will be last message and
Patchwork will finally pick up the patch. Reposted with modifications from
prior discussion per James Ren. Credit for suggestion to split on commas
goes to Jongki Suwandi.
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 escaped comma support to the
topic_common.item_parse_info.get_values.__get_items(...) function to fix
this.
For example, atest host list --label "label0,comma\,label" will now parse
the labels 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
would only split commas not preceded by slashes. In order to handle labels
that may end in slashes, I replace double slashes with a null character
before splitting and then put a single slash back afterwards. The new regex
is r'(?<!\),|\s' if space splitting is enabled and r'(?<!\),' otherwise.
Testing was done through 11 new unit tests, exercising escaped commas with
and without preceding escaped slashes. As well as manual testing on our own
deployment.
All unit tests pass and existing deployments should not be affected. A minor
quibble is consistency from a user point of view. I've only added support
for escaped commas 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
Signed-off-by: Dale Curtis
Regards,
Dale Curtis
Software Engineer in Test @ Google
git-svn-id: http://test.kernel.org/svn/autotest/trunk@4718 592f7852-d20e-0410-864c-8624ca9c26a4
diff --git a/cli/topic_common_unittest.py b/cli/topic_common_unittest.py
index 5f4284e..d3bdacb 100755
--- a/cli/topic_common_unittest.py
+++ b/cli/topic_common_unittest.py
@@ -161,6 +161,20 @@
self.__test_parsing_flist_good(opt(), ['a', 'b', 'c'])
+ def test_file_list_escaped_commas(self):
+ class opt(object):
+ flist_obj = cli_mock.create_file('a\nb\\,c\\,d\nef\\,g')
+ flist = flist_obj.name
+ self.__test_parsing_flist_good(opt(), ['a', 'b,c,d', 'ef,g'])
+
+
+ def test_file_list_escaped_commas_slashes(self):
+ class opt(object):
+ flist_obj = cli_mock.create_file('a\nb\\\\\\,c\\,d\nef\\\\,g')
+ flist = flist_obj.name
+ self.__test_parsing_flist_good(opt(), ['a', 'b\\,c,d', 'ef\\', 'g'])
+
+
def test_file_list_opt_list_one(self):
class opt(object):
inline = 'a'
@@ -191,6 +205,18 @@
self.__test_parsing_inline_good(opt(), ['a', 'b', 'c', 'd', 'e'])
+ def test_file_list_opt_list_escaped_commas(self):
+ class opt(object):
+ inline = 'a\\,b,c, d'
+ self.__test_parsing_inline_good(opt(), ['a,b', 'c', 'd'])
+
+
+ def test_file_list_opt_list_escaped_commas_slashes(self):
+ class opt(object):
+ inline = 'a\\,b\\\\\\,c,d,e'
+ self.__test_parsing_inline_good(opt(), ['a,b\\,c', 'd', 'e'])
+
+
def test_file_list_add_on_space(self):
self.__test_parsing_leftover_good(['a','c','b'],
['a', 'b', 'c'])
@@ -211,6 +237,16 @@
['a', 'b', 'c', 'd'])
+ def test_file_list_add_on_escaped_commas(self):
+ self.__test_parsing_leftover_good(['a', 'c', 'b,', 'd\\,e\\,f'],
+ ['a', 'b', 'c', 'd,e,f'])
+
+
+ def test_file_list_add_on_escaped_commas_slashes(self):
+ self.__test_parsing_leftover_good(['a', 'c', 'b,', 'd\\\\\\,e,f'],
+ ['a', 'b', 'c', 'd\\,e', 'f'])
+
+
def test_file_list_all_opt(self):
class opt(object):
flist_obj = cli_mock.create_file('f\ng\nh\n')
@@ -258,6 +294,26 @@
'f', 'g', 'h', 'i', 'j'])
+ def test_file_list_all_opt_in_common_escaped_commas(self):
+ class opt(object):
+ flist_obj = cli_mock.create_file('a\\,b\\,c\nd,e\nf\ng')
+ flist = flist_obj.name
+ inline = 'a\\,b\\,c,d h'
+ self.__test_parsing_all_good(opt(), ['i','j,d'],
+ ['a,b,c', 'd', 'e', 'f', 'g', 'h',
+ 'i', 'j'])
+
+
+ def test_file_list_all_opt_in_common_escaped_commas_slashes(self):
+ class opt(object):
+ flist_obj = cli_mock.create_file('a\\,b\\\\\\,c\nd,e\nf,ghi, ,, j,')
+ flist = flist_obj.name
+ inline = 'a\\,b\\\\\\,c,d h,ijk'
+ self.__test_parsing_all_good(opt(), ['i','j,d'],
+ ['a,b\\,c', 'd', 'e', 'f', 'ghi', 'h',
+ 'i', 'j', 'ijk'])
+
+
class atest_unittest(cli_mock.cli_unittest):
def setUp(self):
super(atest_unittest, self).setUp()