Closed Bug 1059609 Opened 7 years ago Closed 7 years ago

Mozlog should expose the set of supported log actions

Categories

(Testing :: Mozbase, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla35

People

(Reporter: chmanchester, Assigned: chmanchester)

Details

Attachments

(1 file, 1 obsolete file)

This came up in mochitests and corredor and is probably generally useful for validation. logtypes.py has each log action run through it at import, so we could do this without manually maintaining the list.
Assignee: nobody → cmanchester
Status: NEW → ASSIGNED
Comment on attachment 8482859 [details] [diff] [review]
Provide enumeration of known log actions in mozlog.structured.

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

I agree we should do this, but I think this isn't quite the right way to do it.

::: testing/mozbase/mozlog/mozlog/structured/logtypes.py
@@ +44,1 @@
>          convertor_registry[f.__name__] = self

Umm, isn't this just convertor_registry.keys()? Seems better to use that directly either via a helper like

def log_actions():
    return set(convertor_registry.keys())

or by setting log_actions = convertor_registry.keys()

I'm also not sure why this would be in logtypes.py which is basically an internal API rather than in structuredlog.py which is the external-facing code.

::: testing/mozbase/mozlog/tests/test_structured.py
@@ +638,5 @@
>          self.assertEquals(handler.action_1_count, 1)
>  
> +class TestActionEnumeration(unittest.TestCase):
> +
> +    def test_enumeration(self):

Is this test actually adding much value? I think it's just another thing to update when you add a new log action that you will probably forget. It's also much longer than the implementation, which is generally a bad sign.
Attachment #8482859 - Flags: review?(james) → review-
Attachment #8482859 - Attachment is obsolete: true
Attachment #8482882 - Flags: review?(james) → review+
This is NPOTB
Keywords: checkin-needed
Hi Chris,

the patch does not apply cleanly 

applying enumerate_actions.patch
patching file testing/mozbase/mozlog/mozlog/structured/structuredlog.py
Hunk #1 FAILED at 82
1 out of 1 hunks FAILED -- saving rejects to file testing/mozbase/mozlog/mozlog/structured/structuredlog.py.rej
patch failed, unable to continue (try -v)

could you take a look and maybe rebase? Thanks!
Flags: needinfo?(cmanchester)
Keywords: checkin-needed
Looks like this was landed, thanks!
Flags: needinfo?(cmanchester)
https://hg.mozilla.org/mozilla-central/rev/f7f7e8683a87
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in before you can comment on or make changes to this bug.