JUnit-what to test?

[karina val]

I have to write JUnit test cases for my class CheckVul (code below) but it's my first time using JUnit and am stuck on how to make test cases for these methods since they all are void, and write to a file. The checkDatabase method takes in a 'profile name' and a 'year' provided by the user, to compare that profile with the database of that year and check for vulnerabilities. I have a basic understanding of how to write test cases for simple methods that return something, etc. but not for this. Any suggestions? :S I was thinking making a test case for the checkDatabase method would be simple, but not sure how to go about it.

import java.io.BufferedReader;
import java.io.FileNotFoundException;
import java.io.FileReader;
import java.io.IOException;
import java.util.Scanner;

public class CheckVul {
	
	/**
	 * Vulnerability databases.
	 */
	private String file = "Vulnerability_Recent.txt";  
	private String file2 = "Vulnerability_2011.txt";
	private String file3 = "Vulnerability_2010.txt";
	private String file4 = "Vulnerability_2009.txt";
	
	GenerateReport report = new GenerateReport();
	Scanner s,s2;
	
	public void checkDatabase(String f, String y) throws IOException{
		
		if(y.equals("2012")){
			checkVulRecent(f);
		}
		else if(y.equals("2011")){
			checkVul2011(f);
		}
		else if(y.equals("2010")){
			checkVul2010(f);
		}
		else if(y.equals("2009")){
			checkVul2009(f);
		}
		else{
			System.out.println("Sorry. Database was not found.");
		}
	}
	
	/**
	 * The checkVulnerabilites method will ask the user to input conditions 1, 2, 3, or 4 corresponding to their
	 * respective year. Once that's determined, the method will check their System Profile for vulnerabilities.
	 * 
	 * @param file The name of the system profile.
	 * @throws IOException If valid input isn't found for making a new file.
	 */
	public void checkVulRecent(String f) throws IOException {
		compare(f, file);
	} // end if
		
	public void checkVul2011(String f) throws IOException {
		compare(f, file2);
	} 
	
	public void checkVul2010(String f) throws IOException {
		compare(f, file3);
	}
		
	public void checkVul2009(String f) throws IOException {
		compare(f, file4);
	}

	/**
	 * The inputFile method is used to read a file within the checkVulnerabilities
	 * class.
	 * 
	 * @param f The name of the file.
	 * @return Buffered Reader 'scan' object for reading files.
	 * @throws FileNotFoundException If the file is not found.
	 */
	private Scanner inputFile(String f) throws FileNotFoundException {
		FileReader fr = new FileReader(f);
		BufferedReader br = new BufferedReader(fr);
		Scanner scan = new Scanner(br);

		return scan;
	}
	
	private void compare(String profile, String database) throws IOException{
		
		report.newFile(profile);
		s2 = inputFile(profile + ".txt"); // file for system profile
			
		while(s2.hasNext()){
			s = inputFile(database); // data base file
				
			String thisLine2 = s2.nextLine();
			String[] lineTokens2 = thisLine2.split(";");
				
			while (s.hasNext()) {
				String thisLine = s.nextLine();
				String[] lineTokens = thisLine.split(";");

				if (lineTokens[1].equals(lineTokens2[0])) { // software comparison
					if (lineTokens[2].equals(lineTokens2[1])){ // version comparison
						report.write(lineTokens, 1, 2, 0, 3, 4, 5);				
					}
				}
			}
		}
		report.writeClose();
		System.out.println("Vulnerability Report\n");
		report.print();
		report.readClose();
	}
}// end CheckVul

[Junilu Lacar]

If I were the one being asked to test this, I would first have a serious talk with the developer about the design.

First, it's not unit testable since it expects input from a file. A unit test should not cross system boundaries, that is, it should not access the file system, a database, a web service, etc. For this class, you can only write integration tests, which are expected to cross system boundaries.

Second, the design violates the Open-Closed Principle. What happens in 2013? In 2014? In 2015? etc. Seems like you'd need to change this code every year. That's not a good design.

One other nitpick (my pet peeve): why skimp on keystrokes and make the class name less readable? What's wrong with naming the class VulnerabilityChecker or something that is easy to read without having to do a mental expansion? Write code with the reader in mind. Don't skimp on keystrokes this way. BTW, a class name should generally be a noun or noun phrase. CheckVul, assuming "Vul" is a lazy short form of "Vulnerability", is a verb phrase and is more suitable as a method name.

[Junilu Lacar]

Another thing, why are the checkVul**() methods public? Shouldn't these be private instead? It looks like the checkDatabase() method is meant to be called and then it will call the appropriate checkVul**() method. Since the checkVul** methods are public, there's nothing that prevents them from being called with the wrong parameter values. If they are meant to be public, then why have the checkDatabase() method at all?

[Jeanne Boyarsky]

You could have the unit test read the file to assert what was written out. Or you could refactor GenerateReport to provide a way to write to a variable rather than a file.