Closed Bug 1059609 Opened 7 years ago Closed 7 years ago
Mozlog should expose the set of supported log actions
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
This is NPOTB
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!
Looks like this was landed, thanks!
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.