diff --git a/.github/workflows/ci-pipeline.yml b/.github/workflows/ci-pipeline.yml index 26ba8af3fe..06285af65a 100644 --- a/.github/workflows/ci-pipeline.yml +++ b/.github/workflows/ci-pipeline.yml @@ -509,11 +509,32 @@ jobs: path: ~/byond-zips-cache key: byond-zips - - name: Run Live Tests + - name: Run Live Tests # Logging here is weird because printing massive amounts of text on Windows runners is SLOW AS SHIT!!! + id: live-tests run: | cd tests/Tgstation.Server.Tests Start-Sleep -Seconds 10 - dotnet test -c ${{ matrix.configuration }} --no-build --filter TestCategory=RequiresDatabase --logger "GitHubActions;summary.includePassedTests=true;summary.includeSkippedTests=true" --collect:"XPlat Code Coverage" --settings ../../build/ci.runsettings --results-directory ../../TestResults + $ErrorActionPreference="SilentlyContinue" + $test_output = dotnet test -c ${{ matrix.configuration }} --no-build --filter TestCategory=RequiresDatabase --logger "GitHubActions;summary.includePassedTests=true;summary.includeSkippedTests=true" --collect:"XPlat Code Coverage" --settings ../../build/ci.runsettings --results-directory ../../TestResults + $succeeded = $? + $ErrorActionPreference="Stop" + cd ../.. + $test_output | Out-File -FilePath ./test_output.txt + if (-Not $succeeded) { + echo "succeeded=NO" >> $env:GITHUB_OUTPUT + } else { + echo "succeeded=YES" >> $env:GITHUB_OUTPUT + } + + - name: Store Live Tests Output + uses: actions/upload-artifact@v3 + with: + name: windows-integration-test-logs-${{ matrix.configuration }}-${{ matrix.watchdog-type }}-${{ matrix.database-type }} + path: ./test_output.txt + + - name: Fail if Live Tests Failed + if: ${{ steps.live-tests.outputs.succeeded != 'YES' }} + run: exit 1 - name: Store Code Coverage uses: actions/upload-artifact@v3 diff --git a/src/Tgstation.Server.Host/Extensions/DatabaseCollectionExtensions.cs b/src/Tgstation.Server.Host/Extensions/DatabaseCollectionExtensions.cs index 8514452de5..ea50a7af17 100644 --- a/src/Tgstation.Server.Host/Extensions/DatabaseCollectionExtensions.cs +++ b/src/Tgstation.Server.Host/Extensions/DatabaseCollectionExtensions.cs @@ -1,4 +1,6 @@ -using System.Linq; +using System; +using System.Linq; +using System.Linq.Expressions; using System.Threading; using System.Threading.Tasks; @@ -15,19 +17,26 @@ namespace Tgstation.Server.Host.Extensions static class DatabaseCollectionExtensions { /// - /// Gets the unattached, unpopulated with the name . + /// Gets with the name . /// + /// The transformed return . /// The of s to operate on. + /// A selecting for transforming the returned into the . /// The for the operation. /// A resulting in the unattached TGS on success, on failure. - public static Task GetTgsUser(this IDatabaseCollection databaseCollection, CancellationToken cancellationToken) - => databaseCollection + public static Task GetTgsUser( + this IDatabaseCollection databaseCollection, + Expression> selector, + CancellationToken cancellationToken) + { + ArgumentNullException.ThrowIfNull(databaseCollection); + ArgumentNullException.ThrowIfNull(selector); + + return databaseCollection .AsQueryable() .Where(x => x.CanonicalName == User.CanonicalizeName(User.TgsSystemUserName)) - .Select(x => new User - { - Id = x.Id, - }) + .Select(selector) .FirstAsync(cancellationToken); + } } } diff --git a/src/Tgstation.Server.Host/Jobs/JobService.cs b/src/Tgstation.Server.Host/Jobs/JobService.cs index 41a68e56bf..c74204cd8e 100644 --- a/src/Tgstation.Server.Host/Jobs/JobService.cs +++ b/src/Tgstation.Server.Host/Jobs/JobService.cs @@ -114,61 +114,82 @@ namespace Tgstation.Server.Host.Jobs } /// - public ValueTask RegisterOperation(Job job, JobEntrypoint operation, CancellationToken cancellationToken) - => databaseContextFactory.UseContext( + public async ValueTask RegisterOperation(Job job, JobEntrypoint operation, CancellationToken cancellationToken) + { + ArgumentNullException.ThrowIfNull(job); + ArgumentNullException.ThrowIfNull(operation); + + job.StartedAt = DateTimeOffset.UtcNow; + job.Cancelled = false; + + if (job.StartedBy != null) + { + if (!job.StartedBy.Id.HasValue) + throw new InvalidOperationException("StartedBy User associated with job does not have an Id!"); + + if (job.StartedBy.Name == null) + throw new InvalidOperationException("StartedBy User associated with job does not have a Name!"); + } + + var originalStartedBy = job.StartedBy; + await databaseContextFactory.UseContext( async databaseContext => { - ArgumentNullException.ThrowIfNull(job); - ArgumentNullException.ThrowIfNull(operation); - - job.StartedAt = DateTimeOffset.UtcNow; - job.Cancelled = false; - job.Instance = new Models.Instance { Id = job.Instance.Id.Value, }; + databaseContext.Instances.Attach(job.Instance); - if (job.StartedBy == null) - job.StartedBy = await databaseContext - .Users - .GetTgsUser(cancellationToken); - else - job.StartedBy = new User - { - Id = job.StartedBy.Id ?? throw new InvalidOperationException("StartedBy User associated with job does not have an Id!"), - }; + originalStartedBy ??= await databaseContext + .Users + .GetTgsUser( + dbUser => new User + { + Id = dbUser.Id.Value, + Name = dbUser.Name, + }, + cancellationToken); + + job.StartedBy = new User + { + Id = originalStartedBy.Id.Value, + }; + databaseContext.Users.Attach(job.StartedBy); databaseContext.Jobs.Add(job); await databaseContext.Save(cancellationToken); - - logger.LogDebug("Registering job {jobId}: {jobDesc}...", job.Id, job.Description); - var jobHandler = new JobHandler(jobCancellationToken => RunJob(job, operation, jobCancellationToken)); - try - { - lock (addCancelLock) - { - bool jobShouldStart; - lock (synchronizationLock) - { - jobs.Add(job.Id.Value, jobHandler); - jobShouldStart = !noMoreJobsShouldStart; - } - - if (jobShouldStart) - jobHandler.Start(); - } - } - catch - { - jobHandler.Dispose(); - throw; - } }); + job.StartedBy = originalStartedBy; + + logger.LogDebug("Registering job {jobId}: {jobDesc}...", job.Id, job.Description); + var jobHandler = new JobHandler(jobCancellationToken => RunJob(job, operation, jobCancellationToken)); + try + { + lock (addCancelLock) + { + bool jobShouldStart; + lock (synchronizationLock) + { + jobs.Add(job.Id.Value, jobHandler); + jobShouldStart = !noMoreJobsShouldStart; + } + + if (jobShouldStart) + jobHandler.Start(); + } + } + catch + { + jobHandler.Dispose(); + throw; + } + } + /// public Task StartAsync(CancellationToken cancellationToken) => databaseContextFactory.UseContext(async databaseContext => @@ -235,11 +256,22 @@ namespace Tgstation.Server.Host.Jobs await databaseContextFactory.UseContext(async databaseContext => { - user ??= await databaseContext.Users.GetTgsUser(cancellationToken); - var updatedJob = new Job(job.Id.Value); databaseContext.Jobs.Attach(updatedJob); - var attachedUser = new User { Id = user.Id }; + var attachedUser = user == null + ? await databaseContext + .Users + .GetTgsUser( + dbUser => new User + { + Id = dbUser.Id.Value, + }, + cancellationToken) + : new User + { + Id = user.Id.Value, + }; + databaseContext.Users.Attach(attachedUser); updatedJob.CancelledBy = attachedUser; @@ -465,7 +497,9 @@ namespace Tgstation.Server.Host.Jobs // Resetting the context here because I CBA to worry if the cache is being used await databaseContextFactory.UseContext(async databaseContext => { - // Cancellation might be set in another async context, forced to reload here for the final hub update + // Cancellation might be set in another async context + // Also, startedby could have been renamed + // forced to reload here for the final hub update // DCT: Cancellation token is for job, operation should always run var finalJob = await databaseContext .Jobs diff --git a/tests/Tgstation.Server.Tests/Live/Instance/JobsHubTests.cs b/tests/Tgstation.Server.Tests/Live/Instance/JobsHubTests.cs index d097aae7eb..8875a9b5ff 100644 --- a/tests/Tgstation.Server.Tests/Live/Instance/JobsHubTests.cs +++ b/tests/Tgstation.Server.Tests/Live/Instance/JobsHubTests.cs @@ -53,6 +53,7 @@ namespace Tgstation.Server.Tests.Live.Instance Assert.IsTrue(job.InstanceId.HasValue); Assert.IsNotNull(job.StartedBy); Assert.IsTrue(job.StartedBy.Id.HasValue); + Assert.IsNotNull(job.StartedBy.Name); Assert.IsTrue(job.StartedAt.HasValue); Assert.IsNotNull(job.Description); diff --git a/tests/Tgstation.Server.Tests/Live/Instance/JobsRequiredTest.cs b/tests/Tgstation.Server.Tests/Live/Instance/JobsRequiredTest.cs index 21a8c873d3..bfe8c98d82 100644 --- a/tests/Tgstation.Server.Tests/Live/Instance/JobsRequiredTest.cs +++ b/tests/Tgstation.Server.Tests/Live/Instance/JobsRequiredTest.cs @@ -1,11 +1,14 @@ using System; +using System.Reflection; using System.Threading; using System.Threading.Tasks; using Microsoft.VisualStudio.TestTools.UnitTesting; +using Tgstation.Server.Api.Hubs; using Tgstation.Server.Api.Models; using Tgstation.Server.Api.Models.Response; +using Tgstation.Server.Client; using Tgstation.Server.Client.Components; namespace Tgstation.Server.Tests.Live.Instance @@ -14,30 +17,72 @@ namespace Tgstation.Server.Tests.Live.Instance { protected IJobsClient JobsClient { get; } + readonly IApiClient apiClient; + public JobsRequiredTest(IJobsClient jobsClient) { - JobsClient = jobsClient; + JobsClient = jobsClient ?? throw new ArgumentNullException(nameof(jobsClient)); + apiClient = (IApiClient)jobsClient.GetType().GetProperty("ApiClient", BindingFlags.NonPublic | BindingFlags.Instance).GetValue(jobsClient); + } + + class JobReceiver : IJobsHub + { + public Action Callback { get; set; } + + public Task ReceiveJobUpdate(JobResponse job, CancellationToken cancellationToken) + { + Callback(job); + return Task.CompletedTask; + } } public async Task WaitForJob(JobResponse originalJob, int timeout, bool? expectFailure, ErrorCode? expectedCode, CancellationToken cancellationToken) { Assert.IsNotNull(originalJob.Id); Assert.IsNotNull(originalJob.JobCode); - var job = originalJob; - do - { - await Task.Delay(TimeSpan.FromSeconds(1), cancellationToken); - job = await JobsClient.GetId(job, cancellationToken); - Assert.IsNotNull(job.Id); - Assert.IsNotNull(job.JobCode); - --timeout; - } - while (!job.StoppedAt.HasValue && timeout > 0); + var job = originalJob; if (!job.StoppedAt.HasValue) { - await JobsClient.Cancel(job, cancellationToken); - Assert.Fail($"Job ID {job.Id} \"{job.Description}\" timed out!"); + var tcs = new TaskCompletionSource(); + var receiver = new JobReceiver + { + Callback = updatedJob => + { + if (updatedJob.Id != job.Id) + return; + + job = updatedJob; + if (updatedJob.StoppedAt.HasValue) + tcs.TrySetResult(); + }, + }; + + JobResponse firstCheck; + await using (var hubConnection = await apiClient.CreateHubConnection(receiver, null, null, cancellationToken)) + { + // initial GET after connecting + firstCheck = await JobsClient.GetId(job, cancellationToken); + if (!firstCheck.StoppedAt.HasValue) + { + firstCheck = null; + await Task.WhenAny( + tcs.Task, + Task.Delay(TimeSpan.FromSeconds(timeout), cancellationToken)); + } + } + + if (firstCheck != null) + job = firstCheck; + else if (!job.StoppedAt.HasValue) + // one last get in case SignalR dropped the ball + job = await JobsClient.GetId(job, cancellationToken); + + if (!job.StoppedAt.HasValue) + { + await JobsClient.Cancel(job, cancellationToken); + Assert.Fail($"Job ID {job.Id} \"{job.Description}\" timed out!"); + } } if (expectFailure.HasValue && expectFailure.Value ^ job.ExceptionDetails != null) diff --git a/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs b/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs index 8dfe8058cd..dfe2b6028b 100644 --- a/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs +++ b/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs @@ -1698,6 +1698,8 @@ namespace Tgstation.Server.Tests.Live await new ChatTest(instanceClient.ChatBots, adminClient.Instances, instanceClient.Jobs, instance).RunPostTest(cancellationToken); await repoTest; + await DummyChatProvider.RandomDisconnections(false, cancellationToken); + jobsHubTest.CompleteNow(); await jobsHubTestTask;