From 090c7df4c45de326c4849fed8cfd0d512cb0f551 Mon Sep 17 00:00:00 2001 From: Gary Ewan Park Date: Tue, 26 Sep 2023 14:48:10 +0100 Subject: [PATCH] PR Review Commit Will squash once review is complete --- src/GitReleaseManager.Cli/Program.cs | 8 +- .../Commands/AddAssetsCommand.cs | 2 +- .../Commands/ExportCommand.cs | 6 +- .../Commands/LabelCommand.cs | 2 +- .../Extensions/MilestoneExtensions.cs | 4 +- .../MappingProfiles/GitHubProfile.cs | 3 +- .../Provider/GitLabProvider.cs | 73 ++++++------- .../ReleaseNotes/ReleaseNotesBuilder.cs | 2 +- src/GitReleaseManager.Core/VcsService.cs | 8 +- .../GitHubProviderIntegrationTests.cs | 3 - .../GitLabProviderIntegrationTests.cs | 103 ++++++++++++++++++ 11 files changed, 156 insertions(+), 58 deletions(-) create mode 100644 src/GitReleaseManager.IntegrationTests/GitLabProviderIntegrationTests.cs diff --git a/src/GitReleaseManager.Cli/Program.cs b/src/GitReleaseManager.Cli/Program.cs index f89fc72d..dc44e89c 100644 --- a/src/GitReleaseManager.Cli/Program.cs +++ b/src/GitReleaseManager.Cli/Program.cs @@ -202,17 +202,15 @@ private static void RegisterVcsProvider(BaseVcsOptions vcsOptions, IServiceColle Log.Information("Using {Provider} as VCS Provider", vcsOptions.Provider); if (vcsOptions.Provider == VcsProvider.GitLab) { - var gitlabClient = new GitLabClient("https://gitlab.com", vcsOptions.Token); serviceCollection - .AddSingleton() - .AddSingleton(gitlabClient); + .AddSingleton((_) => new GitLabClient("https://gitlab.com", vcsOptions.Token)) + .AddSingleton(); } else { // default to Github - var gitHubClient = new GitHubClient(new ProductHeaderValue("GitReleaseManager")) { Credentials = new Credentials(vcsOptions.Token) }; serviceCollection - .AddSingleton(gitHubClient) + .AddSingleton((_) => new GitHubClient(new ProductHeaderValue("GitReleaseManager")) { Credentials = new Credentials(vcsOptions.Token) }) .AddSingleton(); } } diff --git a/src/GitReleaseManager.Core/Commands/AddAssetsCommand.cs b/src/GitReleaseManager.Core/Commands/AddAssetsCommand.cs index 8765f18e..d8531c63 100644 --- a/src/GitReleaseManager.Core/Commands/AddAssetsCommand.cs +++ b/src/GitReleaseManager.Core/Commands/AddAssetsCommand.cs @@ -21,7 +21,7 @@ public async Task ExecuteAsync(AddAssetSubOptions options) if (vcsOptions?.Provider == Model.VcsProvider.GitLab) { - _logger.Error("The addasset command is currently not supported when targetting GitLab."); + _logger.Error("The 'addasset' command is currently not supported when targeting GitLab."); return 1; } diff --git a/src/GitReleaseManager.Core/Commands/ExportCommand.cs b/src/GitReleaseManager.Core/Commands/ExportCommand.cs index 011ae3cd..9cde1f8e 100644 --- a/src/GitReleaseManager.Core/Commands/ExportCommand.cs +++ b/src/GitReleaseManager.Core/Commands/ExportCommand.cs @@ -18,13 +18,13 @@ public ExportCommand(IVcsService vcsService, ILogger logger) public async Task ExecuteAsync(ExportSubOptions options) { - if (string.IsNullOrEmpty(options.TagName)) + if (string.IsNullOrWhiteSpace(options.TagName)) { - _logger.Information("Exporting all releases"); + _logger.Information("Exporting all releases."); } else { - _logger.Information("Exporting release {TagName}", options.TagName); + _logger.Information("Exporting release {TagName}.", options.TagName); } var releasesContent = await _vcsService.ExportReleasesAsync(options.RepositoryOwner, options.RepositoryName, options.TagName, options.SkipPrereleases).ConfigureAwait(false); diff --git a/src/GitReleaseManager.Core/Commands/LabelCommand.cs b/src/GitReleaseManager.Core/Commands/LabelCommand.cs index 1a766b90..63786d6b 100644 --- a/src/GitReleaseManager.Core/Commands/LabelCommand.cs +++ b/src/GitReleaseManager.Core/Commands/LabelCommand.cs @@ -21,7 +21,7 @@ public async Task ExecuteAsync(LabelSubOptions options) if (vcsOptions?.Provider == Model.VcsProvider.GitLab) { - _logger.Error("The label command is currently not supported when targetting GitLab."); + _logger.Error("The label command is currently not supported when targeting GitLab."); return 1; } diff --git a/src/GitReleaseManager.Core/Extensions/MilestoneExtensions.cs b/src/GitReleaseManager.Core/Extensions/MilestoneExtensions.cs index 19e29e6c..f919a6ad 100644 --- a/src/GitReleaseManager.Core/Extensions/MilestoneExtensions.cs +++ b/src/GitReleaseManager.Core/Extensions/MilestoneExtensions.cs @@ -17,13 +17,13 @@ public static Version Version(this Octokit.Milestone ver) var nameWithoutPrerelease = ver.Title.Split('-')[0]; if (nameWithoutPrerelease.StartsWith("v", StringComparison.OrdinalIgnoreCase)) { - _logger.Debug("Removing version prefix from {Name}", ver.Title); + _logger.Debug("Removing version prefix from {Name}.", ver.Title); nameWithoutPrerelease = nameWithoutPrerelease.Remove(0, 1); } if (!System.Version.TryParse(nameWithoutPrerelease, out Version parsedVersion)) { - _logger.Warning("No valid version was found on {Title}", ver.Title); + _logger.Warning("No valid version was found on {Title}.", ver.Title); return new Version(0, 0); } diff --git a/src/GitReleaseManager.Core/MappingProfiles/GitHubProfile.cs b/src/GitReleaseManager.Core/MappingProfiles/GitHubProfile.cs index afa1f072..dff999dc 100644 --- a/src/GitReleaseManager.Core/MappingProfiles/GitHubProfile.cs +++ b/src/GitReleaseManager.Core/MappingProfiles/GitHubProfile.cs @@ -1,3 +1,4 @@ +using System; using AutoMapper; using GitReleaseManager.Core.Extensions; @@ -10,7 +11,7 @@ public GitHubProfile() CreateMap() .ForMember(dest => dest.PublicNumber, act => act.MapFrom(src => src.Number)) .ForMember(dest => dest.InternalNumber, act => act.MapFrom(src => src.Id)) - .ForMember(dest => dest.IsPullRequest, act => act.MapFrom(src => src.HtmlUrl.Contains("/pull/"))) + .ForMember(dest => dest.IsPullRequest, act => act.MapFrom(src => src.HtmlUrl.IndexOf("/pull/", StringComparison.OrdinalIgnoreCase) >= 0)) .ReverseMap(); CreateMap().ReverseMap(); CreateMap().ReverseMap(); diff --git a/src/GitReleaseManager.Core/Provider/GitLabProvider.cs b/src/GitReleaseManager.Core/Provider/GitLabProvider.cs index 31e87b79..94523816 100644 --- a/src/GitReleaseManager.Core/Provider/GitLabProvider.cs +++ b/src/GitReleaseManager.Core/Provider/GitLabProvider.cs @@ -49,20 +49,21 @@ public Task DeleteAssetAsync(string owner, string repository, ReleaseAsset asset public Task UploadAssetAsync(Release release, ReleaseAssetUpload releaseAssetUpload) { - return ExecuteAsync(async () => + return ExecuteAsync(() => { - _logger.Warning("Uploading of assets is not currently supported when targetting GitLab."); + _logger.Warning("Uploading of assets is not currently supported when targeting GitLab."); + return Task.CompletedTask; }); } public Task GetCommitsCountAsync(string owner, string repository, string @base, string head) { - return ExecuteAsync(async () => + return ExecuteAsync(() => { // TODO: This is waiting on a PR being merged... // https://github.com/ubisoft/NGitLab/pull/444 // Once it is, we might be able to implement what is necessary here. - return 0; + return Task.FromResult(0); }); } @@ -79,7 +80,7 @@ public string GetCommitsUrl(string owner, string repository, string head, string public Task CreateIssueCommentAsync(string owner, string repository, Issue issue, string comment) { - return ExecuteAsync(async () => + return ExecuteAsync(() => { var projectId = GetGitLabProjectId(owner, repository); @@ -105,12 +106,14 @@ public Task CreateIssueCommentAsync(string owner, string repository, Issue issue issueNotesClient.Create(issueComment); } + + return Task.CompletedTask; }); } public Task> GetIssuesAsync(string owner, string repository, Milestone milestone, ItemStateFilter itemStateFilter = ItemStateFilter.All) { - return ExecuteAsync(async () => + return ExecuteAsync(() => { var issuesClient = _gitLabClient.Issues; @@ -150,13 +153,13 @@ public Task> GetIssuesAsync(string owner, string repository, issuesAndMergeRequests.AddRange(_mapper.Map>(issues)); issuesAndMergeRequests.AddRange(_mapper.Map>(mergeRequests)); - return issuesAndMergeRequests.AsEnumerable(); + return Task.FromResult(issuesAndMergeRequests.AsEnumerable()); }); } public Task> GetIssueCommentsAsync(string owner, string repository, Issue issue) { - return ExecuteAsync(async () => + return ExecuteAsync(() => { IEnumerable issueComments = Enumerable.Empty(); var projectId = GetGitLabProjectId(owner, repository); @@ -175,7 +178,7 @@ public Task> GetIssueCommentsAsync(string owner, strin issueComments = _mapper.Map>(comments); } - return issueComments; + return Task.FromResult(issueComments); }); } @@ -221,7 +224,7 @@ public Task GetMilestoneAsync(string owner, string repository, string public Task> GetMilestonesAsync(string owner, string repository, ItemStateFilter itemStateFilter = ItemStateFilter.All) { - return ExecuteAsync(async () => + return ExecuteAsync(() => { var query = new MilestoneQuery(); @@ -244,13 +247,13 @@ public Task> GetMilestonesAsync(string owner, string repo mappedMilestone.HtmlUrl = string.Format(CultureInfo.InvariantCulture, "https://gitlab.com/{0}/{1}/-/milestones/{2}#tab-issues", owner, repository, mappedMilestone.PublicNumber); } - return mappedMilestones; + return Task.FromResult(mappedMilestones); }); } public Task SetMilestoneStateAsync(string owner, string repository, Milestone milestone, ItemState itemState) { - return ExecuteAsync(async () => + return ExecuteAsync(() => { var mileStoneClient = _gitLabClient.GetMilestone(GetGitLabProjectId(owner, repository)); @@ -262,12 +265,14 @@ public Task SetMilestoneStateAsync(string owner, string repository, Milestone mi { mileStoneClient.Close(milestone.InternalNumber); } + + return Task.CompletedTask; }); } public Task CreateReleaseAsync(string owner, string repository, Release release) { - return ExecuteAsync(async () => + return ExecuteAsync(() => { var releaseClient = _gitLabClient.GetReleases(GetGitLabProjectId(owner, repository)); @@ -280,23 +285,24 @@ public Task CreateReleaseAsync(string owner, string repository, Release mappedRelease.HtmlUrl = string.Format(CultureInfo.InvariantCulture, "https://gitlab.com/{0}/{1}/-/releases/{2}", owner, repository, release.TagName); } - return mappedRelease; + return Task.FromResult(mappedRelease); }); } public Task DeleteReleaseAsync(string owner, string repository, Release release) { - return ExecuteAsync(async () => + return ExecuteAsync(() => { var releaseClient = _gitLabClient.GetReleases(GetGitLabProjectId(owner, repository)); releaseClient.Delete(release.TagName); + return Task.CompletedTask; }); } public Task GetReleaseAsync(string owner, string repository, string tagName) { - return ExecuteAsync(async () => + return ExecuteAsync(() => { var releaseClient = _gitLabClient.GetReleases(GetGitLabProjectId(owner, repository)); @@ -310,7 +316,7 @@ public Task GetReleaseAsync(string owner, string repository, string tag mappedRelease.HtmlUrl = string.Format(CultureInfo.InvariantCulture, "https://gitlab.com/{0}/{1}/-/releases/{2}", owner, repository, tagName); } - return mappedRelease; + return Task.FromResult(mappedRelease); }); } @@ -326,7 +332,7 @@ public Task> GetReleasesAsync(string owner, string reposito public Task PublishReleaseAsync(string owner, string repository, string tagName, Release release) { - return ExecuteAsync(async () => + return ExecuteAsync(() => { var releaseClient = _gitLabClient.GetReleases(GetGitLabProjectId(owner, repository)); @@ -337,18 +343,18 @@ public Task PublishReleaseAsync(string owner, string repository, string tagName, }; releaseClient.Update(update); + return Task.CompletedTask; }); } public Task UpdateReleaseAsync(string owner, string repository, Release release) { - return ExecuteAsync(async () => + return ExecuteAsync(() => { var releaseClient = _gitLabClient.GetReleases(GetGitLabProjectId(owner, repository)); var update = new ReleaseUpdate { - Description = release.Body, ReleasedAt = release.Draft ? DateTime.UtcNow.AddYears(1) : DateTime.UtcNow, Name = release.Name, @@ -357,6 +363,7 @@ public Task UpdateReleaseAsync(string owner, string repository, Release release) }; releaseClient.Update(update); + return Task.CompletedTask; }); } @@ -370,7 +377,7 @@ public RateLimit GetRateLimit() public string GetMilestoneQueryString() { - return "sort=due_date_desc&state=closed"; + return "state=closed"; } public string GetIssueType(Issue issue) @@ -398,14 +405,10 @@ private async Task ExecuteAsync(Func action) { await action().ConfigureAwait(false); } - ////catch (Octokit.ForbiddenException ex) - ////{ - //// throw new ForbiddenException(ex.Message, ex); - ////} - ////catch (Octokit.NotFoundException ex) - ////{ - //// throw new NotFoundException(ex.Message, ex); - ////} + catch (AggregateException ae) + { + throw new ApiException(ae.Message, ae); + } catch (Exception ex) when (!(ex is NotFoundException)) { throw new ApiException(ex.Message, ex); @@ -418,14 +421,10 @@ private async Task ExecuteAsync(Func> action) { return await action().ConfigureAwait(false); } - ////catch (Octokit.ForbiddenException ex) - ////{ - //// throw new ForbiddenException(ex.Message, ex); - ////} - ////catch (Octokit.NotFoundException ex) - ////{ - //// throw new NotFoundException(ex.Message, ex); - ////} + catch (AggregateException ae) + { + throw new ApiException(ae.Message, ae); + } catch (Exception ex) when (!(ex is NotFoundException)) { throw new ApiException(ex.Message, ex); diff --git a/src/GitReleaseManager.Core/ReleaseNotes/ReleaseNotesBuilder.cs b/src/GitReleaseManager.Core/ReleaseNotes/ReleaseNotesBuilder.cs index 1ee2b426..79273145 100644 --- a/src/GitReleaseManager.Core/ReleaseNotes/ReleaseNotesBuilder.cs +++ b/src/GitReleaseManager.Core/ReleaseNotes/ReleaseNotesBuilder.cs @@ -60,7 +60,7 @@ public async Task BuildReleaseNotesAsync(string user, string repository, if (issues.Count == 0) { - var logMessage = string.Format(CultureInfo.InvariantCulture, "No closed issues have been found for milestone {0}, or all assigned issues are meant to be excluded from release notes, aborting creation of release.", _milestoneTitle); + var logMessage = string.Format(CultureInfo.CurrentCulture, "No closed issues have been found for milestone {0}, or all assigned issues are meant to be excluded from release notes, aborting release creation.", _milestoneTitle); throw new InvalidOperationException(logMessage); } diff --git a/src/GitReleaseManager.Core/VcsService.cs b/src/GitReleaseManager.Core/VcsService.cs index c98ec420..601a20b3 100644 --- a/src/GitReleaseManager.Core/VcsService.cs +++ b/src/GitReleaseManager.Core/VcsService.cs @@ -142,7 +142,7 @@ private async Task AddAssetsAsync(string owner, string repository, string tagNam { if (!File.Exists(asset)) { - var message = string.Format(CultureInfo.InvariantCulture, "Requested asset to be uploaded doesn't exist: {0}", asset); + var message = string.Format(CultureInfo.CurrentCulture, "The requested asset to be uploaded doesn't exist: {0}", asset); throw new FileNotFoundException(message); } @@ -155,7 +155,7 @@ private async Task AddAssetsAsync(string owner, string repository, string tagNam if (_vcsProvider is GitLabProvider) { - _logger.Error("Deleting of assets is not currently supported when targetting GitLab."); + _logger.Error("Deleting assets is not currently supported when targeting GitLab."); } else { @@ -186,7 +186,7 @@ private async Task AddAssetsAsync(string owner, string repository, string tagNam if (!release.Body.Contains(_configuration.Create.ShaSectionHeading)) { _logger.Debug("Creating SHA section header"); - stringBuilder.AppendLine(string.Format(CultureInfo.InvariantCulture, "### {0}", _configuration.Create.ShaSectionHeading)); + stringBuilder.AppendFormat(CultureInfo.InvariantCulture, "### {0}", _configuration.Create.ShaSectionHeading).AppendLine(); } foreach (var asset in assets) @@ -405,7 +405,7 @@ private async Task AddIssueCommentsAsync(string owner, string repository, Milest } catch (ForbiddenException) { - _logger.Error("Unable to add comment to issue #{IssueNumber}. Insufficient permissions.", issue.PublicNumber); + _logger.Error("Unable to add a comment to issue #{IssueNumber}. Insufficient permissions.", issue.PublicNumber); break; } } diff --git a/src/GitReleaseManager.IntegrationTests/GitHubProviderIntegrationTests.cs b/src/GitReleaseManager.IntegrationTests/GitHubProviderIntegrationTests.cs index 9133a056..9c7483ad 100644 --- a/src/GitReleaseManager.IntegrationTests/GitHubProviderIntegrationTests.cs +++ b/src/GitReleaseManager.IntegrationTests/GitHubProviderIntegrationTests.cs @@ -6,7 +6,6 @@ using GitReleaseManager.Core.Provider; using NUnit.Framework; using Octokit; -using Serilog; using Shouldly; using Issue = GitReleaseManager.Core.Model.Issue; using Milestone = GitReleaseManager.Core.Model.Milestone; @@ -24,7 +23,6 @@ public class GitHubProviderIntegrationTests private GitHubProvider _gitHubProvider; private IGitHubClient _gitHubClient; private IMapper _mapper; - private ILogger _logger; private string _token; private string _releaseBaseTag; @@ -43,7 +41,6 @@ public void OneTimeSetUp() } _mapper = AutoMapperConfiguration.Configure(); - _logger = new LoggerConfiguration().WriteTo.Console().CreateLogger(); _gitHubClient = new GitHubClient(new ProductHeaderValue("GitReleaseManager")) { Credentials = new Credentials(_token) }; _gitHubProvider = new GitHubProvider(_gitHubClient, _mapper); } diff --git a/src/GitReleaseManager.IntegrationTests/GitLabProviderIntegrationTests.cs b/src/GitReleaseManager.IntegrationTests/GitLabProviderIntegrationTests.cs new file mode 100644 index 00000000..91306939 --- /dev/null +++ b/src/GitReleaseManager.IntegrationTests/GitLabProviderIntegrationTests.cs @@ -0,0 +1,103 @@ +using System; +using System.Linq; +using System.Threading.Tasks; +using AutoMapper; +using GitReleaseManager.Core; +using GitReleaseManager.Core.Provider; +using NGitLab; +using NUnit.Framework; +using Serilog; +using Shouldly; +using Issue = GitReleaseManager.Core.Model.Issue; +using Milestone = GitReleaseManager.Core.Model.Milestone; + +namespace GitReleaseManager.IntegrationTests +{ + [TestFixture] + [Explicit] + public class GitLabProviderIntegrationTests + { + private const string OWNER = "gep13"; + private const string REPOSITORY = "grm-test"; + private const bool SKIP_PRERELEASES = false; + + private GitLabProvider _gitLabProvider; + private IGitLabClient _gitLabClient; + private IMapper _mapper; + private ILogger _logger; + + private string _token; + private string _releaseBaseTag; + private string _releaseHeadTag; + private Issue _issue; + private Milestone _milestone; + + [OneTimeSetUp] + public void OneTimeSetUp() + { + _token = Environment.GetEnvironmentVariable("GITTOOLS_GITLAB_TOKEN"); + + if (string.IsNullOrWhiteSpace(_token)) + { + Assert.Inconclusive("Unable to locate credentials for accessing GitLab API"); + } + + _mapper = AutoMapperConfiguration.Configure(); + _logger = new LoggerConfiguration().WriteTo.Console().CreateLogger(); + _gitLabClient = new GitLabClient("https://gitlab.com", _token); + _gitLabProvider = new GitLabProvider(_gitLabClient, _mapper, _logger); + } + + [Test] + [Order(1)] + public async Task Should_Get_Milestones() + { + var result = await _gitLabProvider.GetMilestonesAsync(OWNER, REPOSITORY).ConfigureAwait(false); + result.Count().ShouldBeGreaterThan(0); + + _milestone = result.OrderByDescending(m => m.PublicNumber).First(); + } + + [Test] + [Order(2)] + public async Task Should_Get_Issues() + { + var result = await _gitLabProvider.GetIssuesAsync(OWNER, REPOSITORY, _milestone).ConfigureAwait(false); + result.Count().ShouldBeGreaterThan(0); + + _issue = result.First(); + } + + [Test] + [Order(3)] + public async Task Should_Get_Issue_Comments() + { + var result = await _gitLabProvider.GetIssueCommentsAsync(OWNER, REPOSITORY, _issue).ConfigureAwait(false); + result.Count().ShouldBeGreaterThan(0); + } + + [Test] + [Order(4)] + public async Task Should_Get_Releases() + { + var result = await _gitLabProvider.GetReleasesAsync(OWNER, REPOSITORY, SKIP_PRERELEASES).ConfigureAwait(false); + result.Count().ShouldBeGreaterThan(0); + + var orderedReleases = result.OrderByDescending(r => r.Id).ToList(); + + _releaseBaseTag = orderedReleases[1].TagName; + _releaseHeadTag = orderedReleases[0].TagName; + } + + [Test] + [Order(5)] + public async Task Should_Get_Commits_Count() + { + // TODO: This is waiting on a PR being merged... + // https://github.com/ubisoft/NGitLab/pull/444 + // Once it is, we might be able to implement what is necessary here. + // var result = await _gitLabProvider.GetCommitsCountAsync(OWNER, REPOSITORY, _releaseBaseTag, _releaseHeadTag).ConfigureAwait(false); + // result.ShouldBeGreaterThan(0); + } + } +}