diff --git a/src/Elastic.Codex/Sourcing/CodexCloneService.cs b/src/Elastic.Codex/Sourcing/CodexCloneService.cs index e0cfd0eca7..a00b906f2d 100644 --- a/src/Elastic.Codex/Sourcing/CodexCloneService.cs +++ b/src/Elastic.Codex/Sourcing/CodexCloneService.cs @@ -84,6 +84,11 @@ public class CodexCloneService(ILoggerFactory logFactory, ILinkIndexReader linkI try { var git = new CodexGitRepository(loggerFactory, context.Collector, subDir); + if (!git.HasHead()) + { + logger.LogWarning("Could not read commit for {Name}; skipping", repoName); + continue; + } currentCommit = git.GetCurrentCommit(); } catch (OperationCanceledException) @@ -186,7 +191,10 @@ bool assumeCloned try { - var git = new CodexGitRepository(logFactory, context.Collector, repoDir); + // Git command failures emit collector errors. Use a local collector so a + // missing clone token policy warns and skips instead of failing the job. + var gitCollector = new DiagnosticsCollector([]); + var git = new CodexGitRepository(logFactory, gitCollector, repoDir); if (assumeCloned && git.IsInitialized()) { diff --git a/src/Elastic.Codex/Sourcing/CodexGitRepository.cs b/src/Elastic.Codex/Sourcing/CodexGitRepository.cs index d4ee79e0dc..7457f514c9 100644 --- a/src/Elastic.Codex/Sourcing/CodexGitRepository.cs +++ b/src/Elastic.Codex/Sourcing/CodexGitRepository.cs @@ -53,25 +53,34 @@ protected override void OnBeforeRetry() => public bool IsInitialized() => Directory.Exists(Path.Join(WorkingDirectory.FullName, ".git")); - public void Fetch(string reference) => - _ = ExecInWithRetry( - EnvironmentVars, - NetworkRetry, - "git", - "fetch", - "--no-tags", - "--prune", - "--no-recurse-submodules", - "--depth", - "1", - "origin", - reference - ); + public void Fetch(string reference) + { + if ( + !ExecInWithRetry( + EnvironmentVars, + NetworkRetry, + "git", + "fetch", + "--no-tags", + "--prune", + "--no-recurse-submodules", + "--depth", + "1", + "origin", + reference + ) + ) + throw new InvalidOperationException($"git fetch failed for '{reference}'"); + } public void EnableSparseCheckout(string[] folders) => ExecIn(EnvironmentVars, "git", ["sparse-checkout", "set", "--no-cone", .. folders]); - public void Checkout(string reference) => ExecIn(EnvironmentVars, "git", "checkout", "--force", reference); + public void Checkout(string reference) + { + if (!ExecInWithRetry(EnvironmentVars, RetryPolicy.None, "git", "checkout", "--force", reference)) + throw new InvalidOperationException($"git checkout failed for '{reference}'"); + } public void GitAddOrigin(string origin) => ExecIn(EnvironmentVars, "git", "remote", "add", "origin", origin); } diff --git a/tests/Navigation.Tests/Codex/CodexGitRepositoryTests.cs b/tests/Navigation.Tests/Codex/CodexGitRepositoryTests.cs new file mode 100644 index 0000000000..d0ccc05f8b --- /dev/null +++ b/tests/Navigation.Tests/Codex/CodexGitRepositoryTests.cs @@ -0,0 +1,77 @@ +// Licensed to Elasticsearch B.V under one or more agreements. +// Elasticsearch B.V licenses this file to you under the Apache 2.0 License. +// See the LICENSE file in the project root for more information + +using System.IO.Abstractions; +using System.IO.Abstractions.TestingHelpers; +using AwesomeAssertions; +using Elastic.Codex.Sourcing; +using Elastic.Documentation.Diagnostics; +using Elastic.Documentation.ExternalCommands; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; + +namespace Elastic.Documentation.Navigation.Tests.Codex; + +public class CodexGitRepositoryTests +{ + [Fact] + public void Fetch_WhenGitFails_ThrowsWithoutLeavingTheFailureAsAnUncaughtJobError() + { + var gitCollector = new DiagnosticsCollector([]); + var git = CreateGit(gitCollector, ExitCode(128), ExitCode(128), ExitCode(128)); + + var act = () => git.Fetch("abc123"); + + act.Should().Throw().WithMessage("git fetch failed for 'abc123'"); + gitCollector.Errors.Should().Be(1); + } + + [Fact] + public void Checkout_WhenGitFails_ThrowsAfterRecordingTheFailureOnTheGitCollector() + { + var gitCollector = new DiagnosticsCollector([]); + var git = CreateGit(gitCollector, ExitCode(1)); + + var act = () => git.Checkout("FETCH_HEAD"); + + act.Should().Throw().WithMessage("git checkout failed for 'FETCH_HEAD'"); + gitCollector.Errors.Should().Be(1); + } + + private static Func ExitCode(int code) => () => code; + + private static ScriptedCodexGitRepository CreateGit(IDiagnosticsCollector gitCollector, params Func[] steps) + { + var fileSystem = new MockFileSystem(); + var workingDirectory = fileSystem.DirectoryInfo.New("/tmp/clone/repo"); + workingDirectory.Create(); + return new ScriptedCodexGitRepository(gitCollector, workingDirectory, steps); + } + + private sealed class ScriptedCodexGitRepository( + IDiagnosticsCollector collector, + IDirectoryInfo workingDirectory, + Func[] steps + ) : CodexGitRepository(NullLoggerFactory.Instance, collector, workingDirectory) + { + private int _callCount; + + protected override int ExecInCore( + Dictionary environmentVars, + TimeSpan? attemptTimeout, + string binary, + params string[] args + ) + { + if (_callCount >= steps.Length) + throw new InvalidOperationException($"Unexpected invocation {_callCount + 1}"); + return steps[_callCount++](); + } + + protected override void DelayBeforeRetry(TimeSpan delay) + { + // Tests must not wait on the production 5s fetch back-off. + } + } +} diff --git a/tests/Navigation.Tests/Navigation.Tests.csproj b/tests/Navigation.Tests/Navigation.Tests.csproj index 7efb6503c1..ed92b1c5c4 100644 --- a/tests/Navigation.Tests/Navigation.Tests.csproj +++ b/tests/Navigation.Tests/Navigation.Tests.csproj @@ -9,6 +9,7 @@ +