Handle CodeFlow conflicts gracefully #4387

dkurepa · 2025-01-27T17:46:09Z

#4182

Makes it so we post a comment detailing all conflicting files on an unsuccessful PR update
Further updates are blocked, and not even tried unless the PR head branch commit changes, in that case we retry to see if the conflict got resolved (if this fails again, we'll post another comment with the list of files)

…ringConflicts

Broken by dotnet#4376

…ringConflicts

premun

I reviewed all but the scenario tests, will have a look at them later

src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/Exceptions.cs

...roductConstructionService/ProductConstructionService.DependencyFlow/InProgressPullRequest.cs

src/ProductConstructionService/ProductConstructionService.DependencyFlow/PullRequestUpdater.cs

test/ProductConstructionService.ScenarioTests/ScenarioTestBase.cs

test/ProductConstructionService.ScenarioTests/ScenarioTests/ScenarioTests_CodeFlowConflicts.cs

…ndencyFlow/PullRequestUpdater.cs Co-authored-by: Přemek Vysoký <premek.vysoky@microsoft.com>

premun · 2025-01-28T14:15:50Z

@dkurepa I think we should add a DependencyFlow.Tests for some of this logic too. These set up the state (fake redis) and then execute a pending check or update and check the state again. They verify which updates have been set and what is in the cache after. They are quite valuable because from them you can tell/understand what is the state like in every step

…ts' into dkurepa/PostCommentDuringConflicts

…he correct files are there

src/ProductConstructionService/ProductConstructionService.DependencyFlow/PullRequestUpdater.cs

…ndencyFlow/PullRequestUpdater.cs

src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/Exceptions.cs

src/ProductConstructionService/ProductConstructionService.DependencyFlow/PullRequestUpdater.cs

test/ProductConstructionService.DependencyFlow.Tests/PullRequestUpdaterTests.cs

premun · 2025-01-29T15:21:55Z

test/ProductConstructionService.ScenarioTests/CodeFlowScenarioTestBase.cs

+            {
+                return pullRequest;
+            }
+            await Task.Delay(5000);


This waits for 3-4 minutes while the service checks every 5 minutes. So this might not be enough if we need an extra check? Or we trigger everything first and then wait until the service does it in the first go?

This check doesn't need to wait for the reminder to kick in. The comment should appear pretty fast after the subscription is triggered

test/ProductConstructionService.ScenarioTests/CodeFlowScenarioTestBase.cs

test/ProductConstructionService.ScenarioTests/ScenarioTestBase.cs

src/ProductConstructionService/ProductConstructionService.DependencyFlow/PullRequestUpdater.cs

…stUpdaterTests.cs Co-authored-by: Přemek Vysoký <premek.vysoky@microsoft.com>

…TestBase.cs Co-authored-by: Přemek Vysoký <premek.vysoky@microsoft.com>

Co-authored-by: Přemek Vysoký <premek.vysoky@microsoft.com>

…ndencyFlow/PullRequestUpdater.cs Co-authored-by: Přemek Vysoký <premek.vysoky@microsoft.com>

src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/Exceptions.cs

Co-authored-by: Přemek Vysoký <premek.vysoky@microsoft.com>

dkurepa and others added 13 commits January 22, 2025 16:00

writing the new conflict test

001dc4c

Merge remote-tracking branch 'source/main' into dkurepa/PostCommentDu…

946b68d

…ringConflicts

Merge remote-tracking branch 'source/main' into dkurepa/PostCommentDu…

2f4fec7

…ringConflicts

Conflict stuff

5ba53fe

Merge remote-tracking branch 'source/main' into dkurepa/PostCommentDu…

ffcfe7f

…ringConflicts

More things

89beccb

Fix VMR cloning in PCS

8673a3b

Broken by dotnet#4376

Add scenario test for other scenario

6ad1da0

what is wrong here..

b4ba888

fix unit tests

8eb1f89

some improvements

ab7f82a

Merge remote-tracking branch 'source/main' into dkurepa/PostCommentDu…

3617791

…ringConflicts

more things

80c14e1

dkurepa marked this pull request as ready for review January 28, 2025 11:08

dkurepa requested review from premun and adamzip January 28, 2025 11:08

dkurepa changed the title ~~Dkurepa/post comment during conflicts~~ Handle CodeFlow conflicts gracefully Jan 28, 2025

tests are passing locally??

bf32b13

premun reviewed Jan 28, 2025

View reviewed changes

Update src/ProductConstructionService/ProductConstructionService.Depe…

07b24e9

…ndencyFlow/PullRequestUpdater.cs Co-authored-by: Přemek Vysoký <premek.vysoky@microsoft.com>

dkurepa added 5 commits January 28, 2025 15:17

Address part of the feedback

cd68e65

Merge remote-tracking branch 'origin/dkurepa/PostCommentDuringConflic…

9db5613

…ts' into dkurepa/PostCommentDuringConflicts

More stuff resolved

a40dda8

Add DependencyFlow tests for conflict handling

68b976a

Don't check whole conflict comment in scenario tests, just check if t…

30d1b32

…he correct files are there

dkurepa requested a review from premun January 29, 2025 15:01

dkurepa commented Jan 29, 2025

View reviewed changes

src/ProductConstructionService/ProductConstructionService.DependencyFlow/PullRequestUpdater.cs Outdated Show resolved Hide resolved

Update src/ProductConstructionService/ProductConstructionService.Depe…

b6df5c2

…ndencyFlow/PullRequestUpdater.cs

premun reviewed Jan 29, 2025

View reviewed changes

dkurepa and others added 5 commits January 29, 2025 17:37

Update test/ProductConstructionService.DependencyFlow.Tests/PullReque…

1cf0352

…stUpdaterTests.cs Co-authored-by: Přemek Vysoký <premek.vysoky@microsoft.com>

Update test/ProductConstructionService.ScenarioTests/CodeFlowScenario…

63b310a

…TestBase.cs Co-authored-by: Přemek Vysoký <premek.vysoky@microsoft.com>

Update test/ProductConstructionService.ScenarioTests/ScenarioTestBase.cs

6831822

Co-authored-by: Přemek Vysoký <premek.vysoky@microsoft.com>

Update src/ProductConstructionService/ProductConstructionService.Depe…

ac92f08

…ndencyFlow/PullRequestUpdater.cs Co-authored-by: Přemek Vysoký <premek.vysoky@microsoft.com>

Address feedback

a31cd31

premun previously approved these changes Jan 30, 2025

View reviewed changes

src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/Exceptions.cs Outdated Show resolved Hide resolved

src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/Exceptions.cs Show resolved Hide resolved

Fix build

6544f92

dkurepa dismissed premun’s stale review via 6544f92 January 30, 2025 09:32

dkurepa and others added 2 commits January 30, 2025 10:32

Update src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/Exceptions.cs

d3fe00b

Co-authored-by: Přemek Vysoký <premek.vysoky@microsoft.com>

Update src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/Exceptions.cs

1182062

Co-authored-by: Přemek Vysoký <premek.vysoky@microsoft.com>

premun approved these changes Jan 30, 2025

View reviewed changes

dkurepa merged commit d38f76c into dotnet:main Jan 30, 2025
9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Handle CodeFlow conflicts gracefully #4387

Handle CodeFlow conflicts gracefully #4387

dkurepa commented Jan 27, 2025 •

edited

Loading

premun left a comment

premun commented Jan 28, 2025

premun Jan 29, 2025

dkurepa Jan 29, 2025

Handle CodeFlow conflicts gracefully #4387

Handle CodeFlow conflicts gracefully #4387

Conversation

dkurepa commented Jan 27, 2025 • edited Loading

premun left a comment

Choose a reason for hiding this comment

premun commented Jan 28, 2025

premun Jan 29, 2025

Choose a reason for hiding this comment

dkurepa Jan 29, 2025

Choose a reason for hiding this comment

dkurepa commented Jan 27, 2025 •

edited

Loading