dart-lang / dart_ci

Tools used by Dart's continuous integration (CI) testing that aren't needed by Dart SDK contributors. Mirrored from dart.googlesource.com/dart_ci. Do not land pull requests on Github.
BSD 3-Clause "New" or "Revised" License
18 stars 5 forks source link

Old approval surfaced after blame and re-approval. #122

Closed ghost closed 2 years ago

ghost commented 2 years ago

During gardening this morning I was approving a failure (see pre-approval screenshot) by:

  1. 'Narrow Blamelist' and selecting the commit in questions.
  2. 'Approve/Comment' to approve and leave a comment with the relevant issue.

But when I had completed these steps I was met with what can be seen in the post-approval screenshot. Result Feed reports that the failure had already been approved 11 hours earlier by someone else.

The approval was not at all visible when I initially loaded the Result Feed and did not show until after I either blamed or re-approved (though I didn't notice until after doing both).

I suspect part of the confusion is that the failures I was approving ran between 6 and 7 this morning (8 hours after the other approval), so probably there is some grouping that's off here?

Pre-approval

image

Post-approval

image

whesse commented 2 years ago

This is "Working as intended". Not all of the failures are showing, because there is a filter to only show failures that include the dartk-win-release-ia32 configurations. Failures of the co19_2 tests with similar names were seen and approved earlier by bkonyi.

The co19_2 versions of those tests were run on dartk-win-release-x64 and failed, and those failures showed up earlier (and were either pinned or had the single-commit blamelist), and were approved by bkonyi. The message is associated with a particular blamelist interval (in this case, a single commit), and only show up when that interval is shown.

The co19 versions of the tests are considered separate, and not grouped in a single failure with the co19_2 versions, so they needed to be pinned and approved separately.

So even if you had seen the existing failures, by removing the filter on the configuration, you still needed to approve (and preferably pin) the failures on the co19 tests.