William Lachance (:wlach) Reporter
	Description • 10 years ago

According to jgraham, the solution is to import the blessing module and use it when not using mozlog.structured in conjunction with mach. This also means adding blessings to the dependency list for mozlog.

Example:

    (mozbase)wlach@eideticker:~/src/eideticker$ cat lt.py
    #!/usr/bin/env python
     
    import argparse
    import sys
     
    from mozlog.structured import structuredlog, commandline, get_default_logger
     
    parser = argparse.ArgumentParser()
    commandline.add_logging_group(parser)
     
    args = parser.parse_args()
    logger = commandline.setup_logging("structured-example", args, {"raw": sys.stdout})
    logger.info("Running tests")
     
Result:

(mozbase)wlach@eideticker:~/src/eideticker$ python lt.py --log-mach_terminal -
Traceback (most recent call last):
  File "lt.py", line 15, in <module>
    logger.info("Running tests")
  File "/home/wlach/src/mozilla-central-hg/testing/mozbase/mozlog/mozlog/structured/structuredlog.py", line 223, in log
    self._log_data("log", data)
  File "/home/wlach/src/mozilla-central-hg/testing/mozbase/mozlog/mozlog/structured/structuredlog.py", line 123, in _log_data
    handler(log_data)
  File "/home/wlach/src/mozilla-central-hg/testing/mozbase/mozlog/mozlog/structured/handlers/__init__.py", line 60, in __call__
    formatted = self.formatter(data)
  File "/home/wlach/src/mozilla-central-hg/testing/mozbase/mozlog/mozlog/structured/formatters/machformatter.py", line 146, in __call__
    t = self.terminal.blue(format_seconds(self._time(data)))
AttributeError: 'NoneType' object has no attribute 'blue'

	James Graham [:jgraham] Assignee
	Comment 1 • 10 years ago

