tko/parsers: make status_line parsing more robust to bad inputs

tko/parsers: make status_line parsing more robust to bad inputs

In the process, I fixed a unittest that was failing (handles_newline)
and added a new case to it. I also changed the logic to not throw
and assertion if it can't options and updated the unittest
accordingly.

BUG=7262
TEST=Ran unittests.

Change-Id: I2c8ed4613c9f965f0259cc1afdfdef7dc9c2ea62

[ericli, 2010-10-11 23:11:14]

coincident.

Dale opened an identical CL this morning.
http://codereview.chromium.org/3719002

<http://codereview.chromium.org/3719002>+dalecurtis

On Mon, Oct 11, 2010 at 4:04 PM, <msb@chromium.org> wrote:

> Reviewers: petkov, ericli,
>
> Description:
> tko/parsers: make status_line parsing more robust to bad inputs
>
> In the process, I fixed a unittest that was failing (handles_newline)
> and added a new case to it. I also changed the logic to not throw
> and assertion if it can't options and updated the unittest
> accordingly.
>
> BUG=7262
> TEST=Ran unittests.
>
> Change-Id: I2c8ed4613c9f965f0259cc1afdfdef7dc9c2ea62
>
> Please review this at http://codereview.chromium.org/3713002/show
>
> SVN Base: http://git.chromium.org/git/autotest.git
>
> Affected files:
>  M tko/parsers/version_0.py
>  M tko/parsers/version_0_unittest.py
>
>
> Index: tko/parsers/version_0.py
> diff --git a/tko/parsers/version_0.py b/tko/parsers/version_0.py
> index
>
7e8a5d884c598c69a2eb41a2a48e9d062ca4c20f..b061f2a75cb9b410840a2fd38d5d45c360ad0f0a
> 100644
> --- a/tko/parsers/version_0.py
> +++ b/tko/parsers/version_0.py
> @@ -251,17 +251,26 @@ class status_line(object):
>     def parse_line(cls, line):
>         if not status_line.is_status_line(line):
>             return None
> -        indent, line = re.search(r"^(\t*)(.*)$", line).groups()
> +        # We sometimes get multi-line input, bug?
> +        lines = line.splitlines()
> +        if len(lines) > 1:
> +            tko_utils.dprint('Got multi-line input to parse_line().')
> +            tko_utils.dprint('Input lines: %s' % repr(lines))
> +        indent, line = re.search(r"^(\t*)(.*)$", line, re.DOTALL).groups()
>         indent = len(indent)
>
>         # split the line into the fixed and optional fields
> -        parts = line.split("\t")
> +        parts = line.rstrip().split("\t")
>         status, subdir, testname = parts[0:3]
>         reason = parts[-1]
>         optional_parts = parts[3:-1]
>
>         # all the optional parts should be of the form "key=value"
> -        assert sum('=' not in part for part in optional_parts) == 0
> +        if sum('=' not in part for part in optional_parts) != 0:
> +            # this may not have been a status line
> +            tko_utils.dprint('This does not look like a status line,
> skipping.')
> +            tko_utils.dprint('Line: %s' % line)
> +            return None
>         optional_fields = dict(part.split("=", 1)
>                                for part in optional_parts)
>
> Index: tko/parsers/version_0_unittest.py
> diff --git a/tko/parsers/version_0_unittest.py
> b/tko/parsers/version_0_unittest.py
> index
>
4807beecb67d0283844a458ece68a1b420ccc96a..ddcfd09dd18d4870dbe3c72257521ab537816e30
> 100755
> --- a/tko/parsers/version_0_unittest.py
> +++ b/tko/parsers/version_0_unittest.py
> @@ -216,7 +216,7 @@ class test_status_line(unittest.TestCase):
>     def test_parse_line_handles_newline(self):
>         input_data = ("\t\tGOOD\t----\t----\t"
>                       "field1=val1\tfield2=val2\tNo newline here!")
> -        for suffix in ("", "\n"):
> +        for suffix in ("", "\n", "\n\n"):
>             line = version_0.status_line.parse_line(input_data +
>                                                     suffix)
>             self.assertEquals(line.indent, 2)
> @@ -261,9 +261,8 @@ class test_status_line(unittest.TestCase):
>
>     def test_parse_line_fails_on_bad_optional_fields(self):
>         input_data = "GOOD\tfield1\tfield2\tfield3\tfield4"
> -        self.assertRaises(AssertionError,
> -                          version_0.status_line.parse_line,
> -                          input_data)
> +        self.assertEquals(None,
> +                          version_0.status_line.parse_line(input_data))
>
>
>  if __name__ == "__main__":
>
>
>


-- 
Eric Li
李咏竹
Google Kirkland

[Mandeep Singh Baines, 2010-10-11 23:14:14]

On 2010/10/11 23:11:14, ericli wrote:
> coincident.
> 
> Dale opened an identical CL this morning.
> http://codereview.chromium.org/3719002
> 

Awesome. I'll close this one then.

> <http://codereview.chromium.org/3719002%3E+dalecurtis
> 
> On Mon, Oct 11, 2010 at 4:04 PM, <mailto:msb@chromium.org> wrote:
> 
> > Reviewers: petkov, ericli,
> >
> > Description:
> > tko/parsers: make status_line parsing more robust to bad inputs
> >
> > In the process, I fixed a unittest that was failing (handles_newline)
> > and added a new case to it. I also changed the logic to not throw
> > and assertion if it can't options and updated the unittest
> > accordingly.
> >
> > BUG=7262
> > TEST=Ran unittests.
> >
> > Change-Id: I2c8ed4613c9f965f0259cc1afdfdef7dc9c2ea62
> >
> > Please review this at http://codereview.chromium.org/3713002/show
> >
> > SVN Base: http://git.chromium.org/git/autotest.git
> >
> > Affected files:
> >  M tko/parsers/version_0.py
> >  M tko/parsers/version_0_unittest.py
> >
> >
> > Index: tko/parsers/version_0.py
> > diff --git a/tko/parsers/version_0.py b/tko/parsers/version_0.py
> > index
> >
>
7e8a5d884c598c69a2eb41a2a48e9d062ca4c20f..b061f2a75cb9b410840a2fd38d5d45c360ad0f0a
> > 100644
> > --- a/tko/parsers/version_0.py
> > +++ b/tko/parsers/version_0.py
> > @@ -251,17 +251,26 @@ class status_line(object):
> >     def parse_line(cls, line):
> >         if not status_line.is_status_line(line):
> >             return None
> > -        indent, line = re.search(r"^(\t*)(.*)$", line).groups()
> > +        # We sometimes get multi-line input, bug?
> > +        lines = line.splitlines()
> > +        if len(lines) > 1:
> > +            tko_utils.dprint('Got multi-line input to parse_line().')
> > +            tko_utils.dprint('Input lines: %s' % repr(lines))
> > +        indent, line = re.search(r"^(\t*)(.*)$", line, re.DOTALL).groups()
> >         indent = len(indent)
> >
> >         # split the line into the fixed and optional fields
> > -        parts = line.split("\t")
> > +        parts = line.rstrip().split("\t")
> >         status, subdir, testname = parts[0:3]
> >         reason = parts[-1]
> >         optional_parts = parts[3:-1]
> >
> >         # all the optional parts should be of the form "key=value"
> > -        assert sum('=' not in part for part in optional_parts) == 0
> > +        if sum('=' not in part for part in optional_parts) != 0:
> > +            # this may not have been a status line
> > +            tko_utils.dprint('This does not look like a status line,
> > skipping.')
> > +            tko_utils.dprint('Line: %s' % line)
> > +            return None
> >         optional_fields = dict(part.split("=", 1)
> >                                for part in optional_parts)
> >
> > Index: tko/parsers/version_0_unittest.py
> > diff --git a/tko/parsers/version_0_unittest.py
> > b/tko/parsers/version_0_unittest.py
> > index
> >
>
4807beecb67d0283844a458ece68a1b420ccc96a..ddcfd09dd18d4870dbe3c72257521ab537816e30
> > 100755
> > --- a/tko/parsers/version_0_unittest.py
> > +++ b/tko/parsers/version_0_unittest.py
> > @@ -216,7 +216,7 @@ class test_status_line(unittest.TestCase):
> >     def test_parse_line_handles_newline(self):
> >         input_data = ("\t\tGOOD\t----\t----\t"
> >                       "field1=val1\tfield2=val2\tNo newline here!")
> > -        for suffix in ("", "\n"):
> > +        for suffix in ("", "\n", "\n\n"):
> >             line = version_0.status_line.parse_line(input_data +
> >                                                     suffix)
> >             self.assertEquals(line.indent, 2)
> > @@ -261,9 +261,8 @@ class test_status_line(unittest.TestCase):
> >
> >     def test_parse_line_fails_on_bad_optional_fields(self):
> >         input_data = "GOOD\tfield1\tfield2\tfield3\tfield4"
> > -        self.assertRaises(AssertionError,
> > -                          version_0.status_line.parse_line,
> > -                          input_data)
> > +        self.assertEquals(None,
> > +                          version_0.status_line.parse_line(input_data))
> >
> >
> >  if __name__ == "__main__":
> >
> >
> >
> 
> 
> -- 
> Eric Li
> 李咏竹
> Google Kirkland
>