Page MenuHomePhabricator

Create a front end for making unit tests for filters
Open, LowestPublic

Description

While editing the filters, I always have a fear of breaking current functionality by mistake. Many of the filters use regex and/or other complex logics. Having some easy way of testing filters would improve the situation a lot.

Provide a way to save some edits or their enviroment variables into unittests. They should be save into two groups: edits that should trigger the abuse filter and edits that should not. Edited filters should pass those tests before being saved successfully.

Event Timeline

Dalba created this task.Nov 6 2018, 10:50 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptNov 6 2018, 10:50 AM

This would require adding lots of new logic. I wonder if Special:AbuseFilter/test would be enough, especially when T36180 will be resolved.

Dalba closed this task as Invalid.Nov 6 2018, 11:32 AM

I don't think it's enough, but I'm OK with withdrawing this task if it's too difficult to implement.

I don't know how difficult would it be, but for sure it would require several changes: at least a new page in the UI (and we'll add several new pages for performance monitoring) and some sort of DB change, being (at least) a new column in abuse_filter_log if testing past hits is OK, or a new table to make it possible to add edits for which there are no entries in the AbuseLog. It could be done, but I'd rather wait for T36180 and see if this feature will still be needed.

Huji added a subscriber: Huji.EditedNov 6 2018, 10:17 PM

There are two issues at hand:

  • We need to have the ability to re-run a filter against some set of old edits, with minimal effort
  • We need those past edits to trigger the filter just like how they did in the past.

The first one is a usability issue. Right now, one can take an old edit that they knew had triggered a filter, and try /test to run the new filter against it, but if you had a set of 100 test cases, you would end up having to interact with /test at least 100 times (if not more, because finding old edits is not super straightforward).

But the second one is more complex. A filter that was triggered by edit X three months ago may not be triggered by it now if the filter definition also includes things like user age, edit count, article age, etc.

For the latter reason, using actual edits as unit tests is not ideal. Instead, we should perhaps create a unit test environment for AbuseFilter front-end, and that would take a lot of work.

I would probably rephrase this task as exactly that ("Create a front end for making unit tests for filters") and make it a child of a parent task called "Major overhaul of abuse filter" which should also have T53294 and T203587 as its children.

Dalba added a comment.Nov 7 2018, 2:45 AM

For the latter reason, using actual edits as unit tests is not ideal. Instead, we should perhaps create a unit test environment for AbuseFilter front-end, and that would take a lot of work.

Thinking about it, I totally agree. That's a very accurate assessment of the problem. Thanks.

Daimona reopened this task as Open.Nov 7 2018, 10:44 AM
Daimona triaged this task as Lowest priority.

@Huji thanks, excellent analysis! The idea is great, however I agree that it will be quite complex, and need lots of work. I'm reopening this task, but I believe that for the moment we should focus on other major changes, which aim to solve software bugs, instead of adding new features.

Daimona moved this task from Backlog to Future on the User-Daimona board.
Huji added a comment.Nov 8 2018, 2:04 AM

@Daimona can I ask you to kindly rename the task as I suggest above, and create the parent task as well?

Daimona renamed this task from Provide a way to save some edits or their enviroment variables as unittests for individual filters to Create a front end for making unit tests for filters.Nov 8 2018, 8:36 AM

@Huji indeed, done :-) I forgot to do it... Also, I have added T193064 instead of T53294 as subtask, as the latter is part of a broader plan.

Reminder: A note by Krinkle at T232498#5485438 suggests:

Perhaps we can have a set of dummy actions we apply the filter to in order to shake out the most obvious errors. Eg. 1 dummy edit, 1 dummy account creation, 1 dummy page move. And verify that it is able to filter all three without errors (doesn't matter whether it matches). And if it produces an error in any of them, treat similar to a syntax error (refuse to safe or auto-disable).

That could be implemented as part of this task.