Page MenuHomePhabricator

Sample herald rule results
Closed, ResolvedPublic


problem: when I write a herald rule, I have no way to tell if it's correct - if it matches the bad code I want it to catch. As is, I have to wait until a diff actually triggers the rule to see it in action, *or* if it's wrong and matches less than I expect, I have to notice that things I getting by that shouldn't and investigate why; this would be extremely hard for me to find.

It would be really useful, when writing a new herald rule, to be able to see sample results of recent things it would have matched. In my case, I almost always use herald rules to match diffs, so it would be nice if it could run against the most recent ~1000 open diffs or so and show me what sort of diffs it matches. Or maybe instead, run against the current codebase and show me what it would match there, if it were sent in a diff today.

Event Timeline

chad added a subscriber: chad.Feb 23 2016, 1:27 AM

Have you tried the test console?

what test console?

Derp. I'll check it out.

epriestley closed this task as Resolved.Feb 23 2016, 12:25 PM
epriestley claimed this task.

This gets a brief mention in the documentation, but generally:

  • You can enter specific objects in the Test Console (like D123) and see what Herald would do if the current rule definitions were executed against them.
  • This isn't as sweeping as the feature you propose (automatically run against 1,000 recent objects) but, at least personally, I have yet to hit a case where it isn't sufficient to test a rule satisfactorily. In particular, it lets you pick a small number of test cases (say, a couple of positive cases and a couple of negative cases) to get results for them, so you can select test cases that are relevant to whatever the rule is doing. You also get a full transcript, so you can see exactly what field values Herald is evaluating.
  • See also T9719 for some technical/usability work.

I'm going to close this since I believe the existing Test Console satisfies this need, but feel free to file a new task if you have a use case where the test console is a poor fit for some reason, like the nature of the rule.

Ok so I just took it for a spin while editing herald rules. While it sort of solves my problems, it's very much a poor-man's solution - it's completely out of the way from the "create a herald rule" workflow - ideally, on the page where I'm creating or editing a herald rule, the test console should be right there so I can try out my new rule without necessarily having to save it; also it would filter to just trying that rule instead of trying all the rules, which would cut down on visual noise. This sort of change would make it much easier for the non-power user to create working herald rules.

Secondly, it doesn't totally solve my problem because I care as much about what sorts of diffs the rule matches as the ones that it doesn't match - and having to go searching for examples of diffs that it should and shouldn't match is potentially painful - it's not hard to verify it has the expected behavior on a diff or two, but what I really want to see is the "wait it matched THAT too?" surprises. We commit 100s of diffs a day so any false positives herald matches really spam my inbox, and having to wade through to figure out why herald added me to a diff, only to realize it's because my rule is overmatching, is not a very fun experience.

@epriestley should I open new tasks for these issues?

Is this rule a lint-like rule, in the vein of T7617? See also T10523 for discussion of lint-like rules.

I suppose the rules I am writing are fairly lint-like; part of this
reflects the sorry state of our lint infrastructure, though it is probably
a bit more flexible than herald. Having said that, sometimes I do want to
have rules that subscribe me to various diffs, before I figure out how to
automate my own advice; is there a way to add subscribers via lint rules?

Btw if Herald isn't supposed to be like lint, what is the intended use case?

is there a way to add subscribers via lint rules?

Not currently.

Btw if Herald isn't supposed to be like lint, what is the intended use case?

Here are some of the rules we have on this install:

H2, H11, and H77, and H95 send me email for every commit, revision, and push to any repository, and every wiki edit.

In the past, H124 automatically CC'd me on Ponder questions.

H104 CCs me on all tasks tagged with Phacility Support.

H65 adds "Blessed Reviewers" as a blocking reviewer for all revisions against open source repositories.

H68 prevents enormous commits to any hosted repository.

H78 triggers Harbormaster builds for revisions submitted by members of Community or Phacility High Command; H130 triggers builds for all open source repository commits.

H86 requires all contributors sign L28, the CLA.

Offhand, T8644 and T7804 describe a few more use cases.

All of these are complex behaviors which are particular to my personal workflow or this project's workflow and wouldn't make sense to encode as default behaviors in Phabricator, but none examine change content or operate in a lint-like way.

Rules like these are the intended use case for Herald, and the test console is fairly well-suited to testing rules like "CC me on any high priority task tagged with iOS" because you only need to test a handful of objects to be confident that the rule is working like you expect.

Thanks for all the examples - definitely makes sense that these behaviors are a bit of a different class than ones that actually examine file content.

I think my earlier suggestion, about having the test console in the herald create flow, is possibly a useful improvement - should I file a separate task?