Added specific NUnit cataegories for RequiresAudioOutputDevice and Re…

…quiresAudioInputDevice to make it clear what requirement might not be met when tests fail.

In FFmpegRunner, changed methods to use LocateAndRememberFFmpeg instead of LocateFFmpeg.
Made the Windows implementation of ISimpleAudioSession attempt to create an irrKlang-based recorder even if there is no audio output device enabled.
Improved some of the AudioSession unit tests to not require an audio input device if they are only testing playback.
Made AudioRecorderTests fail at the fixture level if an audio input device is not present rather than throwing an obscure exception inside OnHandleCrated.

.github/workflows/build.yml

@@ -43,7 +43,7 @@ jobs:
         ffmpeg-version: release
     - run: echo ffmpeg path ${{ steps.setup-ffmpeg.outputs.ffmpeg-path }}
 
-    - name: Install Scream on Windows
+    - name: Install Scream
       shell: powershell
       run: |
         Invoke-WebRequest https://github.com/duncanthrax/scream/releases/download/4.0/Scream4.0.zip -OutFile Scream4.0.zip
@@ -54,8 +54,7 @@ jobs:
     - name: Setup MSVC Dev Cmd
       uses: ilammy/msvc-dev-cmd@v1
 
-    - name: Sign and Install Scream Driver on Windows
-      if: matrix.os == 'windows-latest'
+    - name: Sign and Install Scream Driver
       shell: powershell
       run: |
         signtool sign /v /fd SHA256 /f ScreamCertificate.pfx Scream\Install\driver\x64\Scream.cat

CHANGELOG.md

@@ -81,6 +81,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - [SIL.Core.Desktop] Renamed GetFromRegistryProgramThatOpensFileType to GetDefaultProgramForFileType.
 - [SIL.Media] Made FFmpegRunner able to use version of FFmpeg found on the path.
 - [SIL.Media] Upgraded irrKlang to v. 1.6.
+- [SIL.Media] In FFmpegRunner, changed ExtractMp3Audio, ExtractOggAudio, ExtractAudio, and ChangeNumberOfAudioChannels to use LocateAndRememberFFmpeg instead of LocateFFmpeg. This is potentially a breaking change but only in the edge case where an app does not install FFmpeg and the user installs it while running the app.
+- [SIL.Media] Made the Windows implementation of ISimpleAudioSession more robust in that it will attempt to create an irrKlang-based recorder even if there is no audio output device enabled.
 
 ### Fixed
 - [SIL.Archiving] Fixed typo in RampArchivingDlgViewModel for Ethnomusicology performance collection.

SIL.Core/IO/TempFile.cs

@@ -267,7 +267,7 @@ public static TempFile WithFilenameInTempFolder(string fileName)
 		/// Used to make a real file out of a resource for the purpose of testing
 		/// </summary>
 		/// <param name="resource">e.g., an audio resource</param>
