[mozlog] Allow users to specify formatter color information in their machrc

RESOLVED FIXED in Firefox 68

Status

enhancement
P3
normal
RESOLVED FIXED
2 years ago
5 months ago

People

(Reporter: ahal, Assigned: v.singhal373, Mentored)

Tracking

({good-first-bug})

Version 3
mozilla68
Points:
---

Firefox Tracking Flags

(firefox68 fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

Many of the formatters hardcode color information. It would be neat if we could give users a place to specify this in their machrc files. Maybe something like:

[color]
log.suite_start = yellow
log.process_output = blue
log.test_status.pass = green
log.test_status.fail = red

What these values mean would depend on the formatter being used.

I guess from mozlog's point of view, we'd need a way to pass in a color dict, and then the various test mach commands would be responsible for pulling this information out of machrc.
Priority: -- → P3
Mentor: james
Keywords: good-first-bug

Hi,
I wish to work on this issue.

Hi, My name is Ana and I am an outreachy applicant. Can I work on this issue?

I can't assign this to both of you, but I think it's OK if you both have a go at making patches for this bug. Otherwise if you prefer to select an alternate bug to work on please go ahead.

Please how do I test after making the required changes

Alright James, I am an Outreachy applicant, and would like to give this bug a go too - how is this working? are you not assigning it until you receive the first patch? If that is the case, I'd like to give it a crack. Cheers :)

(In reply to kerlyn from comment #4)

Please how do I test after making the required changes

Try running something like

./mach wpt --metadata=. testing/web-platform/tests/infrastructure

that should give some output that includes failures. I don't think we have automated testing for the actual colouring, but mach python-test will run the existing mozlog tests.

Hi James,
Since I cannot fine any other good-first-bug that is not assigned and will give this one a try.
Thanks!

Hi James,
I am an outreachy applicant and i want to work on this bug.Can i work on this issue?
Thanks.

Flags: needinfo?(james)

Hello James,

I am an outreachy applicant and I am interested to work on this bug! Can you provide some guidance?

Do you have any suggestions related to other tasks I can do?
I saw there were not so many bugs - Do you know some other related project where applicants can have a look at and work on issues?

I have been working with JavaSript and Python - My github profile is: https://github.com/AlbionaHoti

Thank you, Albiona

I'm really sorry about the contention for bugs; I'm happy for people to pick any python bug in the mozbase component, but if it's not mozlog they should verify with me first that it's suitable.

In terms of this bug, the key parts are:

In the machformatter we currently call a bunch of colour functions on a case-by-case basis e.g. https://searchfox.org/mozilla-central/source/testing/mozbase/mozlog/mozlog/formatters/machformatter.py#58 The first step here is to factor those out into a dictionary so that we end up with something like

colors = {"time": "dim_blue", …}

That could for example be a class-level dictionary. Then those values need to be mapped to the attributes of self.term when the class is constructed.

The second part of the problem is to figure out how to get from mach settings to override the values in that formatter. The key constraint here is that mozlog may not rely on mach but mach can rely on mozlog. So we can't have mozlog itself try to read the settings file; instead there must be some way to pass it in. The simplest approach there would be to have some part of the mach command setup override the dinctionary, but there's likley a better approach. I would suggest that we have that discussion after there's a patch for the first part of the bug, to replace the direct uses of colour with symbolic names.

Flags: needinfo?(james)

Hi James,

I'm an Outreachy applicant and was trying to fix this issue. I'm mostly done solving this, but had a few doubts in mind:

  • Currently the mach/config.py doesn't seem to allow more than one period-separated parameters in the mach configuration setings, i.e it'll only work with 'a.b' and not 'a.b.c' as the description mentions.

    Check line 368 in config.py: shouldn't it be section, option = setting[0].split('.', 1) instead? I think this can allow for more detailed granularity in config param names.

  • My understanding is that all settings need to be explicitly defined within @SettingsProvider, however according to the mach docs, the * wildcard allows for 'arbitrary settings' to be defined. The settings still need to be defined within the @SettingsProvider right?

Thanks!

Hey! I am a Outreachy applicant.
It will be great if I get to contribute here!
Please assign me this issue.
Thank you!
Dhruvi

(In reply to v.singhal373 from comment #11)

  • Currently the mach/config.py doesn't seem to allow more than one period-separated parameters in the mach configuration setings, i.e it'll only work with 'a.b' and not 'a.b.c' as the description mentions.

    Check line 368 in config.py: shouldn't it be section, option = setting[0].split('.', 1) instead? I think this can allow for more detailed granularity in config param names.

Yes, I think you're correct that the existing code has a bug here; it should either allow multiple dots or reject that explictly rather than just failing to unpack a tuple. Fixing that seems good, but I think the specification with multiple dots in the name should be regarded as flexible; we can also consider replacing the later dots with _ or similar.

  • My understanding is that all settings need to be explicitly defined within @SettingsProvider, however according to the mach docs, the * wildcard allows for 'arbitrary settings' to be defined. The settings still need to be defined within the @SettingsProvider right?

I think that you always need to define a SettingsProvider for the settings to be registered. See [1] for a relevant example.

BTW if you have a partial patch I would be very interested to see it sooner rather than later, if possible.

[1] https://searchfox.org/mozilla-central/source/testing/mach_commands.py#57

(In reply to Dhruvi Butti from comment #12)

Hey! I am a Outreachy applicant.
It will be great if I get to contribute here!
Please assign me this issue.
Thank you!
Dhruvi

Note that due to a shortage of good first bugs I'm not assigning issues to people since there may be more than one person attempting a particular fix at once. I know this isn't ideal, but it's hard to find enough bugs of a suitable level for everyone who's interested.

The default color dictionary is implemented,to avoid hardcode color information. The functionality for overridding the values in formatter, is hitherto partially
implemented

Pushed by james@hoppipolla.co.uk:
https://hg.mozilla.org/integration/autoland/rev/53bd5240052b
[mozlog] added color dictionary and functionality for overriding formatter r=jgraham

Backed out changeset 53bd5240052b (bug 1424287) for mozabase failures at testing/mozbase/mozlog/tests/test_formatters.py

Backout: https://hg.mozilla.org/integration/autoland/rev/9f03791063253f1d0b5992af1c37db0eb7894a8e

Failure push: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=234809648&revision=53bd5240052b91d99740750f9c36c997ec2df315

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=234809696&repo=autoland&lineNumber=693

[task 2019-03-19T16:31:53.447Z] 0:35.86 platform linux -- Python 3.5.2, pytest-3.6.2, py-1.5.4, pluggy-0.6.0 -- /builds/worker/checkouts/gecko/obj-x86_64-pc-linux-gnu/_virtualenvs/gecko-QUm8IANb-3/bin/python
[task 2019-03-19T16:31:53.447Z] 0:35.86 rootdir: /builds/worker/checkouts/gecko, inifile: /builds/worker/checkouts/gecko/config/mozunit/mozunit/pytest.ini
[task 2019-03-19T16:31:53.447Z] 0:35.86 collecting ... collected 1 item
[task 2019-03-19T16:31:53.448Z] 0:35.86
[task 2019-03-19T16:31:53.448Z] 0:35.86 testing/mozbase/mozinstall/tests/test_is_installer.py::test_is_installer PASSED
[task 2019-03-19T16:31:53.448Z] 0:35.86
[task 2019-03-19T16:31:53.448Z] 0:35.86 =========================== 1 passed in 0.03 seconds ===========================
[task 2019-03-19T16:31:53.614Z] 0:36.03 /builds/worker/checkouts/gecko/testing/mozbase/mozinstall/tests/test_uninstall.py
[task 2019-03-19T16:31:53.615Z] 0:36.03 ============================= test session starts ==============================
[task 2019-03-19T16:31:53.615Z] 0:36.03 platform linux -- Python 3.5.2, pytest-3.6.2, py-1.5.4, pluggy-0.6.0 -- /builds/worker/checkouts/gecko/obj-x86_64-pc-linux-gnu/_virtualenvs/gecko-QUm8IANb-3/bin/python
[task 2019-03-19T16:31:53.615Z] 0:36.03 rootdir: /builds/worker/checkouts/gecko, inifile: /builds/worker/checkouts/gecko/config/mozunit/mozunit/pytest.ini
[task 2019-03-19T16:31:53.615Z] 0:36.03 collecting ... collected 1 item
[task 2019-03-19T16:31:53.616Z] 0:36.03
[task 2019-03-19T16:31:53.616Z] 0:36.03 testing/mozbase/mozinstall/tests/test_uninstall.py::test_uninstall PASSED
[task 2019-03-19T16:31:53.616Z] 0:36.03
[task 2019-03-19T16:31:53.616Z] 0:36.03 =========================== 1 passed in 0.04 seconds ===========================
[task 2019-03-19T16:31:54.051Z] 0:36.47 /builds/worker/checkouts/gecko/testing/mozbase/mozlog/tests/test_capture.py
[task 2019-03-19T16:31:54.051Z] 0:36.47 ============================= test session starts ==============================
[task 2019-03-19T16:31:54.052Z] 0:36.47 platform linux -- Python 3.5.2, pytest-3.6.2, py-1.5.4, pluggy-0.6.0 -- /builds/worker/checkouts/gecko/obj-x86_64-pc-linux-gnu/_virtualenvs/gecko-QUm8IANb-3/bin/python
[task 2019-03-19T16:31:54.067Z] 0:36.47 rootdir: /builds/worker/checkouts/gecko, inifile: /builds/worker/checkouts/gecko/config/mozunit/mozunit/pytest.ini
[task 2019-03-19T16:31:54.067Z] 0:36.48 collecting ... collected 1 item
[task 2019-03-19T16:31:54.067Z] 0:36.48
[task 2019-03-19T16:31:54.068Z] 0:36.48 testing/mozbase/mozlog/tests/test_capture.py::TestCaptureIO::test_captureio_log PASSED
[task 2019-03-19T16:31:54.068Z] 0:36.48
[task 2019-03-19T16:31:54.068Z] 0:36.48 =========================== 1 passed in 0.09 seconds ===========================
[task 2019-03-19T16:31:54.700Z] 0:37.11 /builds/worker/checkouts/gecko/testing/mozbase/mozlog/tests/test_formatters.py
[task 2019-03-19T16:31:54.700Z] 0:37.11 ============================= test session starts ==============================
[task 2019-03-19T16:31:54.700Z] 0:37.11 platform linux -- Python 3.5.2, pytest-3.6.2, py-1.5.4, pluggy-0.6.0 -- /builds/worker/checkouts/gecko/obj-x86_64-pc-linux-gnu/_virtualenvs/gecko-QUm8IANb-3/bin/python
[task 2019-03-19T16:31:54.702Z] 0:37.12 rootdir: /builds/worker/checkouts/gecko, inifile: /builds/worker/checkouts/gecko/config/mozunit/mozunit/pytest.ini
[task 2019-03-19T16:31:54.703Z] 0:37.12 collecting ... collected 4 items
[task 2019-03-19T16:31:54.703Z] 0:37.12
[task 2019-03-19T16:31:54.703Z] 0:37.12 testing/mozbase/mozlog/tests/test_formatters.py::test_pass[mach] TEST-UNEXPECTED-FAIL
[task 2019-03-19T16:31:54.704Z] 0:37.12 testing/mozbase/mozlog/tests/test_formatters.py::test_pass[mach-verbose=True] TEST-UNEXPECTED-FAIL
[task 2019-03-19T16:31:54.704Z] 0:37.12 testing/mozbase/mozlog/tests/test_formatters.py::test_fail[mach] TEST-UNEXPECTED-FAIL
[task 2019-03-19T16:31:54.705Z] 0:37.12 testing/mozbase/mozlog/tests/test_formatters.py::test_fail[mach-verbose=True] TEST-UNEXPECTED-FAIL
[task 2019-03-19T16:31:54.705Z] 0:37.12
[task 2019-03-19T16:31:54.705Z] 0:37.12 =================================== FAILURES ===================================

Flags: needinfo?(james)
Posted file machformatter.py (obsolete) —

Changed dict.itertitems() to dict.items()

Attachment #9052221 - Attachment is obsolete: true
Attachment #9052221 - Attachment is patch: false

The default color dictionary is implemented, to avoid formattor specifying the colors discretly

Pushed by james@hoppipolla.co.uk:
https://hg.mozilla.org/integration/autoland/rev/6f59938c2690
[mozlog] added color dictionary and TerminalColors class r=jgraham
Status: NEW → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Assignee: nobody → v.singhal373
Flags: needinfo?(james)
Regressions: 1541569
You need to log in before you can comment on or make changes to this bug.