Improved unit tests for FFmpegRunner

.github/workflows/build.yml

@@ -34,6 +34,12 @@ jobs:
 
     - name: Get Path to Tests
       run: echo "TEST_PATH=$(dotnet msbuild SIL.Core.Tests/ --getProperty:OutputPath -p:TargetFramework=${{ matrix.framework }} -p:Configuration=Release)" >> $GITHUB_ENV
+
+    - name: Install FFmpeg
+      uses: FedericoCarboni/setup-ffmpeg@v3
+      id: setup-ffmpeg
+      with:
+        ffmpeg-version: release
 
     # there are cases where this will fail and we want to know about it
     # so we don't use continue-on-error, but we still want to publish the results

SIL.Media.Tests/AudioPlayerTests.cs

@@ -10,11 +10,9 @@
 namespace SIL.Media.Tests
 {
 	/// <summary>
-	/// All these tests are skipped on TeamCity (even if you remove this category) because SIL.Media.Tests compiles to an exe,
-	/// and the project that builds libpalaso on TeamCity (build/Palaso.proj, task Test) invokes RunNUnitTC which
-	/// selects the test assemblies using Include="$(RootDir)/output/$(Configuration)/*.Tests.dll" which excludes exes.
-	/// I have not tried to verify that all of these tests would actually have problems on TeamCity, but it seemed
-	/// helpful to document in the usual way that they are not, in fact, run there. 
+	/// SkipOnTeamCity does not affect CI build using GHA, but I'm keeping it here for posterity's sake or if we ever
+	/// need to go back to building on TC. The comment here used to say that this test fixture compiled to an exe so
+	/// the tests would be skipped on TC anyway, but that is no longer true.
 	/// </summary>
 	[Category("SkipOnTeamCity")]
 	[TestFixture]
@@ -33,7 +31,7 @@ public void LoadFile_ThenDispose_FileCanBeDeleted()
 		}
 
 		/// <summary>
-		/// This test shows what caused hearthis to abandon the naudio; previous to a change to this class in 2/2012, it is believed that all was ok.
+		/// This test shows what caused HearThis to abandon NAudio; previous to a change to this class in 2/2012, it is believed that all was ok.
 		/// </summary>
 		[Test, Ignore("Known to Fail (hangs forever")]
 		public void PlayFile_ThenDispose_FileCanBeDeleted()

SIL.Media.Tests/AudioSessionTests.cs

@@ -265,10 +265,7 @@ public RecordingSession(int millisecondsToRecordBeforeStopping)
 				_recorder.StopRecordingAndSaveAsWav();
 			}
 