-		/// <param name="extension">with or with out '.', will work the same</param>
+		/// <param name="extension">with or without '.', will work the same</param>
 		public static TempFile FromResource(Stream resource, string extension)
 		{
 			var f = WithExtension(extension);

SIL.Media.Tests/AudioFactoryTests.cs

@@ -4,9 +4,9 @@
 
 namespace SIL.Media.Tests
 {
-	// These will not work if a speaker is not available.
 	[TestFixture]
 	[Category("AudioTests")]
+	[Category("RequiresAudioOutputDevice")] // These will not work if a speaker is not available.
 	public class AudioFactoryTests
 	{
 		[Test]
@@ -26,7 +26,6 @@ public void Construct_FileDoesExistButEmpty_OK()
 			}
 		}
 
-
 		[Test]
 		public void Construct_FileDoesNotExist_DoesNotCreateFile()
 		{

SIL.Media.Tests/AudioRecorderTests.cs

@@ -8,21 +8,29 @@
 
 namespace SIL.Media.Tests
 {
-	// Some of these tests require a speaker. Others require a microphone.
-	// None of them will work if neither a speaker nor a microphone is available.
 	[TestFixture]
 	[Category("AudioTests")]
+	[Category("RequiresAudioInputDevice")]
 	public class AudioRecorderTests
 	{
-
+		private RecordingDevice _defaultRecordingDevice;
+
+		[OneTimeSetUp]
+		public void SetUpFixture()
+		{
+			_defaultRecordingDevice = RecordingDevice.Devices.FirstOrDefault() as RecordingDevice;
+			Assert.That(_defaultRecordingDevice, Is.Not.Null,
+				"These tests require a microphone.");
+		}
+
 		[Test]
 		public void BeginRecording_OnNonUiThread_Throws()
 		{
 			using (var f = new TempFile())
 			{
 				using (var recorder = new AudioRecorder(1))
 				{
-					recorder.SelectedDevice = RecordingDevice.Devices.First() as RecordingDevice;
+					recorder.SelectedDevice = _defaultRecordingDevice;
 					Assert.That(() => recorder.BeginRecording(f.Path, false), Throws.Exception);
 				}
 			}
@@ -41,8 +49,7 @@ public void BeginRecording_ThenStop_RecordingSavedToFile()
 					// Note: BeginRecording
 					ctrl.HandleCreated += delegate
 					{
-						recorder.SelectedDevice =
-							RecordingDevice.Devices.First() as RecordingDevice;
+						recorder.SelectedDevice = _defaultRecordingDevice;
 						recorder.BeginRecording(f.Path, false);
 						Thread.Sleep(100);
 						recorder.Stop();
@@ -85,8 +92,7 @@ public void BeginRecording_WhileRecording_ThrowsInvalidOperationException()
 					// Note: BeginRecording
 					ctrl.HandleCreated += delegate
 					{
-						recorder.SelectedDevice =
-							RecordingDevice.Devices.First() as RecordingDevice;
+						recorder.SelectedDevice = _defaultRecordingDevice;
 						recorder.BeginRecording(f.Path, false);
 					};
 
@@ -116,8 +122,7 @@ public void BeginMonitoring_WhileRecording_ThrowsInvalidOperationException()
 					// Note: BeginRecording
 					ctrl.HandleCreated += delegate
 					{
-						recorder.SelectedDevice =
-							RecordingDevice.Devices.First() as RecordingDevice;
+						recorder.SelectedDevice = _defaultRecordingDevice;
 						recorder.BeginRecording(f.Path, false);
 					};
 
@@ -147,8 +152,7 @@ public void BeginRecording_OnNonUiThreadAfterMonitoringStartedOnUIThread_NoExcep
 				{
 					ctrl.HandleCreated += delegate
 					{
-						recorder.SelectedDevice =
-							RecordingDevice.Devices.First() as RecordingDevice;
+						recorder.SelectedDevice = _defaultRecordingDevice;
 						recorder.BeginMonitoring();
 						monitoringStarted = true;
 					};

SIL.Media.Tests/AudioSessionTests.cs

@@ -12,8 +12,7 @@
 
 namespace SIL.Media.Tests
 {
-	// Some of these tests require a speaker. Others require a microphone.
-	// None of them will work if neither a speaker nor a microphone is available.
+	// Some of these tests require a microphone (or a virtual audio input device).
 	[TestFixture]
 	[NUnit.Framework.Category("AudioTests")]
 	public class AudioSessionTests
@@ -27,17 +26,6 @@ public void StopRecordingAndSaveAsWav_NotRecording_Throws()
 			}
 		}
 
-		[Test]
-		public void StopPlaying_WhilePlaying_NoExceptionThrown()
-		{
-			using (var session = new RecordingSession(1000))
-			{
-				session.Recorder.Play();
-				Thread.Sleep(100);
-				session.Recorder.StopPlaying();
-			}
-		}
-
 		[Test]
 		public void StopRecordingAndSaveAsWav_WhileRecording_NoExceptionThrown()
 		{
@@ -50,6 +38,7 @@ public void StopRecordingAndSaveAsWav_WhileRecording_NoExceptionThrown()
 		}
 
 		[Test]
+		[NUnit.Framework.Category("RequiresAudioInputDevice")]
 		public void Play_WhileRecording_Throws()
 		{
 			using (var session = new RecordingSession())
@@ -116,19 +105,34 @@ public void Play_FileDoesNotExist_Throws()
 			}
 		}
 
+		/// <summary>
+		/// For reasons I don't entirely understand, this test will actually pass when run
+		/// by itself against a single target framework without an audio output device, but
+		/// to get it to pass when running as part of the fixture or when testing against both
+		/// frameworks, it is necessary to have an audio output device.
+		/// </summary>
 		[Test]
+		[NUnit.Framework.Category("RequiresAudioOutputDevice")]
 		public void CanStop_WhilePlaying_True()
 		{
-			using (var session = new RecordingSession(1000))
+			using (var file = TempFile.FromResource(Resources.finished, ".wav"))
 			{
-				session.Recorder.Play();
-				Thread.Sleep(100);
-				Assert.IsTrue(session.Recorder.CanStop);
+				using (var x = AudioFactory.CreateAudioSession(file.Path))
+				{
+					x.Play();
+					Thread.Sleep(200);
+					Assert.That(x.CanStop, Is.True, "Playback should last more than 200 ms.");
+					// We used to test this in a separate test:
+					// StopPlaying_WhilePlaying_NoExceptionThrown
+					// But now that would be redundant since we need to stop playback in order to
+					// safely dispose.
+					x.StopPlaying();
+				}
 			}
 		}
 
-
 		[Test]
+		[NUnit.Framework.Category("RequiresAudioInputDevice")]
 		public void RecordAndStop_FileAlreadyExists_FileReplaced()
 		{
 			using (var f = new TempFile())
@@ -150,6 +154,7 @@ public void RecordAndStop_FileAlreadyExists_FileReplaced()
 		}
 
 		[Test]
+		[NUnit.Framework.Category("RequiresAudioInputDevice")]
 		public void IsRecording_WhileRecording_True()
 		{
 			using (var f = new TempFile())
@@ -166,6 +171,7 @@ public void IsRecording_WhileRecording_True()
 
 		[Test]
 		[Platform(Exclude = "Linux", Reason = "AudioAlsaSession doesn't implement ISimpleAudioWithEvents")]
+		[NUnit.Framework.Category("RequiresAudioInputDevice")]
 		public void RecordThenPlay_SmokeTest()
 		{
 			using (var f = new TempFile())
@@ -209,37 +215,28 @@ public void RecordThenPlay_SmokeTest()
 		}
 
 		[Test]
-		public void Play_GiveThaiFileName_ShouldHearTwoSounds()
+		[NUnit.Framework.Category("RequiresAudioInputDevice")]
+		public void Play_GiveThaiFileName_ShouldHearTinklingSounds()
 		{
-			using (var d = new TemporaryFolder("palaso media test"))
+			using (var file = TempFile.FromResource(Resources.finished, ".wav"))
 			{
-				var soundPath = d.Combine("ก.wav");
-				File.Create(soundPath).Close();
-				using (var f = TempFile.TrackExisting(soundPath))
+				using (var d = new TemporaryFolder("palaso media test"))
 				{
-					var w = new BackgroundWorker();
-					// ReSharper disable once RedundantDelegateCreation
-					w.DoWork += new DoWorkEventHandler((o, args) => SystemSounds.Exclamation.Play());
-
-					using (var x = AudioFactory.CreateAudioSession(f.Path))
-					{
-						x.StartRecording();
-						w.RunWorkerAsync();
-						Thread.Sleep(2000);
-						x.StopRecordingAndSaveAsWav();
-					}
-
-					using (var y = AudioFactory.CreateAudioSession(f.Path))
+					var soundPath = d.Combine("ก.wav");
+					RobustFile.Copy(file.Path, soundPath);
+					using (var f = TempFile.TrackExisting(soundPath))
 					{
-						y.Play();
-						Thread.Sleep(1000);
-						y.StopPlaying();
+						using (var y = AudioFactory.CreateAudioSession(f.Path))
+						{
+							y.Play();
+							Thread.Sleep(1000);
+							y.StopPlaying();
+						}
 					}
 				}
 			}
 		}
 
-
 		/// <summary>
 		/// for testing things while recording is happening
 		/// </summary>
@@ -284,6 +281,7 @@ public void Dispose()
 		}
 
 		[Test]
+		[NUnit.Framework.Category("RequiresAudioInputDevice")]
 		public void CanStop_WhileRecording_True()
 		{
 			using (var session = new RecordingSession())
@@ -293,6 +291,7 @@ public void CanStop_WhileRecording_True()
 		}
 
 		[Test]
+		[NUnit.Framework.Category("RequiresAudioInputDevice")]
 		public void CanPlay_WhileRecording_False()
 		{
 			using (var session = new RecordingSession())
@@ -302,6 +301,7 @@ public void CanPlay_WhileRecording_False()
 		}
 
 		[Test]
+		[NUnit.Framework.Category("RequiresAudioInputDevice")]
 		public void CanRecord_WhileRecording_False()
 		{
 			using (var session = new RecordingSession())
@@ -319,26 +319,48 @@ public void StartRecording_WhileRecording_Throws()
 			}
 		}
 
+		/// <summary>
+		/// For reasons I don't entirely understand, this test will actually pass when run
+		/// by itself against a single target framework without an audio output device, but
+		/// to get it to pass when running as part of the fixture or when testing against both
+		/// frameworks, it is necessary to have an audio output device.
+		/// </summary>
 		[Test]
+		[NUnit.Framework.Category("RequiresAudioOutputDevice")]
+		[NUnit.Framework.Category("RequiresAudioInputDevice")]
 		public void CanRecord_WhilePlaying_False()
 		{
 			using (var session = new RecordingSession(1000))
 			{
 				session.Recorder.Play();
 				Thread.Sleep(100);
-				Assert.IsTrue(session.Recorder.IsPlaying);
-				Assert.IsFalse(session.Recorder.CanRecord);
+				Assert.That(session.Recorder.IsPlaying, Is.True,
+					"Should be playing, not recording.");
+				Assert.That(session.Recorder.CanRecord, Is.False);
 			}
 		}
 
+		/// <summary>
+		/// For reasons I don't entirely understand, this test will actually pass when run
+		/// by itself against a single target framework without an audio output device, but
+		/// to get it to pass when running as part of the fixture or when testing against both
+		/// frameworks, it is necessary to have an audio output device. I tried setting the
+		/// ParallelScope to None, but even that didn't work, so it does not seem to be an
+		/// issue with parallel test runs affecting each other.
+		/// </summary>
 		[Test]
+		[NUnit.Framework.Category("RequiresAudioOutputDevice")]
 		public void CanPlay_WhilePlaying_False()
 		{
-			using (var session = new RecordingSession(1000))
+			using (var file = TempFile.FromResource(Resources.finished, ".wav"))
 			{
-				session.Recorder.Play();
-				Thread.Sleep(100);
-				Assert.IsFalse(session.Recorder.CanPlay);
+				using (var x = AudioFactory.CreateAudioSession(file.Path))
+				{
+					x.Play();
+					Thread.Sleep(200);
+					Assert.That(x.CanPlay, Is.False, "Playback should last more than 200 ms.");
+					x.StopPlaying();
+				}
 			}
 		}
 
@@ -367,6 +389,7 @@ public void RecordThenStop_CanPlay_IsTrue()
 		}
 
 		[Test]
+		[NUnit.Framework.Category("RequiresAudioInputDevice")]
 		public void RecordThenPlay_OK()
 		{
 			using (var f = new TempFile())
@@ -425,6 +448,7 @@ public void Play_DoesPlayMp3_SmokeTest()
 		}
 
 		[Test]
+		[NUnit.Framework.Category("RequiresAudioInputDevice")]
 		public void Record_DoesRecord ()
 		{
 			using (var folder = new TemporaryFolder("Record_DoesRecord"))