Skip to content

Commit

Permalink
Allow showing, and hide bringup builds by default. (#4141)
Browse files Browse the repository at this point in the history 
Closes flutter/flutter#160697.

I did test that it is possible to show bringup builds (and the query parameters works, for example).

---

There are a _lot_ of ways this code[base] could be better, but intentionally tried to limit the amount of changes in this PR.

(I would be happy to try and improve things in Q1 if we decide this is valuable to do)

/cc @jtmcdole
  • Loading branch information
@matanlurey
matanlurey authored Dec 27, 2024
1 parent 6c83f7f commit 378695f
Show file tree
Hide file tree
Showing 6 changed files with 114 additions and 18 deletions.
8 changes: 6 additions & 2 deletions dashboard/lib/logic/qualified_task.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,21 @@ const String _dartInternalUrl = 'https://ci.chromium.org/p/dart-internal';

@immutable
class QualifiedTask {
const QualifiedTask({this.stage, this.task, this.pool});
const QualifiedTask({this.stage, this.task, this.pool, this.isBringup = false});

QualifiedTask.fromTask(Task task)
: stage = task.stageName,
task = task.builderName,
pool = task.isFlaky ? 'luci.flutter.staging' : 'luci.flutter.prod';
pool = task.isFlaky ? 'luci.flutter.staging' : 'luci.flutter.prod',
isBringup = task.isFlaky;

final String? pool;
final String? stage;
final String? task;

/// Whether this task originated as a `bringup: true` task.
final bool isBringup;

/// Get the URL for the configuration of this task.
///
/// Luci tasks are stored on Luci.
Expand Down
19 changes: 17 additions & 2 deletions dashboard/lib/logic/task_grid_filter.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ class TaskGridFilter extends FilterPropertySource {
final BoolFilterProperty _iosProperty = BoolFilterProperty(fieldName: 'showiOS', label: 'iOS');
final BoolFilterProperty _linuxPorperty = BoolFilterProperty(fieldName: 'showLinux', label: 'Linux');
final BoolFilterProperty _androidProperty = BoolFilterProperty(fieldName: 'showAndroid', label: 'Android');
final BoolFilterProperty _bringUpProperty =
BoolFilterProperty(fieldName: 'showBringup', label: 'Bring Up', value: false);
final BoolFilterProperty _stagingProperty =
BoolFilterProperty(fieldName: 'showStaging', label: 'Staging', value: false);

Expand All @@ -72,7 +74,8 @@ class TaskGridFilter extends FilterPropertySource {
..[_iosProperty.fieldName] = _iosProperty
..[_linuxPorperty.fieldName] = _linuxPorperty
..[_androidProperty.fieldName] = _androidProperty
..[_stagingProperty.fieldName] = _stagingProperty) as LinkedHashMap<String, ValueFilterProperty<dynamic>>?)!;
..[_stagingProperty.fieldName] = _stagingProperty
..[_bringUpProperty.fieldName] = _bringUpProperty) as LinkedHashMap<String, ValueFilterProperty<dynamic>>?)!;

