Testing a console input/output

[Violet_82]

Hi guys, I was trying to test a method which takes a user input, does something and then returns that input but I'm having some issues with it. I've attempted a few solutions, mostly described in SE, I will leaave them out of this
post and discuss instead the one that seemed the most likely to work instead.
This is the method that I'm trying to test (other code omitted for brevity):

public int getUserInput() {
        int choice = 0;
        boolean isValidInput = false;
        System.out.printf("Welcome. Select action: %d for READ, %d for CREATE, %d for UPDATE, %d for DELETE, %d for EXIT.", OperationOptions.READ.getValue(), OperationOptions.CREATE.getValue(), OperationOptions.UPDATE.getValue(), OperationOptions.DELETE.getValue(), OperationOptions.EXIT.getValue());
        while(!isValidInput)
            try {                 
                choice = scanner.nextInt();
                if(choice == OperationOptions.READ.getValue() || choice == OperationOptions.CREATE.getValue() || choice == OperationOptions.UPDATE.getValue() || choice == OperationOptions.DELETE.getValue() || choice == OperationOptions.EXIT.getValue())
                {
                    isValidInput = true;
                }
                else
                {
                    isValidInput = false;
                    System.out.println(String.format(Messages.NOT_ALLOWED_VALUES, OperationOptions.READ.getValue(), OperationOptions.CREATE.getValue(), OperationOptions.UPDATE.getValue(), OperationOptions.DELETE.getValue(), OperationOptions.EXIT.getValue()));
                }

                scanner.hasNextLine();
            }
            catch(InputMismatchException e) {
                 System.out.println(Messages.NOT_NUMERICAL_ERROR);
                 isValidInput = false;
                 scanner.nextLine();                 
            }
        return choice;
    }

A word about the test. What I had in mind was to test the actual input, so have a test that fakes the input, then calls the method and check that the fake input is the same as the one returned from that method. That would be
the first test, then chose another input and make sure it generates an exception and so on.

So this is my test class

package com.test.userInteraction;

import static org.junit.Assert.assertEquals;

import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.ObjectOutput;
import java.io.ObjectOutputStream;
import java.io.PrintStream;

import org.junit.After;
import org.junit.Before;
import org.junit.Test;

public class UserInputTest {
    UserInput userInput;
    private final InputStream systemIn = System.in;
    private final PrintStream systemOut = System.out;

    private ByteArrayInputStream testIn;
    private ByteArrayOutputStream testOut;

    @Before
    public void setUpOutput() {
        testOut = new ByteArrayOutputStream();
        System.setOut(new PrintStream(testOut));
    }
    private String getOutput() {
        return testOut.toString();
    }

    @After
    public void restoreSystemInputOutput() {
        System.setIn(systemIn);
        System.setOut(systemOut);
    }

    @Test
    public void testCase1() {
          int testString = 3;

        ByteArrayOutputStream bos = new ByteArrayOutputStream();
        ObjectOutput out;
        try {
            out = new ObjectOutputStream(bos);
            out.writeInt(testString);
            out.close();
            byte[] int_bytes = bos.toByteArray();
            provideInput(int_bytes);
            userInput = new UserInput();
            int userInput2 = userInput.getUserInput();
            assertEquals(testString, userInput2);

            bos.close();
        } catch (IOException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }
    }

    private void provideInput(byte[] int_bytes) {
        testIn = new ByteArrayInputStream(int_bytes);
        System.setIn(testIn);

    }
}

So the problem is that it fails when it reachesint userInput2 = userInput.getUserInput(); inside getUserInput when it reaches choice = scanner.nextInt(); it generates a InputMismatchException and then controls goes back to choice = scanner.nextInt(); and then fails but I can't quite figure out why
The stack trace in the test is

java.util.NoSuchElementException
    at java.util.Scanner.throwFor(Scanner.java:862)
    at java.util.Scanner.next(Scanner.java:1485)
    at java.util.Scanner.nextInt(Scanner.java:2117)
    at java.util.Scanner.nextInt(Scanner.java:2076)
    at com.test.userInteraction.UserInput.getUserInput(UserInput.java:34)
    at com.test.userInteraction.UserInputTest.testCase1(UserInputTest.java:69)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
    at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
    at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
    at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
    at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
    at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
    at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
    at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
    at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
    at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
    at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
    at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
    at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
    at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:89)
    at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:41)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:541)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:763)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:463)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:209)