chmanchester: this also changes the presentation of results where there are no subtests.
You might want to look at that (I didn't test it properly yet so it might even be broken).

	Chris Manchester (limited bugmail, email directly)
	Comment 2 • 10 years ago

Comment on attachment 8441463 [details] [diff] [review]
Make mach terminal formatter work outside mach context.

Review of attachment 8441463 [details] [diff] [review]:
-----------------------------------------------------------------

New output looks good with a fix to the KeyError, and the colored bits work.

::: testing/mozbase/mozlog/mozlog/structured/formatters/machformatter.py
@@ +67,5 @@
>  
> +            #Reset the counts to 0
> +            test = self._get_test_id(data)
> +            self.status_buffer[test] = {"count": 0, "unexpected": 0, "pass": 0}
> +            self.has_unexpected[test] = bool(unexpected)

I get a key error in _colorize at line 170 because this isn't set if subtests is None.

	William Lachance (:wlach) Reporter
	Comment 5 • 10 years ago

Comment on attachment 8443593 [details] [diff] [review]
Make mach terminal formatter work outside mach context.

Review of attachment 8443593 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, thanks!

::: testing/mozbase/mozlog/mozlog/structured/formatters/machformatter.py
@@ +3,5 @@
>  # file, You can obtain one at http://mozilla.org/MPL/2.0/.
>  
>  import time
>  
> +import blessings

Is there any reason there's blank lines between these imports?

	James Graham [:jgraham] Assignee
	Comment 6 • 10 years ago

"""Imports should be grouped in the following order:

    standard library imports
    related third party imports
    local application/library specific imports

You should put a blank line between each group of imports.""" - http://legacy.python.org/dev/peps/pep-0008/#imports

	William Lachance (:wlach) Reporter
	Comment 7 • 10 years ago

(In reply to James Graham [:jgraham] from comment #6)
> """Imports should be grouped in the following order:
> 
>     standard library imports
>     related third party imports
>     local application/library specific imports
> 
> You should put a blank line between each group of imports.""" -
> http://legacy.python.org/dev/peps/pep-0008/#imports

Wow, you learn something new every day. :)

	James Graham [:jgraham] Assignee
	Comment 8 • 10 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/53b54cd44410

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 9 • 10 years ago

This apparently broke everything: https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=53b54cd44410

Backed out in  https://hg.mozilla.org/integration/mozilla-inbound/rev/8b3b7b0e9f81

	James Graham [:jgraham] Assignee
	Comment 10 • 10 years ago

Sorry about that :(

I hadn't considered that blessings, which is in tree, isn't also available in the mozharness environment. It isn't really needed, so we could work around it somehow, or put it on the internal PyPI.

	William Lachance (:wlach) Reporter
	Comment 11 • 10 years ago

Filed bug 1028386 to get blessings into internal pypi

	James Graham [:jgraham] Assignee
	Comment 12 • 10 years ago

Relanded this after a promising-looking try run

https://hg.mozilla.org/integration/mozilla-inbound/rev/1566b80f6c2b

	Ed Morley [:emorley]
	Comment 13 • 10 years ago

Backed out for missing curses:
https://tbpl.mozilla.org/php/getParsedLog.php?id=42273383&tree=Mozilla-Inbound
Traceback (most recent call last):
  File "c:/builds/moz2_slave/m-in-w32-000000000000000000000/build/testing/mozbase/test.py", line 114, in <module>
    main()
  File "c:/builds/moz2_slave/m-in-w32-000000000000000000000/build/testing/mozbase/test.py", line 101, in main
    unittestlist.extend(unittests(test))
  File "c:/builds/moz2_slave/m-in-w32-000000000000000000000/build/testing/mozbase/test.py", line 52, in unittests
    module = imp.load_source(modname, path)
  File "c:\builds\moz2_slave\m-in-w32-000000000000000000000\build\testing\mozbase\mozlog\tests\test_structured.py", line 8, in <module>
    from mozlog.structured import (
  File "c:\builds\moz2_slave\m-in-w32-000000000000000000000\build\testing\mozbase\mozlog\mozlog\structured\__init__.py", line 5, in <module>
    import commandline
  File "c:\builds\moz2_slave\m-in-w32-000000000000000000000\build\testing\mozbase\mozlog\mozlog\structured\commandline.py", line 11, in <module>
    import formatters
  File "c:\builds\moz2_slave\m-in-w32-000000000000000000000\build\testing\mozbase\mozlog\mozlog\structured\formatters\__init__.py", line 9, in <module>
    from machformatter import MachFormatter, MachTerminalFormatter
  File "c:\builds\moz2_slave\m-in-w32-000000000000000000000\build\testing\mozbase\mozlog\mozlog\structured\formatters\machformatter.py", line 7, in <module>
    import blessings
  File "c:\builds\moz2_slave\m-in-w32-000000000000000000000\build\python\blessings\blessings\__init__.py", line 2, in <module>
    import curses
  File "c:\mozilla-build\python27\Lib\curses\__init__.py", line 15, in <module>
    from _curses import *
ImportError: No module named _curses

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/6a1dd10c12c2

	James Graham [:jgraham] Assignee
	Comment 14 • 10 years ago

I think we need to try/catch the blessings import and not create the formatter if we can't import the module.

	James Graham [:jgraham] Assignee
	Comment 16 • 10 years ago

https://tbpl.mozilla.org/?tree=Try&rev=624beb4e337e

	Chris Manchester (limited bugmail, email directly)
	Comment 17 • 10 years ago

Comment on attachment 8464855 [details] [diff] [review]
Make mach terminal formatter work outside mach context

Review of attachment 8464855 [details] [diff] [review]:
-----------------------------------------------------------------

Works great running locally, let's go with it if it passes try.

::: testing/mozbase/mozlog/mozlog/structured/formatters/machformatter.py
@@ +52,5 @@
> +            time = self.terminal.blue(time)
> +
> +            color = None
> +
> +            if data["action"] == "test_end":

I don't know if we have a use case for it right now, but test_status messages might also benefit from color.

	James Graham [:jgraham] Assignee
	Comment 18 • 10 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/13c5f1142de9

	Ryan VanderMeulen [:RyanVM]
	Comment 19 • 10 years ago

https://hg.mozilla.org/mozilla-central/rev/13c5f1142de9