Bug 183964 – Add a field to analysis test group indicating whether this test group needs automatical bisection.

• Status: NEW
• Component: New Bugs

Add a field to analysis test group indicating whether this test group needs automatical bisection.

Created attachment 336443 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 336443 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=336443&action=review

> Websites/perf.webkit.org/init-database.sql:281
> +    testgroup_auto_bisect boolean NOT NULL DEFAULT FALSE,

I don't think "auto_bisect" is a good name since perf dashboard itself won't automatically bisect this range.
In general, I find that matching database schema naming with a UI facing feature is problematic.
That can lead to a very narrowly scoped database design.

I'm not even certain boolean makes sense here. I could imagine there could be multiple things
change-detection scripts may want to do with this test group like confirming, bisecting, re-testing, etc...
It seems also plausible that change detection scripts need to store some extra states / info per test group.

I really don't think we should be making this database schema change until we have UI & change detection scripts change ready.

Comment on attachment 336443 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=336443&action=review

>> Websites/perf.webkit.org/init-database.sql:281
>> +    testgroup_auto_bisect boolean NOT NULL DEFAULT FALSE,
> 
> I don't think "auto_bisect" is a good name since perf dashboard itself won't automatically bisect this range.
> In general, I find that matching database schema naming with a UI facing feature is problematic.
> That can lead to a very narrowly scoped database design.
> 
> I'm not even certain boolean makes sense here. I could imagine there could be multiple things
> change-detection scripts may want to do with this test group like confirming, bisecting, re-testing, etc...
> It seems also plausible that change detection scripts need to store some extra states / info per test group.
> 
> I really don't think we should be making this database schema change until we have UI & change detection scripts change ready.

Sounds reasonable. The bisect button change is https://bugs.webkit.org/show_bug.cgi?id=183888

[Ryosuke Niwa]

Comment on attachment 336443 [details]
Patch

Again, we shouldn't be making these database schema only changes.
The frontend & backend code logics need to come in along with the database change.

I didn't work on this recently. As we discussed, this change need to be more complicated and I will include all the changes associate with schema change next time I update the patch.