/// The [taskFilter] property is a regular expression that must match the name of the
/// task in the grid. This property will filter out columns on the build dashboard.
Expand Down Expand Up @@ -147,6 +150,13 @@ class TaskGridFilter extends FilterPropertySource {

set showStaging(bool? value) => _stagingProperty.value = value;

/// Whether to display tasks that are marked `bringup` (do not block the tree).
///
/// This property will filter out columns on the build dashboard.
bool? get showBringup => _bringUpProperty.value;

set showBringup(bool? value) => _bringUpProperty.value = value;

/// Check the values in the [CommitStatus] for compatibility with the properties of this
/// filter and return [true] iff the commit row should be displayed.
bool matchesCommit(CommitStatus commitStatus) {
Expand All @@ -173,6 +183,10 @@ class TaskGridFilter extends FilterPropertySource {
return false;
}

if ((!_allProperties['showBringup']?.value) && qualifiedTask.isBringup) {
return false;
}

final bool showAndroid = _allProperties['showAndroid']?.value ?? false;
final LinkedHashMap<String, bool> orderedOSFilter = LinkedHashMap<String, bool>.of({
'ios': _allProperties['showiOS']?.value ?? false,
Expand Down Expand Up @@ -244,11 +258,12 @@ class TaskGridFilter extends FilterPropertySource {
label: 'Stages',
members: <BoolFilterProperty>[
_androidProperty,
_stagingProperty,
_iosProperty,
_linuxPorperty,
_macProperty,
_windowsPorperty,
_stagingProperty,
_bringUpProperty,
],
),
];
Expand Down
18 changes: 16 additions & 2 deletions dashboard/test/build_dashboard_page_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -498,7 +498,14 @@ void main() {
value: buildState,
child: ValueProvider<GoogleSignInService>(
value: buildState.authService,
child: const BuildDashboardPage(),
child: const BuildDashboardPage(
// TODO(matanlurey): Either find a Linux machine, or remove.
// To avoid making a golden-file breaking change as part of
// https://github.com/flutter/cocoon/pull/4141
//
// See https://github.com/flutter/flutter/issues/160931.
queryParameters: {'showBringup': 'true'},
),
),
),
),
Expand Down Expand Up @@ -531,7 +538,14 @@ void main() {
value: buildState,
child: ValueProvider<GoogleSignInService>(
value: buildState.authService,
child: const BuildDashboardPage(),
child: const BuildDashboardPage(
// TODO(matanlurey): Either find a Linux machine, or remove.
// To avoid making a golden-file breaking change as part of
// https://github.com/flutter/cocoon/pull/4141
//
// See https://github.com/flutter/flutter/issues/160931.
queryParameters: {'showBringup': 'true'},
),
),
),
),
Expand Down
30 changes: 30 additions & 0 deletions dashboard/test/logic/task_grid_filter_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ void main() {
expect(filter.hashFilter, null);
expect(filter.showiOS, true);
expect(filter.showStaging, false);
expect(filter.showBringup, false);

expect(filter.matchesTask(QualifiedTask.fromTask(Task())), true);
expect(filter.matchesTask(QualifiedTask.fromTask(Task()..builderName = 'foo')), true);
Expand Down Expand Up @@ -62,6 +63,7 @@ void main() {
expect(TaskGridFilter.fromMap(<String, String>{'hashFilter': 'foo'}), TaskGridFilter()..hashFilter = RegExp('foo'));
expect(TaskGridFilter.fromMap(<String, String>{'showMac': 'false'}), TaskGridFilter()..showMac = false);
expect(TaskGridFilter.fromMap(<String, String>{'showStaging': 'false'}), TaskGridFilter()..showStaging = false);
expect(TaskGridFilter.fromMap(<String, String>{'showBringup': 'false'}), TaskGridFilter()..showBringup = false);
});

test('cross check on inequality', () {
Expand All @@ -88,6 +90,34 @@ void main() {
}
});

test('bringup filter show all tasks', () {
final List<TaskGridFilter> filters = <TaskGridFilter>[
TaskGridFilter()..showBringup = true,
];
for (final TaskGridFilter filter in filters) {
expect(filter.matchesTask(QualifiedTask.fromTask(Task()..builderName = 'Good task')), true);
expect(
filter.matchesTask(QualifiedTask.fromTask(Task()
..builderName = 'Bringup task'
..isFlaky = true)),
true);
}
});

test('bringup filter hide bringup tasks', () {
final List<TaskGridFilter> filters = <TaskGridFilter>[
TaskGridFilter()..showBringup = false,
];
for (final TaskGridFilter filter in filters) {
expect(filter.matchesTask(QualifiedTask.fromTask(Task()..builderName = 'Good task')), true);
expect(
filter.matchesTask(QualifiedTask.fromTask(Task()
..builderName = 'Bringup task'
..isFlaky = true)),
false);
}
});

test('staging filter show all tasks', () {
final List<TaskGridFilter> filters = <TaskGridFilter>[
TaskGridFilter()..showStaging = true,
Expand Down
51 changes: 39 additions & 12 deletions dashboard/test/widgets/task_grid_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,15 @@ void main() {
theme: ThemeData(useMaterial3: false),
home: ValueProvider<BuildState>(
value: buildState,
child: const Material(
child: TaskGridContainer(),
child: Material(
child: TaskGridContainer(
// TODO(matanlurey): Either find a Linux machine, or remove.
// To avoid making a golden-file breaking change as part of
// https://github.com/flutter/cocoon/pull/4141
//
// See https://github.com/flutter/flutter/issues/160931.
filter: TaskGridFilter()..showBringup = true,
),
),
),
),
Expand Down Expand Up @@ -126,8 +133,15 @@ void main() {
theme: ThemeData(useMaterial3: false),
home: ValueProvider<BuildState>(
value: buildState,
child: const Material(
child: TaskGridContainer(),
child: Material(
child: TaskGridContainer(
// TODO(matanlurey): Either find a Linux machine, or remove.
// To avoid making a golden-file breaking change as part of
// https://github.com/flutter/cocoon/pull/4141
//
// See https://github.com/flutter/flutter/issues/160931.
filter: TaskGridFilter()..showBringup = true,
),
),
),
),
Expand Down Expand Up @@ -207,8 +221,15 @@ void main() {
theme: ThemeData.dark(useMaterial3: false),
home: ValueProvider<BuildState>(
value: buildState,
child: const Material(
child: TaskGridContainer(),
child: Material(
child: TaskGridContainer(
// TODO(matanlurey): Either find a Linux machine, or remove.
// To avoid making a golden-file breaking change as part of
// https://github.com/flutter/cocoon/pull/4141
//
// See https://github.com/flutter/flutter/issues/160931.
filter: TaskGridFilter()..showBringup = true,
),
),
),
),
Expand Down Expand Up @@ -286,17 +307,17 @@ void main() {

testWidgets('Task name filter affects grid', (WidgetTester tester) async {
// Default filters
await testGrid(tester, null, 27, 101);
await testGrid(tester, TaskGridFilter(), 27, 101);
await testGrid(tester, TaskGridFilter.fromMap(null), 27, 101);
await testGrid(tester, null, 27, 100);
await testGrid(tester, TaskGridFilter(), 27, 100);
await testGrid(tester, TaskGridFilter.fromMap(null), 27, 100);

// QualifiedTask (column) filters
await testGrid(tester, TaskGridFilter()..taskFilter = RegExp('Linux_android 2'), 27, 12);

// CommitStatus (row) filters
await testGrid(tester, TaskGridFilter()..authorFilter = RegExp('bob'), 8, 101);
await testGrid(tester, TaskGridFilter()..messageFilter = RegExp('developer'), 18, 101);
await testGrid(tester, TaskGridFilter()..hashFilter = RegExp('2d22b5e85f986f3fa2cf1bfaf085905c2182c270'), 4, 101);
await testGrid(tester, TaskGridFilter()..authorFilter = RegExp('bob'), 8, 100);
await testGrid(tester, TaskGridFilter()..messageFilter = RegExp('developer'), 18, 100);
await testGrid(tester, TaskGridFilter()..hashFilter = RegExp('2d22b5e85f986f3fa2cf1bfaf085905c2182c270'), 4, 100);
});

testWidgets('Skipped tasks do not break the grid', (WidgetTester tester) async {
Expand Down Expand Up @@ -823,6 +844,12 @@ void main() {
child: TaskGrid(
buildState: FakeBuildState(),
commitStatuses: statuses,
// TODO(matanlurey): Either find a Linux machine, or remove.
// To avoid making a golden-file breaking change as part of
// https://github.com/flutter/cocoon/pull/4141
//
// See https://github.com/flutter/flutter/issues/160931.
filter: TaskGridFilter()..showBringup = true,
),
),
),
Expand Down
6 changes: 6 additions & 0 deletions dashboard/test/widgets/task_overlay_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import 'package:fixnum/fixnum.dart';
import 'package:flutter/material.dart';
import 'package:flutter_dashboard/logic/qualified_task.dart';
import 'package:flutter_dashboard/logic/task_grid_filter.dart';
import 'package:flutter_dashboard/model/commit.pb.dart';
import 'package:flutter_dashboard/model/commit_status.pb.dart';
import 'package:flutter_dashboard/model/task.pb.dart';
Expand All @@ -28,16 +29,19 @@ class TestGrid extends StatelessWidget {
const TestGrid({
required this.buildState,
required this.task,
this.filter,
super.key,
});

final BuildState buildState;
final Task task;
final TaskGridFilter? filter;

@override
Widget build(BuildContext context) {
return Material(
child: TaskGrid(
filter: filter,
buildState: buildState,
commitStatuses: <CommitStatus>[
CommitStatus()
Expand Down Expand Up @@ -159,6 +163,8 @@ void main() {
body: TestGrid(
buildState: buildState,
task: flakyTask,
// Otherwise the task is not rendered at all.
filter: TaskGridFilter()..showBringup = true,
),
),
),
Expand Down

0 comments on commit 378695f

Please sign in to comment.