From dca8c7a8e3ae6c1e4136358b810df3377136c526 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Helge=20Stenstr=C3=B6m?= Date: Sun, 22 Dec 2019 11:09:10 +0100 Subject: [PATCH] Improved and more complete unit tests. StreamPlayerMethodsTest still contains tests that don't pass. The tested methods are candidates for removal from StreamPlayer. All tests in StreamPlayerMethodsTest must be reviewed: Do they actually verify anything that need to be verified? Or are they too coupled with the current implementation? StreamPlayerFutureImprovementTest contains tests that currently fail. Failures are caused by behavior in StreamPlayer which I think is wrong or bad. But I may have misinterpreted the intended behavior. --- .../StreamPlayerFutureImprovementTest.java | 26 +++++++ .../stream/StreamPlayerMethodsTest.java | 71 ++++++++++++------- 2 files changed, 73 insertions(+), 24 deletions(-) diff --git a/src/test/java/com/goxr3plus/streamplayer/stream/StreamPlayerFutureImprovementTest.java b/src/test/java/com/goxr3plus/streamplayer/stream/StreamPlayerFutureImprovementTest.java index 807c4d9..4a6c452 100644 --- a/src/test/java/com/goxr3plus/streamplayer/stream/StreamPlayerFutureImprovementTest.java +++ b/src/test/java/com/goxr3plus/streamplayer/stream/StreamPlayerFutureImprovementTest.java @@ -2,6 +2,7 @@ import com.goxr3plus.streamplayer.enums.Status; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import javax.sound.sampled.AudioFileFormat; @@ -44,4 +45,29 @@ void addStreamPlayerListener_dontAcceptNull() { fail("Test not done"); } + + @Test + @DisplayName("When play() is called without first calling open(), an exception is thrown") + void playingUnopenedSourceThrowsException() { + + assertThrows(Exception.class, () -> player.play()); + } + + @Test + void seekBytes() throws StreamPlayerException { + player.open(audioFile); + player.play(); + int positionByte1 = player.getPositionByte(); + + player.seekBytes(100); + int positionByte2 = player.getPositionByte(); + + assertTrue( positionByte2 > positionByte1); + + // TODO: It seems that getPositionByte doesn't work. + // It isn't called from within this project, except for in this test. + // It is however called by XR3Player. If XR3Player needs this method, it must be tested + // within this project. The method relies on a map, which doesn't seem to be updated by play() + } + } diff --git a/src/test/java/com/goxr3plus/streamplayer/stream/StreamPlayerMethodsTest.java b/src/test/java/com/goxr3plus/streamplayer/stream/StreamPlayerMethodsTest.java index 98dcab0..1c247e4 100644 --- a/src/test/java/com/goxr3plus/streamplayer/stream/StreamPlayerMethodsTest.java +++ b/src/test/java/com/goxr3plus/streamplayer/stream/StreamPlayerMethodsTest.java @@ -135,7 +135,7 @@ void gain() throws StreamPlayerException, InterruptedException { @Test void logScaleGain() throws StreamPlayerException, InterruptedException { // Setup - final boolean listen = true; + final boolean listen = false; // Set to true to listen to the test. // Exercise @@ -299,8 +299,8 @@ void addStreamPlayerListener() throws StreamPlayerException, InterruptedExceptio verify(listener, times(1)).opened(any(), any()); - verify(listener, atLeast(5)).progress(anyInt(), anyLong(), any(), any()); - verify(listener, atMost(10)).progress(anyInt(), anyLong(), any(), any()); + verify(listener, atLeast(4)).progress(anyInt(), anyLong(), any(), any()); + verify(listener, atMost(30)).progress(anyInt(), anyLong(), any(), any()); // TODO: Make separate tests for the different calls made to the listener // TODO: Do we need to test the values passed to these methods? @@ -348,15 +348,25 @@ void speedFactor() throws StreamPlayerException, InterruptedException { @Test void equalizer() { player.setEqualizer(null, 0); - - fail("Test not done"); + // TODO: Find out what the intention of setEqualizer() is, and make a test for that assumption. } @Test - void play() throws StreamPlayerException { + void play() throws StreamPlayerException, InterruptedException { + // Setup + player.open(audioFile); + + // Pre-validate + assertFalse(player.isPlaying()); + + // Execute player.play(); - fail("Test not done"); + // Verify + assertTrue(player.isPlaying()); + + // TODO: Find way to verify that the player is actually playing, that doesn't need listening. + // The method might look at the playing position, but it must be fairly quick. } @Test @@ -378,17 +388,33 @@ void resume() throws StreamPlayerException { } @Test - void pause() { + void pause() throws StreamPlayerException { + + // Setup + player.open(audioFile); + player.play(); + // Pre-validate + assertFalse(player.isPaused()); + + // Execute player.pause(); - fail("Test not done"); + // Verify + assertTrue(player.isPaused()); + } @Test void stop() { + + assertFalse(player.isStopped()); + player.stop(); - fail("Test not done"); + assertTrue(player.isStopped()); + + // TODO: Find a way to verify that playback is stopped by running the stop method. + // The isStopped() method is not enough. } @Test @@ -412,22 +438,27 @@ void pan() throws StreamPlayerException { float precision = player.getPrecision(); assertNotEquals(0, precision); - assertEquals(3f, 1.0/precision); + double expected = 128.0; // Possibly platform dependent. Tested on a Mac with Intellij. + assertEquals(expected, 1.0/precision, 2.0); } @Test void unknown() { player.isUnknown(); - - fail("Test not done"); + // This is a useless test of a useless method. + // TODO: Remove player.isUnknown(). It's not used, and it's useless. + // There is already getStatus(). } @Test void open() throws StreamPlayerException { - File file = null; + File file = spy(audioFile); player.open(file); + verify(file, atLeast(1)).getPath(); - fail("Test not done"); + // It's unclear what the contract of open() is; what we need it to do. + // It's a pre-requisite for play(), but play() doesn't throw an + // exception if open() is missing. } @Test @@ -437,19 +468,11 @@ void mixers() { // There is nothing that can be done with the information outside the private scope. } - @Test - void seekBytes() throws StreamPlayerException { - player.open(audioFile); - int positionByte1 = player.getPositionByte(); - - player.seekBytes(100); - int positionByte2 = player.getPositionByte(); - fail("Test not done"); - } // The methods tested below aren't used elsewhere in this project, nor in XR3Player + // TODO: Consider each of the tested methods below, to see if they can be removed from StreamPlayer. @Test void lineBufferSize() {