diff --git a/.gitignore b/.gitignore index d5a665fd..e85f7850 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,7 @@ .idea/ .vs/ *.files +*.lscache *.mdb *.nupkg *.orig diff --git a/src/LibChorus/VcsDrivers/Mercurial/HgRepository.cs b/src/LibChorus/VcsDrivers/Mercurial/HgRepository.cs index d69f1b36..0f5eb695 100644 --- a/src/LibChorus/VcsDrivers/Mercurial/HgRepository.cs +++ b/src/LibChorus/VcsDrivers/Mercurial/HgRepository.cs @@ -294,16 +294,9 @@ public bool Pull(RepositoryAddress source, string targetUri) public void PushToTarget(string targetLabel, string targetUri) { - try - { - CheckAndUpdateHgrc(); - Execute(false, _mercurialTwoCompatible,SecondsBeforeTimeoutOnRemoteOperation, "push --new-branch " + GetProxyConfigParameterString(targetUri), SurroundWithQuotes(targetUri)); - } - catch (Exception err) - { - _progress.WriteMessageWithColor("OrangeRed", - $"Could not send to {ServerSettingsModel.RemovePasswordForLog(targetUri)}{Environment.NewLine}{err.Message}"); - } + CheckAndUpdateHgrc(); + Execute(false, _mercurialTwoCompatible, SecondsBeforeTimeoutOnRemoteOperation, + "push --new-branch " + GetProxyConfigParameterString(targetUri), SurroundWithQuotes(targetUri)); if (GetIsLocalUri(targetUri)) { @@ -313,8 +306,9 @@ public void PushToTarget(string targetLabel, string targetUri) } catch (Exception err) { - _progress.WriteMessageWithColor("OrangeRed", - $"Could not update the actual files after a pushing to {ServerSettingsModel.RemovePasswordForLog(targetUri)}{Environment.NewLine}{err.Message}"); + throw new ApplicationException( + $"Could not update the actual files after pushing to {ServerSettingsModel.RemovePasswordForLog(targetUri)}", + err); } } } diff --git a/src/LibChorus/sync/Synchronizer.cs b/src/LibChorus/sync/Synchronizer.cs index c2173e30..31a00f05 100644 --- a/src/LibChorus/sync/Synchronizer.cs +++ b/src/LibChorus/sync/Synchronizer.cs @@ -251,16 +251,39 @@ public void SetIsOneOfDefaultSyncAddresses(RepositoryAddress address, bool enabl private void SendToOthers(HgRepository repo, List sourcesToTry, Dictionary connectionAttempt) { using var activity = LibChorusActivitySource.Value.StartActivity(); + List errors = new List(); + bool atLeastOneSucceeded = false; foreach (RepositoryAddress address in sourcesToTry) { ThrowIfCancelPending(); if (!address.IsReadOnly) { - SendToOneOther(address, connectionAttempt, repo); + try + { + SendToOneOther(address, connectionAttempt, repo); + atLeastOneSucceeded = true; + } + catch (UserCancelledException) + { + throw; + } + catch (Exception e) + { + _progress.WriteException(e); + _progress.WriteError(e.Message); + errors.Add(e); + } } } ThrowIfCancelPending(); + + // Only throw and fail the sync if every writable target failed; partial success still counts as success. + if (errors.Any() && !atLeastOneSucceeded) + { + var error = errors.Count == 1 ? errors.Single() : new AggregateException(errors); + throw new SynchronizationException(error, WhatToDo.CheckAddressAndConnection, "Failed to send changes"); + } } private void ThrowIfCancelPending() diff --git a/src/LibChorusTests/sync/Synchronizer.BadSituationTests.cs b/src/LibChorusTests/sync/Synchronizer.BadSituationTests.cs index c40af5ec..41137d71 100644 --- a/src/LibChorusTests/sync/Synchronizer.BadSituationTests.cs +++ b/src/LibChorusTests/sync/Synchronizer.BadSituationTests.cs @@ -4,6 +4,7 @@ using Chorus.merge; using Chorus.sync; using Chorus.Utilities; +using Chorus.VcsDrivers; using Chorus.VcsDrivers.Mercurial; using LibChorus.TestUtilities; using NUnit.Framework; @@ -55,6 +56,81 @@ public void Sync_HgrcInUseByOther_FailsGracefully() } } + [Test] + public void Sync_PushFails_ErrorEncounteredIsSet() + { + using (var sender = new RepositorySetup("sender")) + { + sender.AddAndCheckinFile("one.txt", "sender's content"); + + var options = new SyncOptions + { + DoMergeWithOthers = false, + DoPullFromOthers = false, + DoSendToOthers = true, + }; + options.RepositorySourcesToTry.Add(new FailingConnectableHttpRepositoryAddress("test-repo")); + + var result = sender.SyncWithOptions(options); + Assert.That(result.Succeeded, Is.False); + Assert.That(result.ErrorEncountered, Is.Not.Null); + Assert.That(result.ErrorEncountered.Message, Does.Contain("Failed to send changes")); + Assert.That(sender.GetProgressString(), Does.Contain("Failed to send to test-repo")); + } + } + + [Test] + public void Sync_FirstPushFails_OtherSourcesStillTried() + { + using (var sender = new RepositorySetup("sender")) + { + sender.AddAndCheckinFile("one.txt", "sender's content"); + + var options = new SyncOptions + { + DoMergeWithOthers = false, + DoPullFromOthers = false, + DoSendToOthers = true, + }; + options.RepositorySourcesToTry.Add(new FailingConnectableHttpRepositoryAddress("first-test-repo")); + options.RepositorySourcesToTry.Add(new FailingConnectableHttpRepositoryAddress("second-test-repo")); + + var result = sender.SyncWithOptions(options); + Assert.That(result.Succeeded, Is.False); + Assert.That(result.ErrorEncountered, Is.Not.Null); + // Both targets should have been attempted even though the first failed. + var progress = sender.GetProgressString(); + Assert.That(progress, Does.Contain("Failed to send to first-test-repo")); + Assert.That(progress, Does.Contain("Failed to send to second-test-repo")); + } + } + + [Test] + public void Sync_OneOfMultiplePushTargetsFails_SyncStillSucceeds() + { + using (var sender = new RepositorySetup("sender")) + { + sender.AddAndCheckinFile("one.txt", "sender's content"); + + using (var receiver = new RepositorySetup("receiver", sender)) + { + var options = new SyncOptions + { + DoMergeWithOthers = false, + DoPullFromOthers = false, + DoSendToOthers = true, + }; + options.RepositorySourcesToTry.Add(new FailingConnectableHttpRepositoryAddress("test-repo")); + options.RepositorySourcesToTry.Add(receiver.GetRepositoryAddress()); + + var result = sender.SyncWithOptions(options); + Assert.That(result.Succeeded, Is.True); + Assert.That(result.ErrorEncountered, Is.Null); + Assert.That(sender.GetProgressString(), Does.Contain("Failed to send to test-repo")); + } + } + } + [Test] public void Sync_ExceptionInMergeCode_LeftWith2HeadsAndErrorOutputToProgress() { @@ -518,5 +594,27 @@ public void Sync_NewFileWithNonAsciCharacters_FileAdded() } } + /// + /// Test double that bypasses HttpRepositoryPath's remote validation. + /// HttpRepositoryPath.CanConnect() validates the endpoint and aborts if invalid, preventing + /// sync operations from executing. This allows CanConnect() to return true while sync operations + /// fail, enabling tests of sync failure scenarios (push/pull/merge failures). + /// + private class FailingConnectableHttpRepositoryAddress : RepositoryAddress + { + public FailingConnectableHttpRepositoryAddress(string name) : base(name, "http://127.0.0.1:1/test-project", false) + { + } + + public override bool CanConnect(HgRepository localRepository, string projectName, IProgress progress) + { + return true; + } + + public override string GetPotentialRepoUri(string repoIdentifier, string projectName, IProgress progress) + { + return URI; + } + } } -} \ No newline at end of file +}