From 1df0c03f9b9a76d1ba04d0cf49d3b25769e7a1d8 Mon Sep 17 00:00:00 2001 From: tombogle Date: Sat, 16 Nov 2024 14:13:42 -0600 Subject: [PATCH] Added CI build step to install ffprobe and ffmpeg before running tests. Also cleaned up some outdated comments and did minor refactoring. --- .github/workflows/build.yml | 11 +++++++++++ SIL.Media.Tests/AudioPlayerTests.cs | 10 ++++------ SIL.Media.Tests/AudioSessionTests.cs | 9 +++------ SIL.Media.Tests/FFmpegRunnerTests.cs | 25 ++++++++++-------------- SIL.Media.Tests/MediaInfoTests.cs | 29 +++++++++++----------------- SIL.Media/Naudio/AudioPlayer.cs | 17 ++++++++-------- 6 files changed, 47 insertions(+), 54 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index fa3399d39..02b30d489 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -34,6 +34,17 @@ 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: + # 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 index 87a40a40c..3ed98953a 100644 --- a/SIL.Media.Tests/AudioPlayerTests.cs +++ b/SIL.Media.Tests/AudioPlayerTests.cs @@ -10,11 +10,9 @@ namespace SIL.Media.Tests { /// - /// 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. /// [Category("SkipOnTeamCity")] [TestFixture] @@ -33,7 +31,7 @@ public void LoadFile_ThenDispose_FileCanBeDeleted() } /// - /// 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. /// [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 index ea557bbff..debdeb25f 100644 --- 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 index 7eed32dfd..1efd967b0 100644 --- a/SIL.Media.Tests/FFmpegRunnerTests.cs +++ b/SIL.Media.Tests/FFmpegRunnerTests.cs @@ -9,13 +9,11 @@ namespace SIL.Media.Tests { /// - /// 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. /// - [Category("SkipOnTeamCity")] + [Category("RequiresFfmpeg")] [TestFixture] public class FFmpegRunnerTests { @@ -23,18 +21,21 @@ public class FFmpegRunnerTests 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 index a9f8a608b..0ed373f63 100644 --- a/SIL.Media.Tests/MediaInfoTests.cs +++ b/SIL.Media.Tests/MediaInfoTests.cs @@ -1,3 +1,4 @@ +using System; using NUnit.Framework; using SIL.IO; using SIL.Media.Tests.Properties; @@ -5,13 +6,12 @@ namespace SIL.Media.Tests { /// - /// 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. /// [Category("SkipOnTeamCity")] + [Category("RequiresFFprobe")] [TestFixture] public class MediaInfoTests { @@ -19,18 +19,21 @@ public class MediaInfoTests 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 index 46cdf41c1..c6d325b75 100644 --- 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() {