-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
created tests for FruitShop #785
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job. Global comment: names for test.methods + i'm not sure about initialisation in beforeAll, but if you think it's correct, leave it for mentors
String expected = "fruit,quantity\n" | ||
+ "banana,152\r\n" | ||
+ "apple,90\r\n"; | ||
List<String> inputData = new ReadServiceImpl().readInputData(INPUT_PATH); | ||
List<FruitTransaction> parseData = new ParseServiceImpl().parseInputData(inputData); | ||
operationService = new OperationService(operationStrategy); | ||
operationService.processOperation(parseData); | ||
String actual = new ReportServiceImpl().createReport(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unit testing is isolated testing. Ensure that you test your services and their independently. I think you are using a lot of classes here
emptyList = new ArrayList<>(); | ||
inputData = new ArrayList<>(Arrays.asList( | ||
"type,fruit,quantity", | ||
"b,banana,100", | ||
"s,banana,100", | ||
"p,banana,100", | ||
"r,banana,100")); | ||
parseData = parseService.parseInputData(inputData); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you need to use it in test methods.
import java.util.Map; | ||
|
||
public class ReportServiceImpl implements ReportService { | ||
private static final String FIRST_ROW = "fruit,quantity\n"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use System.lineSeparator(), not \n.
String expected = FIRST_ROW | ||
+ "banana,20\r\n" | ||
+ "apple,15\r\n"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use System.lineSeparator()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good Job! Lets improve your solution)
+ System.lineSeparator(); | ||
List<String> inputData = new ReadServiceImpl().readInputData(INPUT_PATH); | ||
List<FruitTransaction> parseData = new ParseServiceImpl().parseInputData(inputData); | ||
operationService = new OperationService(operationStrategy); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
u must initializate it in BeforeAll method
List<FruitTransaction> parseData = new ParseServiceImpl().parseInputData(inputData); | ||
operationService = new OperationService(operationStrategy); | ||
operationService.processOperation(parseData); | ||
String actual = new ReportServiceImpl().createReport(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
u must test here only operationService. Dont use another services
private static final int BALANCE_INDEX = 0; | ||
private static final int SUPPLY_INDEX = 1; | ||
private static final int PURCHASE_INDEX = 2; | ||
private static final int RETURN_INDEX = 3; | ||
private static final String FRUIT = "banana"; | ||
private static final int QUANTITY = 100; | ||
private static ParseService parseService; | ||
private static List<String> inputData; | ||
private static List<String> emptyList; | ||
private static List<FruitTransaction> parseData; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
private static final int BALANCE_INDEX = 0; | |
private static final int SUPPLY_INDEX = 1; | |
private static final int PURCHASE_INDEX = 2; | |
private static final int RETURN_INDEX = 3; | |
private static final String FRUIT = "banana"; | |
private static final int QUANTITY = 100; | |
private static ParseService parseService; | |
private static List<String> inputData; | |
private static List<String> emptyList; | |
private static List<FruitTransaction> parseData; | |
private static ParseService parseService; |
you don't really need these variables. Create them and use them in the test methods you need. Likewise in other test classes
} | ||
|
||
@Test | ||
void correctParseData_Ok() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
which method is being tested here - not an informative method name. The same in other tests
"p,banana,100", | ||
"r,banana,100")); | ||
parseData = parseService.parseInputData(inputData); | ||
FruitTransaction balanceTransaction = parseData.get(BALANCE_INDEX); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here you can create a list that you expect to return as a result of executing the method and compare
String expected = FIRST_ROW | ||
+ "banana,20" | ||
+ System.lineSeparator() | ||
+ "apple,15" | ||
+ System.lineSeparator(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets use StrringBuilder
class WriteServiceImplTest { | ||
private static final String OUTPUT_PATH = "src/main/java/resources/output.csv"; | ||
private static final String REPORT = "fruit,quantity\n" | ||
+ "banana,152\r\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not System.lineSeparator()?
|
||
@Test | ||
void writeReport_Ok() { | ||
String actual = writerService.writeToFile(OUTPUT_PATH, REPORT); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here it is better to read the result from the corresponding file and compare the data
|
||
@Test | ||
void getSuppleHandler_Ok() { | ||
operationHandler = new SupplyOperationHandler(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets initializate it in BeforeAll method
import org.junit.jupiter.api.Test; | ||
|
||
class ReportServiceImplTest { | ||
//private static final String FIRST_ROW = "fruit,quantity" + System.lineSeparator(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't forget to remove commented code
No description provided.