Fix side panel sometimes ending up with duplicate sets of remotes whe…

[amaiorano]

…n "Activate & fetch" selected on an inactive remote

Problem is that the "fetch" operation would invoke UICommands.RepoChangedNotifier.Notify(), then this same function would get invoked yet again right after. This second call would sometimes cancel the first, which would cancel the repopulation of nodes in the TreeView.

Fixes #6106

Proposed changes

• Don't call UICommands.RepoChangedNotifier.Notify() twice in a row from the side panel as this might cancel the tree view update

Test methodology

• Manual testing. This was easier to reproduce without a debugger attached (timing), and when activating a repo with many branches (e.g. gerhardol's fork)

Test environment(s)

• GIT 2.19.1.windows.1
• Windows 10

[amaiorano]

@gerhardol can you try this branch and see if it fixes the problem for you?

[codecov]

Codecov Report

Merging #6107 into master will decrease coverage by <.01%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master    #6107      +/-   ##
==========================================
- Coverage   44.27%   44.26%   -0.01%     
==========================================
  Files         646      646              
  Lines       49028    49029       +1     
  Branches     6552     6552              
==========================================
- Hits        21707    21705       -2     
- Misses      26088    26090       +2     
- Partials     1233     1234       +1

[RussKie]

I'm adding a scaffold for ROT UI tests, we should be able simulate the issue and write a test to confirm the fix.

…

On Tue, Jan 15, 2019, 2:43 PM codecov[bot] ***@***.*** wrote: Codecov <https://codecov.io/gh/gitextensions/gitextensions/pull/6107?src=pr&el=h1> Report Merging #6107 <https://codecov.io/gh/gitextensions/gitextensions/pull/6107?src=pr&el=desc> into master <https://codecov.io/gh/gitextensions/gitextensions/commit/17a779e9e0b674e902fe1e2df87c1431b63ea877?src=pr&el=desc> will *decrease* coverage by <.01%. The diff coverage is 0%. @@ Coverage Diff @@## master #6107 +/- ## ==========================================- Coverage 44.27% 44.26% -0.01% ========================================== Files 646 646 Lines 49028 49029 +1 Branches 6552 6552 ==========================================- Hits 21707 21705 -2 - Misses 26088 26090 +2 - Partials 1233 1234 +1 — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#6107 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEMyXrWHmV6loe04hy9Q0GxpkZtbzaC-ks5vDU58gaJpZM4aALcA> .

[gerhardol]

Will try tonight

[amaiorano]

I'm adding a scaffold for ROT UI tests, we should be able simulate the issue and write a test to confirm the fix.

Maybe, but it was actually very difficult to reproduce, as you can probably see from my comments above. I managed to reproduce it by figuring out what the problem might be. In fact, I modified the code to invoke UICommands.RepoChangedNotifier.Notify(); three times in a loop, then detached my debugger and was able to reproduce more easily. In truth, this might mean we have a deeper problem related to cancellation in general for the left panel.

[gerhardol]

Works fine
Testing with debug build of GE, standalone (not in debugger), Activate&Fetch followed by Deactivate of drewnoakes in my GE repo with 113 branches an6 6+25 remotes.
Before, I got duplication 5 out of 5 times and after I got none of 10 times.

In truth, this might mean we have a deeper problem related to cancellation in general for the left panel.

Will your pubsub branch improve here?

[amaiorano]

Will your pubsub branch improve here?

Maybe. I think we just have to be careful not to issue more than one refresh from the left panel code itself, since a second refresh cancels the first.

Thanks for the review. Can I merge this immediately?

[gerhardol]

Thanks for the review. Can I merge this immediately?

I do not think that the chief will mind that, go ahead.

[RussKie]

I can see you 😎