Any idea what is going on? It's like it doesn't like the fake input somehow

[JamesCherrill]

Why are you using an ObjectOutStream? Those streams write data in a binary format that only ObjectInputStream understands (see excerpt from API doc below).
Scanner expects data as simple text, so to create a file that it will understand you need a PrintStream to write simple text. Or simply create a String for the input and convert that to a byte array using your default locale encoding.

API doc:
Primitive data, excluding serializable fields and externalizable data, is written to the ObjectOutputStream in block-data records. A block data record is composed of a header and data. The block data header consists of a marker and the number of bytes to follow the header. Consecutive primitive data writes are merged into one block-data record. The blocking factor used for a block-data record will be 1024 bytes. (Etc)

[Violet_82]

I used ObjectOutStream mostly because the example I found did it, but it could easily be omitted from the code altogether.
The reason for using a byte[] is just so I could create a ByteArrayInputStream and inserting it in the stream to fake the input, or at least this is what I thought.
So, just to be sure I understand, you're saying have my testString (not a good name as it isn't a string anymore but an int now - I should've given it a better name) and then use a PrintStream to inject that test value in the stream (like use the PrintStream to create the byte[] and then create the ByteArrayInputStream)? I don't necessarily need a file, so probably a string would do.

[Violet_82]

I think I'll provide an example. This actually works OK

public void testCase1() {

          String testVal = "3";

            byte[] testVal_bytes = testVal.getBytes();

            provideInput(testVal_bytes);
            userInput = new UserInput();
            int userInput2 = userInput.getUserInput();
            assertEquals(Integer.parseInt(testVal), userInput2);    
    }

    private void provideInput(byte[] int_bytes) {
        testIn = new ByteArrayInputStream(int_bytes);
        System.setIn(testIn);

    }

Is this the kind of thing you had in mind? What doesn't really convince me is the fact that I had to use a String rather than an int

[tinstaafl]

It seems to me that with a bit of refactoring you can solve your problem and simplify things quite a bit.

First I would recommend making your menu choices char instead of int. The standard input from an ASCII keyboard will fit into a String. This eliminates the try block, since your code won't throw an error by the user pressing the wrong key.

From here a simple switch block with char cases, can validate the input. Simply extract the first character from scanner.next().

Assuming that your OperationOptions is set up something like this:

public enum OperationOptions {
    READ(0),
    CREATE(1),
    UPDATE(2),
    DELETE(3),
    EXIT(4);
    public int option;
    OperationOptions(int option) {
        this.option = option;
    }

    public int getValue() {
        return option;
    }
}

You can add the validation right into the enum:

public enum OperationOptions {
    READ('0'),
    CREATE('1'),
    UPDATE('2'),
    DELETE('3'),
    EXIT('4');
    public char option;
    OperationOptions(char option) {
        this.option = option;
    }
    public static boolean isValid(char choice){
        switch(choice){
            case '0':
            case '1':
            case '2':
            case '3':
            case '4':
                return true;
            default:
                System.out.println(String.format(Messages.NOT_ALLOWED_VALUES, OperationOptions.READ.getValue(), OperationOptions.CREATE.getValue(), OperationOptions.UPDATE.getValue(), OperationOptions.DELETE.getValue(), OperationOptions.EXIT.getValue()));
                return false;
        }
    }
    public char getValue() {
        return option;
    }
}

The getUserInput method now can look like this:

public int getUserInput() {
    char choice = '\0';
    boolean isValidInput = false;
    System.out.printf("Welcome. Select action:\n\t%c - READ\n\t%c - CREATE\n\t%c - UPDATE\n\t%c - DELETE\n\t%c - EXIT\n", OperationOptions.READ.getValue(), OperationOptions.CREATE.getValue(), OperationOptions.UPDATE.getValue(), OperationOptions.DELETE.getValue(), OperationOptions.EXIT.getValue());
    Scanner scanner = new Scanner(System.in);
    while (!isValidInput) {
        choice = scanner.next().charAt(0);
        isValidInput = OperationOptions.isValid(choice);
        scanner.hasNextLine();
    }
    return choice;
}

