1334488 - [manifestparser] Improve error message when ini parser fails to find a new section

Status: RESOLVED FIXED

(Testing :: Mozbase, defect)

Description

[Andrew Halberstadt [:ahal]]

This is a spin off from bug 1333564 comment 17.

Currently we print "No sections found" if an unexpected token is found at the start of a manifest:
https://dxr.mozilla.org/mozilla-central/rev/1e0e193b0812f68a12fbd69198552af62347af1e/testing/mozbase/manifestparser/manifestparser/ini.py#73

Instead it should probably say something like:
"Expected a comment or section, instead found ';'."

While we're doing this we also might as well create a new IniParseError exception or something similar.

Comment 1

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1334488 - [manifestparser] Improve error message when using an invalid comment character at the start of a manifest,

This refactors some of the error logic into a new IniParseError exception. In addition to checking for a 'name' property, we also
check for a 'path' property. This is because some file-like objects coming out of the build system use a 'path' attribute instead
of the standard 'name'.

Review commit: https://reviewboard.mozilla.org/r/107774/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/107774/

Comment 2

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8831149 [details]
Bug 1334488 - [manifestparser] Improve error message when using an invalid comment character at the start of a manifest,

https://reviewboard.mozilla.org/r/107774/#review108934

::: testing/mozbase/manifestparser/manifestparser/ini.py:105
(Diff revision 1)
>              continue
>  
>          # if there aren't any sections yet, something bad happen
>          if not section_names:
> -            raise Exception("Error parsing manifest file '%s', line %s: No sections found" %
> -                            (getattr(fp, 'name', 'unknown'), linenum))
> +            raise IniParseError(fp, linenum, "Expected a comment or section, "
> +                                             "instead found '{}'".format(stripped[0]))

why stripped[0], could we instead print stripped in full?

::: testing/mozbase/manifestparser/manifestparser/ini.py:129
(Diff revision 1)
>              if line[0].isspace() and key:
>                  value = '%s%s%s' % (value, os.linesep, stripped)
>                  current_section[key] = value
>              else:
>                  # something bad happened!
> -                raise Exception("Error parsing manifest file '%s', line %s" %
> +                raise IniParseError(fp, linenum, "Unexpected token '{}'".format(stripped[0]))

why stripped[0], could we instead print stripped in full?

Comment 3

[Andrew Halberstadt [:ahal]]

Comment on attachment 8831149 [details]
Bug 1334488 - [manifestparser] Improve error message when using an invalid comment character at the start of a manifest,

https://reviewboard.mozilla.org/r/107774/#review108934

> why stripped[0], could we instead print stripped in full?

Good point!

Comment 4

[Andrew Halberstadt [:ahal]]

Comment on attachment 8831149 [details]
Bug 1334488 - [manifestparser] Improve error message when using an invalid comment character at the start of a manifest,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/107774/diff/1-2/

Comment 5

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8831149 [details]
Bug 1334488 - [manifestparser] Improve error message when using an invalid comment character at the start of a manifest,

https://reviewboard.mozilla.org/r/107774/#review108948

thanks!

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a15be64c8440
[manifestparser] Improve error message when using an invalid comment character at the start of a manifest, r=jmaher

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/a15be64c8440