Merge branch 'V6' into OpenDream

.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

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
 	{
 		/// <summary>
-		/// Gets the unattached, unpopulated <see cref="User"/> with the name <see cref="User.TgsSystemUserName"/>.
+		/// Gets <see cref="User"/> with the name <see cref="User.TgsSystemUserName"/>.
 		/// </summary>
+		/// <typeparam name="TResult">The transformed return <see cref="Type"/>.</typeparam>
 		/// <param name="databaseCollection">The <see cref="IDatabaseCollection{TModel}"/> of <see cref="User"/>s to operate on.</param>
+		/// <param name="selector">A selecting <see cref="Expression"/> for transforming the returned <see cref="User"/> into the <typeparamref name="TResult"/>.</param>
 		/// <param name="cancellationToken">The <see cref="CancellationToken"/> for the operation.</param>
 		/// <returns>A <see cref="Task{TResult}"/> resulting in the unattached TGS <see cref="User"/> on success, <see langword="null"/> on failure.</returns>
-		public static Task<User> GetTgsUser(this IDatabaseCollection<User> databaseCollection, CancellationToken cancellationToken)
-			=> databaseCollection
+		public static Task<TResult> GetTgsUser<TResult>(
+			this IDatabaseCollection<User> databaseCollection,
+			Expression<Func<User, TResult>> 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);
+		}
 	}
 }

src/Tgstation.Server.Host/Jobs/JobService.cs

@@ -114,61 +114,82 @@ namespace Tgstation.Server.Host.Jobs
 		}
 
 		/// <inheritdoc />
-		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;
+			}
+		}
+
 		/// <inheritdoc />
 		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

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);
 

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<JobResponse> Callback { get; set; }
+
+			public Task ReceiveJobUpdate(JobResponse job, CancellationToken cancellationToken)
+			{
+				Callback(job);
+				return Task.CompletedTask;
+			}
 		}
 
 		public async Task<JobResponse> 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<IJobsHub>(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)

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;