792934 - Rework custom logger levels

Status: RESOLVED INVALID

(Testing Graveyard :: Mozmill, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Currently our logger levels are a bit strange and need to be reworked. Some output will not appear in the levels we would expect. So what we currently have is:

        self.custom_levels = {
            "RESULTS": 1000,
            "TEST-UNEXPECTED-PASS": 43,
            "TEST-UNEXPECTED-FAIL": 42,
            "TEST-SKIPPED": 31,
            "TEST-KNOWN-FAIL": 23,
            "TEST-PASS": 22,
            "TEST-START": 21,
            }

That means that "TEST-UNEXPECTED-FAIL" would not fall into "ERROR", which is bad. :/

Here my proposal:

* "RESULT" we leave as is
* "TEST-UNEXPECTED-PASS" and "TEST-UNEXPECTED-FAIL" should be part of "ERROR".
* "TEST-SKIPPED" should be part of "WARNING".
* "TEST-KNOWN-FAIL", "TEST-PASS", and "TEST-START" should be part of "INFO".

So all the affected numbers would have to be decreased, so that they are below the appropriate default level. Does that sound fine?

Comment 1

[Pranav Ravichandran [:pranavrc]]

Attachment: Reworked levels.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 663822 [details] [diff] [review]
Reworked levels.

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

::: mozmill/mozmill/logger.py
@@ +37,5 @@
>  
>          self.custom_levels = {
>              "RESULTS": 1000,
> +            "TEST-UNEXPECTED-PASS": 33,
> +            "TEST-UNEXPECTED-FAIL": 32,

logging.ERROR is 40, so I think that we should count downwards. This would also apply to the other changes. Jeff, what do you think of?

Comment 3

[Jeff Hammel]

So I'm actually beginning to think that we should have a list in order from least severe to most severe.  Strictly speaking, the numbers don't really mean anything: only the orders matter.  But that can be done later.  For now, I have no strong preference.

Comment 4

[Jeff Hammel]

Comment on attachment 663822 [details] [diff] [review]
Reworked levels.

this works for me, unless :whimboo has objections

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to Jeff Hammel [:jhammel] from comment #3)
> So I'm actually beginning to think that we should have a list in order from
> least severe to most severe.  Strictly speaking, the numbers don't really
> mean anything: only the orders matter.  But that can be done later.  For
> now, I have no strong preference.

Would you mind to file a new bug with specific details? Not sure what you mean by list and how this should work with the logging module. What we have right now is a list sorted by severity and the items map to the logging constants which are numbers. So further input would be appreciated.

(In reply to Jeff Hammel [:jhammel] from comment #4)
> this works for me, unless :whimboo has objections

I would like to see my mentioned nits addressed. Once done I'm happy to check-in.

Comment 6

[Jeff Hammel]

> Would you mind to file a new bug with specific details?

Filed bug 793853 . I hope it is clear.  The point is that the numbers don't really matter for our purposes.  All we care about is the order.  That said, it is a nice to have, not a huge win.

Comment 7

[Pranav Ravichandran [:pranavrc]]

Attachment: patch v2

About the other bug, do the levels have to be reassigned so that they're numerically close to each other?

Comment 8

[Jeff Hammel]

(In reply to Pranav Ravichandran [:pranavrc] from comment #7)
> Created attachment 664279 [details] [diff] [review]
> patch v2
> 
> About the other bug, do the levels have to be reassigned so that they're
> numerically close to each other?

bug 793853 implies that we don't care about the numerical values, which I don't think we do.  I don't know if we can map multiple levels to a single numerical value (though if we can, I'm not sure why we don't, though IIRC python logging doesn't like that which is a limitation in its code).  But if we can't, then we mostly want 

a) an ordering of levels
b) a console level that you can set from the command line which that and all higher levels are printed to stdout

TL;DR outside of the fact that the numbers we're assigning to don't really mean anything (they're just a substitute for ordering, the way that we use them) the main purpose of decoupling order from numerical level is familiar to BASIC programmers from way back when:  let's say you have levels 30-40 accounted for and you need to insert a level in that range... If we must have a bijection of integers to levels, you need to rewrite many numbers to do this.  If you just have a list of levels then just put the new level where you want it and you're done.

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Thank you Pranav for your first contribution to the Mozmill project. The changeset is not in our master repository!

https://github.com/mozilla/mozmill/commit/972589136c98dbe9f51f00067646018fc3f86118

Lets do any follow-up discussion about logger levels on the newly created bug. This one is fixed now.

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+1]]

As it turns out we made a mistake here. But that shows how complicated those logging levels are! It's hard to understand.

So with this change in place we do no longer see the TEST-START and other log messages when running at the INFO level. :(

I backed this out:
https://github.com/mozilla/mozmill/commit/ab1e9366e77866ec1dfb842236fba4fb07a0e6c5

And sorry again for this wrong assumption.