Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 22 additions & 17 deletions app_dart/lib/src/request_handlers/update_suppressed_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import '../service/firestore.dart';
/// - `repository`: The repository slug (e.g. `flutter/flutter`).
/// - `action`: `SUPPRESS` or `UNSUPPRESS`.
/// - `issueLink`: URL to the GitHub issue tracking the failure.
/// required if SUPPRESS
/// - `note`: Optional note describing the action.
final class UpdateSuppressedTest extends ApiRequestHandler {
const UpdateSuppressedTest({
Expand Down Expand Up @@ -75,13 +76,6 @@ final class UpdateSuppressedTest extends ApiRequestHandler {
);
}

final issueLink = body[_paramIssueLink];
if (issueLink is! String) {
throw const BadRequestException(
'Parameter "$_paramIssueLink" must be a string',
);
}

final RepositorySlug repository;
{
final repositoryString = body[_paramRepository];
Expand All @@ -100,16 +94,26 @@ final class UpdateSuppressedTest extends ApiRequestHandler {
);
}

// Validate issue link
final issueNumber = _parseIssueNumber(issueLink);
if (issueNumber == null) {
throw const BadRequestException(
'Invalid issue link format, expected https://github.com/flutter/flutter/issues/1234',
);
}
String? issueLink;

// Validate issue exists and is open if suppressing
if (action == _actionSuppress) {
final link = body[_paramIssueLink];
if (link is! String) {
throw const BadRequestException(
'Parameter "$_paramIssueLink" must be a string',
);
}
issueLink = link;

// Validate issue link
final issueNumber = _parseIssueNumber(issueLink);
if (issueNumber == null) {
throw const BadRequestException(
'Invalid issue link format, expected https://github.com/flutter/flutter/issues/1234',
);
}

final githubService = await config.createGithubService(repository);
final issue = await githubService.getIssue(
repository,
Expand Down Expand Up @@ -160,8 +164,8 @@ final class UpdateSuppressedTest extends ApiRequestHandler {
required String testName,
required RepositorySlug repository,
required String action,
required String issueLink,
required String note,
String? issueLink,
}) async {
// Query for existing suppression - we assume there is at most one document
// per (repo, name) based on business logic, though the DB constraint might
Expand Down Expand Up @@ -197,7 +201,8 @@ final class UpdateSuppressedTest extends ApiRequestHandler {
final updatedSuppression = SuppressedTest(
name: testName,
repository: repository.fullName,
issueLink: issueLink,
// issue: today we don't have UI affordance for updating the link.
issueLink: existingSuppression.issueLink,
isSuppressed: isSuppressed,
createTimestamp: existingSuppression.createTimestamp,
updates: [...existingSuppression.updates, updateEntry],
Expand All @@ -224,7 +229,7 @@ final class UpdateSuppressedTest extends ApiRequestHandler {
final newSuppression = SuppressedTest(
name: testName,
repository: repository.fullName,
issueLink: issueLink,
issueLink: issueLink ?? 'BUG',
isSuppressed: true,
createTimestamp: now,
updates: [updateEntry],
Expand Down
4 changes: 1 addition & 3 deletions app_dart/lib/src/request_handling/static_file_handler.dart
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,7 @@ final class StaticFileHandler extends RequestHandler {
'.smcbin': 'application/octet-stream',
};

const mimeFileMap = {
'apple-app-site-association': 'application/json',
};
const mimeFileMap = {'apple-app-site-association': 'application/json'};

final resultPath = filePath == '/' ? '/index.html' : filePath;

Expand Down
1 change: 0 additions & 1 deletion app_dart/test/model/firestore/suppressed_test_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import 'package:cocoon_service/src/model/firestore/suppressed_test.dart';
import 'package:googleapis/firestore/v1.dart';
import 'package:test/test.dart';


void main() {
useTestLoggerPerTest();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,6 @@ void main() {
ciYaml: examplePresubmitRescheduleFusionConfig,
);



handler = PresubmitLuciSubscription(
cache: CacheService(inMemory: true),
config: config,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ void main() {
'testName': 'my_test',
'repository': 'flutter/flutter',
'action': 'UNSUPPRESS',
'issueLink': 'https://github.com/flutter/flutter/issues/123',
'issueLink': 'https://github.com/flutter/flutter/issues/345',
'note': 'Closing issue',
});

Expand Down
15 changes: 3 additions & 12 deletions app_dart/test/service/scheduler_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3643,10 +3643,7 @@ targets:

final check = PresubmitCompletedCheck.fromBuild(build, userData);

expect(
await scheduler.processCheckRunCompleted(check),
isTrue,
);
expect(await scheduler.processCheckRunCompleted(check), isTrue);

// Should schedule tests for the next stage (fusionTests)
expect(fakeLuciBuildService.scheduledTryBuilds, isNotEmpty);
Expand Down Expand Up @@ -3722,10 +3719,7 @@ targets:

final check = PresubmitCompletedCheck.fromBuild(build, userData);

expect(
await scheduler.processCheckRunCompleted(check),
isTrue,
);
expect(await scheduler.processCheckRunCompleted(check), isTrue);

verify(
mockGithubChecksUtil.updateCheckRun(
Expand Down Expand Up @@ -3810,10 +3804,7 @@ targets:

final check = PresubmitCompletedCheck.fromBuild(build, userData);

expect(
await scheduler.processCheckRunCompleted(check),
isTrue,
);
expect(await scheduler.processCheckRunCompleted(check), isTrue);

verify(
mockGithubChecksUtil.updateCheckRun(
Expand Down
8 changes: 3 additions & 5 deletions dashboard/lib/service/appengine_cocoon.dart
Original file line number Diff line number Diff line change
Expand Up @@ -264,20 +264,18 @@ class AppEngineCocoonService implements CocoonService {
required String repo,
required String testName,
required bool suppress,
required String issueLink,
String? issueLink,
String? note,
}) async {
final updateTestSuppressionUrl = apiEndpoint(
'/api/update-suppressed-test',
);
final updateTestSuppressionUrl = apiEndpoint('/api/update-suppressed-test');
final response = await _client.post(
updateTestSuppressionUrl,
headers: {'X-Flutter-IdToken': idToken},
body: jsonEncode({
'repository': repo,
'testName': testName,
'action': suppress ? 'SUPPRESS' : 'UNSUPPRESS',
'issueLink': issueLink,
'issueLink': ?issueLink,
'note': ?note,
}),
);
Expand Down
2 changes: 1 addition & 1 deletion dashboard/lib/service/cocoon.dart
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ abstract class CocoonService {
required String repo,
required String testName,
required bool suppress,
required String issueLink,
String? issueLink,
String? note,
});
}
Expand Down
5 changes: 3 additions & 2 deletions dashboard/lib/service/dev_cocoon.dart
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ class DevelopmentCocoonService implements CocoonService {
required String repo,
required String testName,
required bool suppress,
required String issueLink,
String? issueLink,
String? note,
}) async {
final list = _suppressedTests.putIfAbsent(repo, () => []);
Expand All @@ -156,7 +156,8 @@ class DevelopmentCocoonService implements CocoonService {
SuppressedTest(
name: testName,
repository: repo,
issueLink: issueLink,
issueLink:
issueLink ?? 'https://github.com/flutter/flutter/issues/123',
createTimestamp: DateTime.now().millisecondsSinceEpoch,
updates: [
SuppressionUpdate(
Expand Down
2 changes: 1 addition & 1 deletion dashboard/lib/state/build.dart
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ class BuildState extends ChangeNotifier {
Future<bool> updateTestSuppression({
required String testName,
required bool suppress,
required String issueLink,
String? issueLink,
String? note,
}) async {
if (!authService.isAuthenticated) {
Expand Down
57 changes: 22 additions & 35 deletions dashboard/lib/widgets/task_grid.dart
Original file line number Diff line number Diff line change
Expand Up @@ -220,8 +220,8 @@ class _TaskGridState extends State<TaskGrid> {
filter ??= TaskGridFilter();

// Pre-compute suppressed task names for faster lookup
final suppressedTaskNames = {
for (final s in taskGrid.buildState.suppressedTests) s.name,
final suppressedTasks = {
for (final s in taskGrid.buildState.suppressedTests) s.name: s,
};

// 1: PREPARE ROWS
Expand Down Expand Up @@ -274,7 +274,7 @@ class _TaskGridState extends State<TaskGrid> {
// won't know how to sort the task later.
scores.putIfAbsent(qualifiedTask, () => 0.0);
}
final isSuppressed = suppressedTaskNames.contains(task.builderName);
final isSuppressed = suppressedTasks.containsKey(task.builderName);
rows[commitCount - 1].cells[qualifiedTask] = LatticeCell(
painter: _painterFor(task, isSuppressed),
builder: _builderFor(task, isSuppressed),
Expand All @@ -286,8 +286,8 @@ class _TaskGridState extends State<TaskGrid> {
final tasks = scores.keys.toList()
..sort((QualifiedTask a, QualifiedTask b) {
// Suppressed tests go first (far left)
final aSuppressed = suppressedTaskNames.contains(a.task);
final bSuppressed = suppressedTaskNames.contains(b.task);
final aSuppressed = suppressedTasks.containsKey(a.task);
final bSuppressed = suppressedTasks.containsKey(b.task);
if (aSuppressed && !bSuppressed) {
return -1;
}
Expand All @@ -307,43 +307,29 @@ class _TaskGridState extends State<TaskGrid> {
<LatticeCell>[
const LatticeCell(),
...tasks.map<LatticeCell>((QualifiedTask task) {
final isSuppressed = suppressedTaskNames.contains(task.task);
return LatticeCell(
builder: (BuildContext context) {
final suppressedTest = suppressedTasks[task.task];
final icon = TaskIcon(
qualifiedTask: task,
onTap: () => _showTestDetails(context, task),
onTap: (BuildContext context) =>
_showTestDetails(context, task),
);
if (isSuppressed) {
final suppressedTest = taskGrid.buildState.suppressedTests
.firstWhere((s) => s.name == task.task);
if (suppressedTest != null) {
// Format audit log
final tip = [
'Issue: ${suppressedTest.issueLink}',
for (var update
in [...suppressedTest.updates]..sort(
(a, b) =>
b.updateTimestamp.compareTo(a.updateTimestamp),
))
'[${DateTime.fromMillisecondsSinceEpoch(update.updateTimestamp)}] ${update.action} by ${update.user} ${update.note != null ? '- ${update.note}' : ''}',
].join('\n');

return Tooltip(
message: tip,
child: Stack(
alignment: Alignment.bottomRight,
children: [
Opacity(opacity: 0.3, child: icon),
const IgnorePointer(
child: Icon(
Icons.snooze,
fontWeight: FontWeight.bold,
size: 20,
color: Colors.red,
),
return Stack(
alignment: Alignment.bottomRight,
children: [
Opacity(opacity: 0.3, child: icon),
const IgnorePointer(
child: Icon(
Icons.snooze,
fontWeight: FontWeight.bold,
size: 20,
color: Colors.red,
),
],
),
),
],
);
}
return icon;
Expand Down Expand Up @@ -510,6 +496,7 @@ class _TaskGridState extends State<TaskGrid> {
final renderBox = context.findRenderObject() as RenderBox;
final position = renderBox.localToGlobal(Offset.zero);

print('_showTestDetails: $position');
_taskOverlay = OverlayEntry(
builder: (BuildContext context) => Stack(
children: <Widget>[
Expand Down
4 changes: 2 additions & 2 deletions dashboard/lib/widgets/task_icon.dart
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class TaskIcon extends StatelessWidget {
final QualifiedTask qualifiedTask;

/// Callback when the icon is tapped.
final VoidCallback? onTap;
final void Function(BuildContext)? onTap;

/// A lookup table for matching [stageName] to [Image].
///
Expand Down Expand Up @@ -91,7 +91,7 @@ class TaskIcon extends StatelessWidget {
child: InkWell(
onTap: () async {
if (onTap != null) {
onTap!();
onTap!(context);
} else {
await launchUrl(qualifiedTask.sourceConfigurationUrl);
}
Expand Down
Loading
Loading