Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Removing throwing of IllegalStateException
Convert throwing of IllegalStateException to setting a boolean variable, printing an error with DebugTool, and returning
Changed unit tests to reflect this new intended behavior
Added a private boolean variable and public getter function to make unit testing simpler
The alternative to reading a flag to trigger tests failing is reading logs programmatically, which is more complicated and less reliable
  • Loading branch information
noah-livio committed Dec 14, 2021
commit 0aebb4ea1c628ee2eef98475bd56fa69f0ca87a9
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import java.util.Collections;
import java.util.List;


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just cleaning up white space.

import static junit.framework.TestCase.assertEquals;
import static junit.framework.TestCase.assertFalse;
import static junit.framework.TestCase.assertNull;
Expand All @@ -56,13 +57,12 @@
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

import junit.framework.TestCase;

/**
* This is a unit test class for the SmartDeviceLink library manager class :
* {@link SoftButtonManager}
*/
@RunWith(AndroidJUnit4.class)
//@RunWith(JUnit4.class)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can remove this comment as well

public class SoftButtonManagerTests {

private SoftButtonManager softButtonManager;
Expand Down Expand Up @@ -411,14 +411,12 @@ public void testSoftButtonStateEquals() {
@Test
public void testConstructSoftButtonObjectWithEmptyStateList() {
List<SoftButtonState> stateList = new ArrayList<>();
// Uncommenting the following lines should make the test fail
// SoftButtonState softButtonState = new SoftButtonState("object1-state1", "o1s1", null);
// stateList.add(softButtonState);
SoftButtonObject softButtonObject = new SoftButtonObject("hi", stateList, "Hi", null);

try {
new SoftButtonObject("hi", stateList, "Hi", null);
TestCase.fail("IllegalStateException expected");
}
catch (IllegalStateException ignored) {

}
assertTrue(softButtonObject.getAttemptedToAssignEmptyStateList());
}

/**
Expand All @@ -429,14 +427,9 @@ public void testAssignEmptyStateListToSoftButtonObject() {
List<SoftButtonState> stateList = new ArrayList<>();
SoftButtonState softButtonState = new SoftButtonState("object1-state1", "o1s1", null);


try {
SoftButtonObject softButtonObject = new SoftButtonObject("hi", softButtonState, null);
softButtonObject.setStates(stateList);
TestCase.fail("IllegalStateException expected");
}
catch (IllegalStateException ignored) {

}
SoftButtonObject softButtonObject = new SoftButtonObject("hi", softButtonState, null);
// Commenting the following line should make the test fail
softButtonObject.setStates(stateList);
assertTrue(softButtonObject.getAttemptedToAssignEmptyStateList());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import com.smartdevicelink.proxy.rpc.OnButtonPress;
import com.smartdevicelink.proxy.rpc.SoftButton;
import com.smartdevicelink.util.DebugTool;
import com.smartdevicelink.util.Log;

import java.util.Collections;
import java.util.List;
Expand All @@ -61,6 +62,7 @@ public class SoftButtonObject implements Cloneable{
private int buttonId;
private OnEventListener onEventListener;
private UpdateListener updateListener;
private boolean attemptedToAssignEmptyStateList = false;

/**
* Create a new instance of the SoftButtonObject with multiple states
Expand All @@ -70,7 +72,6 @@ public class SoftButtonObject implements Cloneable{
* @param initialStateName a String value represents the name for the initial state
* @param onEventListener a listener that has a callback that will be triggered when a button event happens
* Note: the initialStateName should match exactly the name of one of the states for the object. Otherwise an exception will be thrown.
* @throws IllegalStateException if states is an empty list
*/
public SoftButtonObject(@NonNull String name, @NonNull List<SoftButtonState> states, @NonNull String initialStateName, OnEventListener onEventListener) {

Expand Down Expand Up @@ -264,12 +265,13 @@ public List<SoftButtonState> getStates() {
* Set the the SoftButtonState list
*
* @param states a list of the object's soft button states. <strong>states should be unique for every SoftButtonObject. A SoftButtonState instance cannot be reused for multiple SoftButtonObjects.</strong>
* @throws IllegalStateException if states is an empty list
*/
public void setStates(@NonNull List<SoftButtonState> states) {
// Make sure the list of states is not empty
if (states.isEmpty()) {
throw new IllegalStateException("The state list is empty");
// If the list of states is empty, throw an error with DebugTool and return
attemptedToAssignEmptyStateList = states.isEmpty();
if (attemptedToAssignEmptyStateList) {
DebugTool.logError(TAG,"The state list is empty");
return;
}

this.states = states;
Expand Down Expand Up @@ -403,4 +405,13 @@ public SoftButtonObject clone() {
}
return null;
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just cleaning up whitespace

/**
* Used to make unit testing easier by removing the need to read debug logs programmatically
*
* @return True if the last list passed to setStates() was empty and false otherwise
*/
public boolean getAttemptedToAssignEmptyStateList() {
return attemptedToAssignEmptyStateList;
}
}