-			public ISimpleAudioSession Recorder
-			{
-				get { return _recorder; }
-			}
+			public ISimpleAudioSession Recorder => _recorder;
 
 			public void Dispose()
 			{
@@ -451,8 +448,8 @@ public void Record_LongRecording()
 		{
 			using (var folder = new TemporaryFolder("Record_LongRecording"))
 			{
-				string fpath = Path.Combine(folder.Path, "long.wav");
-				using (var x = AudioFactory.CreateAudioSession(fpath))
+				string fPath = Path.Combine(folder.Path, "long.wav");
+				using (var x = AudioFactory.CreateAudioSession(fPath))
 				{
 					SystemSounds.Beep.Play();
 					Assert.DoesNotThrow(() => x.StartRecording());

SIL.Media.Tests/FFmpegRunnerTests.cs

@@ -1,4 +1,6 @@
+using System;
 using System.IO;
+using FFMpegCore;
 using NUnit.Framework;
 using SIL.IO;
 using SIL.Media.Tests.Properties;
@@ -7,32 +9,33 @@
 namespace SIL.Media.Tests
 {
 	/// <summary>
-	/// All these tests are skipped on TeamCity (even if you remove this category) because SIL.Media.Tests compiles to an exe,
-	/// and the project that builds libpalaso on TeamCity (build/Palaso.proj, task Test) invokes RunNUnitTC which
-	/// selects the test assemblies using Include="$(RootDir)/output/$(Configuration)/*.Tests.dll" which excludes exes.
-	/// I have not tried to verify that all of these tests would actually have problems on TeamCity, but it seemed
-	/// helpful to document in the usual way that they are not, in fact, run there. 
+	/// SkipOnTeamCity does not affect CI build using GHA, but I'm keeping it here for posterity's sake or if we ever
+	/// need to go back to building on TC. The comment here used to say that this test fixture compiled to an exe so
+	/// the tests would be skipped on TC anyway, but that is no longer true.
 	/// </summary>
-	[Category("SkipOnTeamCity")]
+	[Category("RequiresFfmpeg")]
 	[TestFixture]
 	public class FFmpegRunnerTests
 	{
 		[OneTimeSetUp]
 		public void CheckRequirements()
 		{
 			if (!FFmpegRunner.HaveNecessaryComponents)
-				Assert.Ignore("These tests require ffmpeg to be installed.");
+			{
+				if (Environment.GetEnvironmentVariable("CI") == null)
+					Assert.Ignore("These tests require ffmpeg to be installed.");
+				else
+					Assert.Fail("On CI build using GHA, FFMpeg should have been installed before running tests.");
+			}
 		}
 
 		[Test]
-		[Category("RequiresFfmpeg")]
 		public void HaveNecessaryComponents_ReturnsTrue()
 		{
 			Assert.IsTrue(FFmpegRunner.HaveNecessaryComponents);
 		}
 
 		[Test]
-		[Category("RequiresFfmpeg")]
 		public void ExtractMp3Audio_CreatesFile()
 		{
 			using (var file = TempFile.FromResource(Resources.tiny, ".wmv"))
@@ -44,7 +47,6 @@ public void ExtractMp3Audio_CreatesFile()
 		}
 
 		[Test]
-		[Category("RequiresFfmpeg")]
 		public void ExtractOggAudio_CreatesFile()
 		{
 			using (var file = TempFile.FromResource(Resources.tiny, ".wmv"))
@@ -56,7 +58,6 @@ public void ExtractOggAudio_CreatesFile()
 		}
 
 		[Test]
-		[Category("RequiresFfmpeg")]
 		public void ChangeNumberOfAudioChannels_CreatesFile()
 		{
 			using (var file = TempFile.FromResource(Resources._2Channel, ".wav"))
@@ -68,7 +69,6 @@ public void ChangeNumberOfAudioChannels_CreatesFile()
 		}
 
 		[Test]
-		[Category("RequiresFfmpeg")]
 		public void MakeLowQualityCompressedAudio_CreatesFile()
 		{
 			using (var file = TempFile.FromResource(Resources.tiny, ".wmv"))
@@ -79,20 +79,58 @@ public void MakeLowQualityCompressedAudio_CreatesFile()
 				var outputPath = originalAudioPath.Replace("mp3", "low.mp3");
 				FFmpegRunner.MakeLowQualityCompressedAudio(originalAudioPath, outputPath, new ConsoleProgress());
 				Assert.IsTrue(File.Exists(outputPath));
-				System.Diagnostics.Process.Start(outputPath);
+
+				var mediaInfoOrig = FFProbe.Analyse(file.Path);
+				var mediaInfo = FFProbe.Analyse(outputPath);
+
+				// Validate resolution and bit rate
+				Assert.That(mediaInfo.PrimaryVideoStream, Is.Null);
+				Assert.That(mediaInfo.PrimaryAudioStream, Is.Not.Null);
+				Assert.That(mediaInfo.AudioStreams.Count, Is.EqualTo(1));
+				Assert.That(mediaInfo.PrimaryAudioStream.Channels, Is.EqualTo(1));
+				Assert.That(mediaInfo.Format.BitRate, Is.LessThan(mediaInfoOrig.Format.BitRate));
+				Assert.That(mediaInfo.PrimaryAudioStream.SampleRateHz, Is.EqualTo(8000));
+				try
+				{
+					RobustFile.Delete(outputPath);
+				}
+				catch (Exception e)
+				{
+					Console.WriteLine(e);
+				}
 			}
 		}
 
 		[Test]
-		[Category("RequiresFfmpeg")]
 		public void MakeLowQualitySmallVideo_CreatesFile()
 		{
 			using (var file = TempFile.FromResource(Resources.tiny, ".wmv"))
 			{
 				var outputPath = file.Path.Replace("wmv", "low.wmv");
 				FFmpegRunner.MakeLowQualitySmallVideo(file.Path, outputPath, 0, new ConsoleProgress());
 				Assert.IsTrue(File.Exists(outputPath));
-				System.Diagnostics.Process.Start(outputPath);
+
+				var mediaInfoOrig = FFProbe.Analyse(file.Path);
+				var mediaInfo = FFProbe.Analyse(outputPath);
+
+				// Validate resolution and bit rate
+				Assert.That(mediaInfo.PrimaryVideoStream, Is.Not.Null);
+				Assert.That(mediaInfo.VideoStreams.Count, Is.EqualTo(1));
+				Assert.That(mediaInfo.PrimaryAudioStream, Is.Not.Null);
+				Assert.That(mediaInfo.AudioStreams.Count, Is.EqualTo(1));
+				Assert.That(mediaInfo.PrimaryVideoStream.Width, Is.EqualTo(160));
+				Assert.That(mediaInfo.PrimaryVideoStream.Height, Is.EqualTo(120));
+				Assert.That(mediaInfo.Format.BitRate, Is.LessThan(mediaInfoOrig.Format.BitRate));
+				try
+				{
+					// When running the by-hand test, the default media player might leave this
+					// locked, so this cleanup will fail.
+					RobustFile.Delete(outputPath);
+				}
+				catch (Exception e)
+				{
+					Console.WriteLine(e);
+				}
 			}
 		}
 	}

SIL.Media.Tests/MediaInfoTests.cs

@@ -1,36 +1,39 @@
+using System;
 using NUnit.Framework;
 using SIL.IO;
 using SIL.Media.Tests.Properties;
 
 namespace SIL.Media.Tests
 {
 	/// <summary>
-	/// All these tests are skipped on TeamCity (even if you remove this category) because SIL.Media.Tests compiles to an exe,
-	/// and the project that builds libpalaso on TeamCity (build/Palaso.proj, task Test) invokes RunNUnitTC which
-	/// selects the test assemblies using Include="$(RootDir)/output/$(Configuration)/*.Tests.dll" which excludes exes.
-	/// I have not tried to verify that all of these tests would actually have problems on TeamCity, but it seemed
-	/// helpful to document in the usual way that they are not, in fact, run there.
+	/// SkipOnTeamCity does not affect CI build using GHA, but I'm keeping it here for posterity's sake or if we ever
+	/// need to go back to building on TC. The comment here used to say that this test fixture compiled to an exe so
+	/// the tests would be skipped on TC anyway, but that is no longer true.
 	/// </summary>
 	[Category("SkipOnTeamCity")]
+	[Category("RequiresFFprobe")]
 	[TestFixture]
 	public class MediaInfoTests
 	{
 		[OneTimeSetUp]
 		public void CheckRequirements()
 		{
 			if (!MediaInfo.HaveNecessaryComponents)
-				Assert.Ignore(MediaInfo.MissingComponentMessage);
+			{
+				if (Environment.GetEnvironmentVariable("CI") == null)
+					Assert.Ignore(MediaInfo.MissingComponentMessage);
+				else
+					Assert.Fail("On CI build using GHA, FFMpeg should have been installed before running tests.");
+			}
 		}
 
 		[Test]
-		[Category("RequiresFFprobe")]
 		public void HaveNecessaryComponents_ReturnsTrue()
 		{
 			Assert.IsTrue(MediaInfo.HaveNecessaryComponents);
 		}
 
 		[Test]
-		[Category("RequiresFFprobe")]
 		public void VideoInfo_Duration_Correct()
 		{
 			using (var file = TempFile.FromResource(Resources.tiny, ".wmv"))
@@ -43,7 +46,6 @@ public void VideoInfo_Duration_Correct()
 		}
 
 		[Test]
-		[Category("RequiresFFprobe")]
 		public void VideoInfo_Encoding_Correct()
 		{
 			using (var file = TempFile.FromResource(Resources.tiny, ".wmv"))
@@ -54,7 +56,6 @@ public void VideoInfo_Encoding_Correct()
 		}
 
 		[Test]
-		[Category("RequiresFFprobe")]
 		public void VideoInfo_Resolution_Correct()
 		{
 			using (var file = TempFile.FromResource(Resources.tiny, ".wmv"))
@@ -65,7 +66,6 @@ public void VideoInfo_Resolution_Correct()
 		}
 
 		[Test]
-		[Category("RequiresFFprobe")]
 		public void VideoInfo_FramesPerSecond_Correct()
 		{
 			using (var file = TempFile.FromResource(Resources.tiny, ".wmv"))
@@ -77,7 +77,6 @@ public void VideoInfo_FramesPerSecond_Correct()
 		}
 
 		[Test]
-		[Category("RequiresFFprobe")]
 		public void AudioInfo_Duration_Correct()
 		{
 			using (var file = TempFile.FromResource(Resources.finished, ".wav"))
@@ -89,7 +88,6 @@ public void AudioInfo_Duration_Correct()
 
 
 		[Test]
-		[Category("RequiresFFprobe")]
 		public void AudioInfo_SampleFrequency_Correct()
 		{
 			using (var file = TempFile.FromResource(Resources.finished, ".wav"))
@@ -100,7 +98,6 @@ public void AudioInfo_SampleFrequency_Correct()
 		}
 
 		[Test]
-		[Category("RequiresFFprobe")]
 		public void AudioInfo_Channels_Correct()
 		{
 			using (var file = TempFile.FromResource(Resources.finished, ".wav"))
@@ -111,7 +108,6 @@ public void AudioInfo_Channels_Correct()
 		}
 
 		[Test]
-		[Category("RequiresFFprobe")]
 		public void AudioInfo_BitDepth_Correct()
 		{
 			using (var file = TempFile.FromResource(Resources.finished, ".wav"))
@@ -122,7 +118,6 @@ public void AudioInfo_BitDepth_Correct()
 		}
 
 		[Test]
-		[Category("RequiresFFprobe")]
 		public void AudioInfo_H4N24BitStereoBitDepth_Correct()
 		{
 			using (var file = TempFile.FromResource(Resources._24bitH4NSample, ".wav"))
@@ -133,14 +128,12 @@ public void AudioInfo_H4N24BitStereoBitDepth_Correct()
 		}
 
 		[Test]
-		[Category("RequiresFFprobe")]
 		public void GetMediaInfo_AudioFile_VideoInfoAndImageInfoAreNull()
 		{
 			using (var file = TempFile.FromResource(Resources.finished,".wav"))
 			{
 				var info =MediaInfo.GetInfo(file.Path);
 				Assert.IsNull(info.Video);
-				//Assert.IsNull(info.Image);
 			}
 		}
 

SIL.Media/FFmpegRunner.cs

@@ -168,7 +168,7 @@ public static ExecutionResult ExtractMp3Audio(string inputPath, string outputPat
 			var arguments = $"-i \"{inputPath}\" -vn {mp3LameCodecArg} -ac {channels} \"{outputPath}\"";
 			var result = RunFFmpeg(arguments, progress);
 
-			//hide a meaningless error produced by some versions of liblame
+			// Hide a meaningless error produced by some versions of liblame
 			if (result.StandardError.Contains("lame: output buffer too small")
 				&& File.Exists(outputPath))
 			{
@@ -203,7 +203,7 @@ public static ExecutionResult ExtractOggAudio(string inputPath, string outputPat
 			var arguments = $"-i \"{inputPath}\" -vn -acodec vorbis -ac {channels} \"{outputPath}\"";
 			var result = RunFFmpeg(arguments, progress);
 
-			//hide a meaningless error produced by some versions of liblame
+			// Hide a meaningless error produced by some versions of liblame
 			if (result.StandardError.Contains("lame: output buffer too small")
 				&& File.Exists(outputPath))
 			{
@@ -294,7 +294,7 @@ private static ExecutionResult ExtractAudio(string inputPath, string outputPath,
 
 			var result = RunFFmpeg(arguments, progress);
 
-			//hide a meaningless error produced by some versions of liblame
+			// Hide a meaningless error produced by some versions of liblame
 			if (result.StandardError.Contains("lame: output buffer too small")
 				&& File.Exists(outputPath))
 			{
@@ -328,7 +328,7 @@ public static ExecutionResult ChangeNumberOfAudioChannels(string inputPath,
 
 			var result = RunFFmpeg(arguments, progress);
 
-			//hide a meaningless error produced by some versions of liblame
+			// Hide a meaningless error produced by some versions of liblame
 			if (result.StandardError.Contains("lame: output buffer too small") && File.Exists(outputPath))
 			{
 				var doctoredResult = new ExecutionResult
@@ -361,7 +361,7 @@ public static ExecutionResult MakeLowQualityCompressedAudio(string inputPath, st
 
 			var result = RunFFmpeg(arguments, progress);
 
-			//hide a meaningless error produced by some versions of liblame
+			// Hide a meaningless error produced by some versions of liblame
 			if (result.StandardError.Contains("lame: output buffer too small")
 				&& File.Exists(outputPath))
 			{
@@ -404,7 +404,7 @@ public static ExecutionResult MakeLowQualitySmallVideo(string inputPath, string
 
 			var result = RunFFmpeg(arguments, progress);
 
-			//hide a meaningless error produced by some versions of liblame
+			// Hide a meaningless error produced by some versions of liblame
 			if (result.StandardError.Contains("lame: output buffer too small")
 				&& File.Exists(outputPath))
 			{