Added CI build step to install ffprobe and ffmpeg before running tests.

Also cleaned up some outdated comments and did minor refactoring.
sillsdev · Nov 16, 2024 · 2146d15 · 2146d15
1 parent dba4714
commit 2146d15
Show file tree

Hide file tree

Showing 6 changed files with 48 additions and 54 deletions.
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
@@ -34,6 +34,18 @@ 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: actions/checkout@v3
+      uses: FedericoCarboni/setup-ffmpeg@v3
+      id: setup-ffmpeg
+      with:
+      # A specific version to download, may also be "release" or a specific version
+      # like "6.1.0". At the moment semver specifiers (i.e. >=6.1.0) are supported
+      # only on Windows, on other platforms they are allowed but version is matched
+      # exactly regardless.
+      ffmpeg-version: release
+    - run: ffmpeg -i input.avi output.mkv
 
     # 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

diff --git a/SIL.Media.Tests/AudioPlayerTests.cs b/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()

diff --git a/SIL.Media.Tests/AudioSessionTests.cs b/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());

diff --git a/SIL.Media.Tests/FFmpegRunnerTests.cs b/SIL.Media.Tests/FFmpegRunnerTests.cs
@@ -9,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"))
@@ -46,7 +47,6 @@ public void ExtractMp3Audio_CreatesFile()
 		}
 
 		[Test]
-		[Category("RequiresFfmpeg")]
 		public void ExtractOggAudio_CreatesFile()
 		{
 			using (var file = TempFile.FromResource(Resources.tiny, ".wmv"))
@@ -58,7 +58,6 @@ public void ExtractOggAudio_CreatesFile()
 		}
 
 		[Test]
-		[Category("RequiresFfmpeg")]
 		public void ChangeNumberOfAudioChannels_CreatesFile()
 		{
 			using (var file = TempFile.FromResource(Resources._2Channel, ".wav"))
@@ -70,7 +69,6 @@ public void ChangeNumberOfAudioChannels_CreatesFile()
 		}
 
 		[Test]
-		[Category("RequiresFfmpeg")]
 		public void MakeLowQualityCompressedAudio_CreatesFile()
 		{
 			using (var file = TempFile.FromResource(Resources.tiny, ".wmv"))
@@ -94,8 +92,6 @@ public void MakeLowQualityCompressedAudio_CreatesFile()
 				Assert.That(mediaInfo.PrimaryAudioStream.SampleRateHz, Is.EqualTo(8000));
 				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)
@@ -106,7 +102,6 @@ public void MakeLowQualityCompressedAudio_CreatesFile()
 		}
 
 		[Test]
-		[Category("RequiresFfmpeg")]
 		public void MakeLowQualitySmallVideo_CreatesFile()
 		{
 			using (var file = TempFile.FromResource(Resources.tiny, ".wmv"))

diff --git a/SIL.Media.Tests/MediaInfoTests.cs b/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);
 			}
 		}
 

diff --git a/SIL.Media/Naudio/AudioPlayer.cs b/SIL.Media/Naudio/AudioPlayer.cs
@@ -1,5 +1,6 @@
-using System;
+using System;
 using System.Timers;
+using JetBrains.Annotations;
 using NAudio.Wave;
 
 namespace SIL.Media.Naudio
@@ -26,6 +27,7 @@ public AudioPlayer()
 			};
 		}
 
+		[PublicAPI]
 		public void LoadStream(WaveStream stream)
 		{
 			InternalLoad(stream as TrimWaveStream ?? new TrimWaveStream(stream));
@@ -82,22 +84,19 @@ public void Stop()
 
 		public TimeSpan StartPosition
 		{
-			get { return _inStream.StartPosition; }
-			set { _inStream.StartPosition = value; }
+			get => _inStream.StartPosition;
+			set => _inStream.StartPosition = value;
 		}
 
 		public TimeSpan EndPosition
 		{
-			get { return _inStream.EndPosition; }
-			set { _inStream.EndPosition = value; }
+			get => _inStream.EndPosition;
+			set => _inStream.EndPosition = value;
 		}
 
 		public TimeSpan CurrentPosition { get; set; }
 
-		public PlaybackState PlaybackState
-		{
-			get { return _waveOut == null ? PlaybackState.Stopped : _waveOut.PlaybackState; }
-		}
+		public PlaybackState PlaybackState => _waveOut?.PlaybackState ?? PlaybackState.Stopped;
 
 		public void Dispose()
 		{