Now instead of manipulating streams, your test methods can simply pass a char to the isValid method in the OperationOptions enum.

On a side note, displaying your menu as a list is much easier for the user to read and understand, than a long string.

[JamesCherrill]

Violet: yes, your latest code is exactly what I was suggesting. Don't forget that your Scanner is reading text strings from as text input stream. int is a binary value. The Scanner expects the string representation of an integer value, and its nextInt() method parses that and converts it to an int. There are noints in the input stream!
A simple change of name may make this clearer...

String correctInput = "3\n" ; // this is what the user should have typed
byte[] (etc)

[JamesCherrill]

If, on the other hand, we are talking about refactoring, then I personally would go towards something like this:

    HashMap<String, Runnable> OPTIONS = new HashMap<>();   {
        OPTIONS.put("A", this::add);
        OPTIONS.put("D", this::delete);
        // etc
    }

    BufferedReader in = new BufferedReader(new InputStreamReader(System.in));

    void menu() {
        System.out.println("usual prompts");
        String input = in.readLine();
        OPTIONS.getOrDefault(input, this::inputError).run();
    }

    void add() {
        // do whatever
    }

    void delete() {
        //do whatever
    }

    void inputError() {
        System.out.println("error message");
        menu(); // try again
    }

[Violet_82]

Thanks for the feedback. I like your idea about the the enum but having the validation directly in it seems a bit odd as opposed to have it in a separate class?
Sorry I didn't share the full story here but this is essentially just a bit of a test console application. I'm planning to rewrite the whole thing and add some decent a front end to it too (maybe angular, I haven't decided yet and this will be, at some point soon, the subject of a different thread) so any tip for a rewrite is of course VERY welcome, like for the example the enumeration bit.
One thing that became obvious in here is the fact that the tests are now being added almost as an afterthought, so during the rewrite I will try to stick to a more TDD approach: granted it will take more time but the code should in theory be more robust, and it will be a good exercise anyway.

For now, I will finish off the tests, upload the SQL scripts and then move one :-)

With the refactoring, were you referring to the class that does the CRUD operations?

[tinstaafl]

One of the reasons I made the isValid method static is to allow for easy refactoring if necessary. I kept it in the enum to make it more obvious what was being validated against. Moving to another class could possibly complicate that. Keep in mind the Singular Responsibility Principle when designing your functions, it makes testing and bug tracing much easier.

[JamesCherrill]

I agree that isValid belongs in the enum, but I don't agree with generatng a complete error message at that level - it belongs in the UI layer.
Rather than the switch, why not check to see if the char is inOperationOptions.values() ? That avoids having to add members in two different places.

Executing the menu and checking the user input is only part of the story - you the have to execute the relevant code for each option. That's why I favour a map<user choice, runnable> (where user choice could be a char, sring or number, and the runnable is a lambda). The menu then reduces to one or two lines of code to validate and execute the choice, and new options just need an additional entry in the map and a method for their implementation. Personally I find that much simpler and clearer, and easier to extend.

[Violet_82]

Thanks. A few things from my side :-)
So String correctInput = "3\n" ; // this is what the user should have typed would mean that I would have to do something like this though assertTrue(testVal.equals(String.valueOf(userChoice))); and - I haven't tried yet, hopefully tomorrow - perhaps remove that \n from the string before doing the assertion.
Also, one thing occurred to me. If I then run another test, say something like this

 @Test
    public void test() {
        String testVal = "99";
        byte[] testVal_bytes = testVal.getBytes();      
        insertInStream(testVal_bytes);
        userInput = new UserInput();
        int userChoice = userInput.getUserInput();

    }

99 isn't within the valid values but the getUserInput() method keeps asking for an input till the user uses a valid one. So, the above test will inject 99 in the stream, great, but the getUserInput() will correctly identify that as a non valid input and then will ask again for another input and the stram doesn't have anything else in it so it will fail miserably. So, how would I go about testing this then?

[JamesCherrill]

presumably you should give it another input, eg "99\n1\n"
(I'm unclear on exactly what you are testing for here)

[Violet_82]

Well, maybe there isn't anything to test there....I thought I'd test a range of values, right ones and not right ones, but maybe I don't really need to do that, not sure to be honest