Skip to content
This repository has been archived by the owner on Nov 1, 2023. It is now read-only.

Fix notifications bugs #2534

Merged
merged 6 commits into from
Oct 20, 2022
Merged

Conversation

tevoinea
Copy link
Member

@tevoinea tevoinea commented Oct 19, 2022

Summary of the Pull Request

What is this about?

Implements 1. and 3. from #2530

PR Checklist

  • Applies to work item: #xxx
  • CLA signed. If not, go over here and sign the CLI.
  • Tests added/passed
  • Requires documentation to be updated
  • I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

Info on Pull Request

What does this include?

Validation Steps Performed

  1. Create a work item through onefuzz debug notification job {jobId}. This validates this block:
               // We never saw a work item like this before, it must be new
              else {
                var entry = await CreateNew();
                var adoEventType = "AdoNewItem";
                _logTracer.WithTags(notificationInfo).Event($"{adoEventType} {entry.Id:Tag:WorkItemId}");
            }

Verify the work item was created properly.
2. Re-run onefuzz debug notification job {jobId} to make sure the same work item can continue to be updated. This validates this block:

            } else if (numNonDuplicate == 1) {
                await UpdateExisting(await nonDuplicate.SingleAsync(), notificationInfo);
            }

Verify the work item was updated properly.
3. Manually create a duplicate of the work item in ADO then re-run onefuzz debug notification job {jobId}. This will validate having multiple work items that aren't marked as duplicate in this block:

            if (numNonDuplicate > 1) {
                var nonDuplicateWorkItemIds = nonDuplicate
                    .Select(wi => wi.Id)
                    .ToEnumerable();


                var matchingWorkItemIds = matchingWorkItems
                    .Select(wi => wi.Id)
                    .ToEnumerable();


                var extraTags = new List<(string, string)> {
                    ("NonDuplicateWorkItemIds", JsonSerializer.Serialize(nonDuplicateWorkItemIds)),
                    ("MatchingWorkItemIds", JsonSerializer.Serialize(matchingWorkItemIds))
                };
                extraTags.AddRange(notificationInfo);


                _logTracer.WithTags(extraTags).Info($"Found more than 1 matching, non-duplicate work item");
                await foreach (var workItem in nonDuplicate) {
                    await UpdateExisting(workItem, notificationInfo);
                }

Verify both work items were updated properly and log message created.
4. Mark the manually copied work item from Step 3 as "Duplicate" using the "Related Work" panel in the work item.
5. Re-run onefuzz debug notification job {jobId}, this will validate this block:

            var matchingWorkItems = ExistingWorkItems();


            var nonDuplicate = matchingWorkItems
                .Where(wi => !IsDuplicate(wi));


            var numNonDuplicate = await nonDuplicate.CountAsync();

Validate that only the work item created in step 1 was updated.
6. Mark the work item created in step 1 as "Resolved" with reason "Duplicate", this will validate this block:

              // We have matching work items but all are duplicates
              else if (await matchingWorkItems.AnyAsync()) {
                _logTracer.WithTags(notificationInfo).Info($"All matching work items were duplicates, re-opening the oldest one");
                var oldestWorkItem = await matchingWorkItems.OrderBy(wi => wi.Id).FirstAsync();
                await UpdateExisting(oldestWorkItem, notificationInfo);


                _ = await _client.AddCommentAsync(
                    new CommentCreate() {
                        Text = "This work item was re-opened because OneFuzz could only find related work items that are marked as duplicate."
                    },
                    _project,
                    (int)oldestWorkItem.Id!);
            }

Validate that the work item created in step 1 is re-opened, is updated, and has a new comment explaining why it was re-opened.

@codecov-commenter
Copy link

codecov-commenter commented Oct 19, 2022

Codecov Report

Merging #2534 (7af1b4c) into main (b3fd6d5) will increase coverage by 5.53%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #2534      +/-   ##
==========================================
+ Coverage   29.99%   35.53%   +5.53%     
==========================================
  Files         289      135     -154     
  Lines       35488    17118   -18370     
==========================================
- Hits        10645     6083    -4562     
+ Misses      24843    11035   -13808     
Impacted Files Coverage Δ
src/agent/coverage/src/test.rs 92.85% <0.00%> (-7.15%) ⬇️
src/agent/coverage/src/code.rs 8.29% <0.00%> (-6.12%) ⬇️
src/agent/onefuzz/src/machine_id.rs 51.21% <0.00%> (-5.63%) ⬇️
src/agent/onefuzz/src/sanitizer.rs 69.69% <0.00%> (-2.91%) ⬇️
src/agent/onefuzz-agent/src/worker/tests.rs 99.10% <0.00%> (-0.11%) ⬇️
src/agent/onefuzz/src/asan.rs 0.00% <0.00%> (ø)
src/agent/coverage/src/cache.rs 0.00% <0.00%> (ø)
src/agent/onefuzz-agent/src/setup.rs 0.00% <0.00%> (ø)
src/agent/coverage/src/block/linux.rs 0.00% <0.00%> (ø)
src/agent/onefuzz-agent/src/reboot.rs 0.00% <0.00%> (ø)
... and 166 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@tevoinea tevoinea marked this pull request as ready for review October 20, 2022 14:48
@tevoinea tevoinea merged commit e83a18b into microsoft:main Oct 20, 2022
@AdamL-Microsoft AdamL-Microsoft mentioned this pull request Oct 20, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Nov 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants