1435199 - [compare-locales] ParserTestMixin should use instanceof instead of type equality for non-entity entries

Status: RESOLVED FIXED

(Localization Infrastructure and Tools :: compare-locales, enhancement)

Description

[Axel Hecht [:Pike]]

Right now, ParserTestMixin._test tests type() against strings, which locks in the rest of the code base.

Let's be nice and use instanceof instead.

Comment 1

[Axel Hecht [:Pike]]

Attachment: bug 1435199, use assertIsInstance instead of type name comparison,

type(foo).name == 'ClassName' is ugly for one, but also
prevents us from subclassing parser classes easily.

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

Comment 2

[Staś Małolepszy :stas]

Comment on attachment 8947750 [details]
bug 1435199, use assertIsInstance instead of type name comparison,

https://reviewboard.mozilla.org/r/217460/#review223226

It looks much better this way, thanks!

Comment 3

[Axel Hecht [:Pike]]

Comment on attachment 8947750 [details]
bug 1435199, use assertIsInstance instead of type name comparison,

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

Comment 4

[Axel Hecht [:Pike]]

https://hg.mozilla.org/l10n/compare-locales/rev/df21885c38bc5c205afd75435c84cfdb125bd155, FIXED.